moleculer

moleculer copied to clipboard

Prerequisites

Please answer the following questions for yourself before submitting an issue.

• [X] I am running the latest version
• [X] I checked the documentation and found no answer
• [X] I checked to make sure that this issue has not already been filed
• [X] I'm reporting the issue to the correct repository

Current Behavior

We (@rulrok and me) currently create and destroy services on our project. After destroying a service (With broker.destroyService), we are running into an increasing amount of memory usage.

After some research, we found out that the services objects are destroyed correctly on broker.services, but on the registry (registry.nodes.localNodes) and discoverer (that uses the same object as the registry, as you can see below), the objects are left there, without deletion, leading to a memory leak as the garbage collector can't remove them. (See picture one)

https://github.com/moleculerjs/moleculer/blob/6ea23c6280179cfc8eb7d8f01c09c1d65a4a7a07/src/registry/discoverers/base.js#L69

Events too are still being persisted, even after deleting the services.

After clearing the services by hand on the chrome dev console, the garbage collector worked as intended, but it still shows the deleted services on the actions -a on moleculer-cli. (step by step on the images below).

(We are currently using TCP transporter with local discovery on a monolithic model)

Expected Behavior

It is expected that the destroyService would remove the services from the registry (as well as from the discoverer) resulting in the deletion of the references, making the objects free to be garbage collected.

Failure Information

The project fail after a lot of creation and deletions because of the memory leak

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

1. Make a service that creates services on demand
2. Try and delete these services (broker.destroyService)
3. Look into the registry.nodes.localNode and you'll see the services are still referenced there.

Reproduce code snippet

https://codesandbox.io/s/moleculer-sample-forked-h7xdxe?file=/index.js

const { ServiceBroker } = require("moleculer");

const broker = new ServiceBroker();

broker.createService({
  name: "service-creator",
  actions: {
    createService(ctx) {
      return this.broker.createService({
        name: `${ctx.params.name}-${ctx.params.id}`
      });
    }
  }
});

broker.start().then(() => {
  let id = 1;
  setInterval(() => {
    broker
      .call("service-creator.createService", { name: "Service", id: id++ })
      .then((res) => {
        broker.logger.info(res.name);
        return res;
      })
      .then(async (res) => broker.destroyService(res))
      .then(() => {
        broker.logger.info(
          `broker.registry.nodes.localNode.services.length: ${broker.registry.nodes.localNode.services.length}`
        );
        broker.logger.info(`broker.services.length: ${broker.services.length}`);
      })
      .catch((err) => broker.logger.error(err.message));
  }, 1000);
});

Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

• Moleculer version: 0.14.22
• NodeJS version: v16.16
• Operating System: Ubuntu 20.04.4 LTS and Fedora 36

Failure Logs

---

---

Update (2022-08-15)

Issue still occurs on version 0.14.22 (sandbox updated)

Hi, I was also involved in looking up this issue.

Some points we thought worth looking into:

Maybe it missed removing nodes in this point: https://github.com/moleculerjs/moleculer/blob/6ea23c6280179cfc8eb7d8f01c09c1d65a4a7a07/src/registry/service-catalog.js#L221-L228

Because there is a deepClone happening here, once the original references are deleted, these ones remaing untouched it seems. https://github.com/moleculerjs/moleculer/blob/6ea23c6280179cfc8eb7d8f01c09c1d65a4a7a07/src/registry/node.js#L67

Here, maybe it could be related to the issues reported for the actions -a command https://github.com/moleculerjs/moleculer/blob/6ea23c6280179cfc8eb7d8f01c09c1d65a4a7a07/src/registry/endpoint-list.js#L240-L248

And finally here: could it be that running as a monolith would have some interference with the localNode variable not cleaning the services array. https://github.com/moleculerjs/moleculer/blob/6ea23c6280179cfc8eb7d8f01c09c1d65a4a7a07/src/registry/node-catalog.js#L133-L153

Thanks for the detailed report, I will check it.

Just an update that could help:

We are using a forced hack to clean up the resources and so far the memory has been more stable along the week (yet higher than what we anticipated)

function tryClearMoleculerLeakedResources(broker: ServiceBroker, serviceName: string) {
  try {
    let totalCleared = 0;
    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
    // @ts-ignore
    const leakedServices = broker.registry.discoverer.localNode.services;
    const foundIdx = leakedServices.findIndex((s: any) => s.name === serviceName);
    if (foundIdx > -1) {
      totalCleared += 1;
      leakedServices.splice(foundIdx, 1);
    }

    const leakedActions: Map<string, never> = broker.registry.actions.actions;
    Array.from(leakedActions.keys())
      .filter((k: string) => k.startsWith(serviceName))
      .forEach((keyName) => {
        totalCleared += 1;
        leakedActions.delete(keyName);
      });

    const leakedEventsIdxs: Array<number> = [];
    broker.registry.events.events.forEach((e: any, idx: number) => {
      if (e.group === serviceName) {
        leakedEventsIdxs.push(idx);
      }
    });

    // As splice re-indexes the array, the removal while looping must occur backwards.
    leakedEventsIdxs.sort().reverse(); // reverse array in-place

    leakedEventsIdxs.forEach((leakIdx) => {
      totalCleared += 1;
      broker.registry.events.events.splice(leakIdx, 1);
    });

    metricGeneralCount.inc({
      metric: 'memory-leak:cleared-resources',
    }, totalCleared);
  } catch (e) {
    foxLogger.crit('Failed to clean broker resources', e);
  }
}

I've fixed. @Marinell0 could you test it plz?

Hello @icebob, thanks for the fix!

With a limited test I made in this repository, the fix worked correctly :smile:

When it gets released we will test it in production.

@rulrok

Awesome, thanks for your feedback.