node-resque icon indicating copy to clipboard operation
node-resque copied to clipboard

Worker did not remove the event listener when it ended.

Open peteAhn opened this issue 2 years ago • 7 comments

Hi, I've found the following warning message.

(node:80974) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 21 end listeners added to [Redis]. 
Use emitter.setMaxListeners() to increase limit

And then investigated it. As a result, First, the multi-worker had been running one or two if the queue is empty. Second, Each worker added the event listener into the Redis when the worker started. Finally, any worker did not remove the event listener when it ended.

As the time goes the event listener adds infinitely. I think when the worker ends - in the end method of the worker, it should remove the event listener in the Redis.

Please let me know your opinion. Thanks.

peteAhn avatar May 20 '22 08:05 peteAhn

We should be doing most of that, e.g. https://github.com/actionhero/node-resque/blob/main/src/core/multiWorker.ts#L275-L277. There might be bugs! Can you share a way to reproduce that problem?

evantahler avatar May 20 '22 22:05 evantahler

We should be doing most of that, e.g. https://github.com/actionhero/node-resque/blob/main/src/core/multiWorker.ts#L275-L277. There might be bugs! Can you share a way to reproduce that problem?

Hi, evantahler.

The worker has the queueObject and it is connected. https://github.com/actionhero/node-resque/blob/5092dc114ccbfe1de79ae02f7ee5bfb207561473/src/core/worker.ts#L141

At this time, In the connection class, it is registered the event listener on the Redis. https://github.com/actionhero/node-resque/blob/5092dc114ccbfe1de79ae02f7ee5bfb207561473/src/core/connection.ts#L78

However, in the end method of the worker class, the connection is ended. https://github.com/actionhero/node-resque/blob/5092dc114ccbfe1de79ae02f7ee5bfb207561473/src/core/worker.ts#L190

But also, In the connection class, it is not deregistered the event listener on the Redis. So then whenever the worker connects it, the event listener adds infinitely. https://github.com/actionhero/node-resque/blob/5092dc114ccbfe1de79ae02f7ee5bfb207561473/src/core/connection.ts#L144

I hope that makes sense. Please feel free if you have any further questions.

Thanks.

peteAhn avatar May 23 '22 02:05 peteAhn

We should be doing most of that, e.g. https://github.com/actionhero/node-resque/blob/main/src/core/multiWorker.ts#L275-L277. There might be bugs! Can you share a way to reproduce that problem?

Each workers add the 'error', 'end' event listeners to Redis at worker start. When the worker end, the event listeners added to the worker self are removed, but the listeners added to Redis are not removed.

Here is example for reproduce this problem:

import { Connection, MultiWorker, Queue } from 'node-resque'

const connectionDetails = {
  pkg: 'ioredis',
  host: '[127.0.0.1](http://127.0.0.1/)',
  password: null,
  port: 6379
}

const connection = new Connection(connectionDetails)

await connection.connect()
console.log('connection connected:', connection.redis.listenerCount('error'), 'error listeners added to [Redis]')

const queue = new Queue({ connection: connection }, {})
queue.connect()
console.log('queue connected:', connection.redis.listenerCount('error'), 'error listeners added to [Redis]')

const multiWorker = new MultiWorker({ connection: connection, queues: [] }, {})

multiWorker.on('start', (workerId) => {
  console.log('worker[' + workerId + '] started:', connection.redis.listenerCount('error'), 'error listeners added to [Redis]')
})
multiWorker.on('end', (workerId) => {
  console.log('worker[' + workerId + '] ended:', connection.redis.listenerCount('error'), 'error listeners added to [Redis]')
})

multiWorker.start()

/*
result:
connection connected: 1 error listeners added to [Redis]
queue connected: 2 error listeners added to [Redis]
worker[1] started: 3 error listeners added to [Redis]
worker[2] started: 4 error listeners added to [Redis]
worker[2] ended: 4 error listeners added to [Redis]
worker[2] started: 5 error listeners added to [Redis]
worker[2] ended: 5 error listeners added to [Redis]
worker[2] started: 6 error listeners added to [Redis]
worker[2] ended: 6 error listeners added to [Redis]
worker[2] started: 7 error listeners added to [Redis]
worker[2] ended: 7 error listeners added to [Redis]
... ...
*/

bbyong2410 avatar May 23 '22 15:05 bbyong2410

Amazing - please submit a PR with the fix, and I'll get a release out quickly!

evantahler avatar May 24 '22 01:05 evantahler

Hi! Came here to report this problem. Are there any plans to still merge this?

herval avatar Sep 06 '22 04:09 herval

A PR with a fix was never submitted. Please let me know if you need help working on this!

evantahler avatar Sep 06 '22 04:09 evantahler

I'm not sure I have enough familiarity w/ the codebase to contribute atm :-(

herval avatar Sep 06 '22 04:09 herval