Mqtt.js: Memory leak in mqtt client

[stephendeyoung]

I think I've discovered a memory leak in mqtt.js. Here is an example script:

https://gist.github.com/stephendeyoung/3180ce2ec527332e0ece6eee573e6ecb

Steps to reproduce:

1. Run script with 100 mqtt clients and using the node.js inspector: NUMBER_OF_CLIENTS=100 node --inspect mqtt-memory-leak.js
2. Wait for all mqtt clients to connect
3. Take a heap snapshot with the Chrome devtools
4. Wait 5-10 minutes
5. Take another heap snapshot
6. Wait another 5-10 minutes
7. Take a final heap snapshot

You should see from the snapshots that memory use is growing over time. Here is an example:

https://ufile.io/bdmme

You can even see this for just 1 mqtt client although you have to wait quite a long time before creating snapshots to see the effect.

For my particular use case I wanted to create over 1000 mqtt clients in a node.js script but I kept on running out of memory and it seems like this memory leak is causing the problem.

I am using version 2.16.0 of mqtt.js. Please let me know if I've made a mistake anywhere.

[mcollina]

@stephendeyoung unfortunately I will have no time to investigate this issue for the next few weeks. Feel free to send a pull request if you figure it out.

[RangerMauve]

Nothing so far, I don't think anyone has looked into it in detail. If you have time, you could help by running a node process and using the new inspector feature to take memory snapshots to find where the leak is happening.

[senthil5053]

I have exact same issue in my application as well.
@stephendeyoung Did you find any solution or work around for this ?

[stephendeyoung]

@senthil5053 no i didn't unfortunately. Sorry I can't help.

[aaamitsingh]

Any update on this issue?

[svogl]

I ran into the same issue and had a look around. I see memory use increasing in a minimal example that just receives (frequent) messages in an empty message handler. looking further, I suspect calling the work function with process.nextTick in lib/client.js

https://github.com/mqttjs/MQTT.js/blob/0379bf53a8ec803b0ac3ccbe94e22467abe57213/lib/client.js#L259

directly calling work() instead results in a much flatter memory consumption line. I don't know which side-effects this would cause when you're reconnecting with many messages in the queues though.

Any ideas on this @stephendeyoung ?

Simon

[RangerMauve]

@svogl Which version of node are you using to test this?

[svogl]

I have a nodejs 10.15.0 running on my mac, on one of our ARM boards (where memory hurts), there's a node 9.8 running at the moment; same hack applied, memory consumption seems meander in a stable range

[RangerMauve]

Weird, there's nothing obvious that would cause this to happen. If you have time, do you think you could use the memory snapshot feature of the node inspector to see what exactly is being stored in memory?

As far as I can see, it should be garbage collectable once the function exits.

Another thing you could try is setting node ---max-old-space-size=666 where 666 is the memory limit in MB before it starts garbage collecting more aggressively.

[svogl]

I created a small test set and a few inspector screenshots saved in this gist:

https://gist.github.com/svogl/4425b31e45b64a5fbb65a83e6c7d7946

I call gc() some times before collecting dumps to compare them, there is no other traffic except 1k packets sent from the sender code.

Some of the packets seem to be coming from inspector itself, but I see many occasions of leftover objects coming from a) copied buffer lists from bl/bl.js. and b) many process/nextTick generated small code blocks.

Still, I might be looking at a totally wrong corner, ideas welcome :)

[RangerMauve]

Have you tried setting the --max-old-space-size flag? It might be that the garbage collector isn't kicking in because it doesn't think it's reached a high enough limit.

[svogl]

If you look at the gist link, I forced the gc to run with
node --inspect --gc-interval=100 --trace_gc --expose-gc reader.js
and explicitly calling gc() ; with the --trace_gc you get a line of information whenever it ran, so yes, it did run.

[RangerMauve]

This really seems like an issue with node internals rather than something MQTT.js has control over.
The reason I say this is the newAsyncId call in one of your screenshots that's a node internal.

Is this reproducible if you generate some junk data and process it within process.nextTick calls?

[svogl]

Set aside the root cause of the leak, I rewrote nextTickWork to call work only when there are packets left to process -

  function nextTickWork () {
    if (packets.length) {
      process.nextTick(work)
    }
  }

seems that solves it for me so far and creates less objects on the fly (good for embedded systems :) )
Does this cause any adverse side effects I am not aware of?

[rodrigomvf]

When this patch wil be release?

[brettjashford]

When this patch wil be release?

a little late, but looks like this got released in 3.0.0

[YoDaMa]

yeah gonna go ahead and close this issue.