Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] Memory leak on client.js 'connect' event #348

Closed
srmarjani opened this issue Jan 1, 2020 · 4 comments
Closed

[bug] Memory leak on client.js 'connect' event #348

srmarjani opened this issue Jan 1, 2020 · 4 comments
Labels
Projects

Comments

@srmarjani
Copy link

Hi
I am using mqtt as mqtt client.
In my use case I need to send about 1000 QOS1 packet every 4 second.
every 4 seconds I open a new client by mqtt.connect then I publish 1000 packets by p-map by concurrency 100 and at the end I end connection

But I am encountering below warning at aedes side

(node:15528) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 connected listeners added. Use emitter.setMaxListeners() to increase limit

And the stack trace is

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 connected listeners added. Use emitter.setMaxListeners() to increase limit
    at _addListener (events.js:247:17)
    at Client.addListener (events.js:263:10)
    at Parser.enqueue (***\node_modules\aedes\lib\client.js:313:17)
    at Parser.emit (events.js:189:13)
    at Parser._newPacket (***\node_modules\mqtt-packet\parser.js:631:10)
    at Parser.parse (***\node_modules\mqtt-packet\parser.js:39:41)
    at Socket.nextBatch (***\node_modules\aedes\lib\client.js:68:23)
    at Socket.emit (events.js:189:13)
    at emitReadable_ (_stream_readable.js:535:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)

I think the problem is in this function

function enqueue (packet) {
  this.client._parsingBatch++
  // already connected or it's the first packet
  if (this.client.connackSent || this.client._parsingBatch === 1) {
    handle(this.client, packet, this.client._nextBatch)
  } else {
    this.client.on('connected', () => {
      handle(this.client, packet, this.client._nextBatch)
    })
  }
}

Is there any problem in opening and closing the connection to broker?

Thanks

@robertsLando
Copy link
Member

@srmarjani Try to set concurrency option of aedes to 200

@robertsLando
Copy link
Member

@srmarjani Did you fixed? Can this be closed?

@srmarjani
Copy link
Author

Hi
I will try to reproduce the bug. because I choose another approach.
But I guess increasing the concurrency is not a solution.
Because concurrency is 100 by default but as you see in warning 11 event on connected event of client make problem.

@robertsLando
Copy link
Member

robertsLando commented Jan 23, 2020 via email

@robertsLando robertsLando changed the title (node:16044) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 connected listeners added. [bug] Memory leak on client.js 'connect' event Jan 23, 2020
@robertsLando robertsLando added this to To do in TODOs Jan 24, 2020
@robertsLando robertsLando moved this from To do to In progress in TODOs Jan 24, 2020
robertsLando added a commit that referenced this issue Jan 28, 2020
fix: Memory leak on client.js 'connected' event #348
@robertsLando robertsLando moved this from In progress to Done in TODOs Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
TODOs
  
Done
Development

No branches or pull requests

2 participants