Possible Memory Leak: SocketIO + RedisStore

(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace: 
    at RedisClient.<anonymous> (events.js:126:17)
    at RedisClient.subscribe (/var/nodejs/lib/node_modules/socket.io/lib/stores/redis.js:116:18)
    at RedisClient.emit (events.js:81:20)
    at RedisClient.return_reply (/var/nodejs/lib/node_modules/socket.io/node_modules/redis/index.js:450:22)
    at HiredisReplyParser.<anonymous> (/var/nodejs/lib/node_modules/socket.io/node_modules/redis/index.js:81:14)
    at HiredisReplyParser.emit (events.js:64:17)
    at HiredisReplyParser.execute (/var/nodejs/lib/node_modules/socket.io/node_modules/redis/lib/parser/hiredis.js:35:22)
    at RedisClient.on_data (/var/nodejs/lib/node_modules/socket.io/node_modules/redis/index.js:358:27)
    at Socket.<anonymous> (/var/nodejs/lib/node_modules/socket.io/node_modules/redis/index.js:93:14)
    at Socket.emit (events.js:64:17)
(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace: 
    at Redis.<anonymous> (events.js:126:17)
    at RedisClient.subscribe (/var/nodejs/lib/node_modules/socket.io/lib/stores/redis.js:118:14)
    at RedisClient.emit (events.js:81:20)
    at RedisClient.return_reply (/var/nodejs/lib/node_modules/socket.io/node_modules/redis/index.js:450:22)
    at HiredisReplyParser.<anonymous> (/var/nodejs/lib/node_modules/socket.io/node_modules/redis/index.js:81:14)
    at HiredisReplyParser.emit (events.js:64:17)
    at HiredisReplyParser.execute (/var/nodejs/lib/node_modules/socket.io/node_modules/redis/lib/parser/hiredis.js:35:22)
    at RedisClient.on_data (/var/nodejs/lib/node_modules/socket.io/node_modules/redis/index.js:358:27)
    at Socket.<anonymous> (/var/nodejs/lib/node_modules/socket.io/node_modules/redis/index.js:93:14)
    at Socket.emit (events.js:64:17)

This error seemed to have started when i switched from MemoryStore to RedisStore, Seems to happen periodontally, This may not be a memory leak but none the less should be fixed.

NPM Stats:

├─┬ socket.io@0.8.2 
│ ├── policyfile@0.0.4 
│ ├── redis@0.6.6 
│ └─┬ socket.io-client@0.8.2 
│   ├── uglify-js@1.0.6 
│   ├── websocket-client@1.0.0 
│   └── xmlhttprequest@1.2.2 
├─┬ socket.io-client@0.8.2 
│ ├── uglify-js@1.0.6 
│ ├── websocket-client@1.0.0 
│ └── xmlhttprequest@1.2.2

Redis server version 2.2.12 (00000000:0)

[dshaw]

Not an actual memory leak. We just establish a large number of event listeners with RedisStore. I have never found a good value for emitter.setMaxListeners that worked reliably here and I'm still hesitant to set it to unlimited (0).

Ok thanks for your reply and clearing that up for me, Just a few more questions.

1. Can I configure the max listeners, if so how.
2. Would this have an effect on clients not being able to connect if server reaches high load

Thanks.

[xaptronic]

Is there a work around to this problem? I tried setMaxListeners to 0 and 100 and both calls result in the node process being killed.

[dshaw]

First and foremost, it's just a notification. It will show up once when the app loads.

What are you setting? Your process is probably exiting because you're trying to set setMaxListeners(0) on the wrong thing that perhaps isn't even an instanceof EventEmitter.

[xaptronic]

Heh, I tried to set process.EventEmitter.prototype.setMaxListeners(0); just before I the call to listen in which I pass an options object with an instance of a RedisStore object. I haven't gotten that far into this code so I am assuming there is a better/working way to do this, as I've noticed that it actually does work despite this warning notice.

[dshaw]

Yes, you're touching Node inappropriately with that call. :)

The EventEmitter notification is just to help you be aware that you've attached a bunch of listeners. We know that.

[xaptronic]

Right, ok. I should be able to set the max number of listeners higher for this instance in order to supress that message though correct? I feel dirty leaving it there.

I applied the following changes https://gist.github.com/1268886 to get this message to go away. I sort of hacked stores/redis.js to get the anonymous function to set the value of the max listeners set by socket.io/lib/store.js ..

It seems that when the anonymous function is run by nodes event.js the context of this changes and no longer refers to the object in which this is called that has the correct value of maxListeners..

Not really sure what best way to solve this is.. perhaps making the call back function there a member of the object that is being worked it?

[xaptronic]

Attached an alternate version of redis.js where the maxListeners can be set on the RedisClient (this.sub) - that works too.

[dshaw]

@alex-pilon Thanks for taking the time to deep dive into this. Since this is specific to RedisStore, I hesitate to push any changes all the way up to Store. I'll review and test.

[xaptronic]

With the version of redis.js that sets the max listeners on the redisclient object (this.sub) no changes are needed to be made to Store (with the exception of setting this._events.. however that would simply be a work around needing to be made to account for an out of date version of events.js in node itself).

Looking at the comments for EventEmitter.setMaxListeners (https://github.com/joyent/node/blob/master/lib/events.js) it seems to me that it may actually be the correct place to put that since it isnt really up to the event emitter to know how many events will be attached to it but to whatever is attaching the events. Perhaps the value can be passed in via an option and then set.. however doing that would push the responsibility of knowing how many events are attached to user code which doesn't really make sense.

I think redis.js should be setting the maxlistener count on the RedisClient as it what is attaching all the events to the object which is a subclass of EventEmitter.

I'm interested to know what you come up with while testing.

[dshaw]

This seems to do the trick. dshaw@9a09c1f
Can you confirm?

[xaptronic]

I still get the warning with that change, however instead of two copies of it per connection I get one.

I'm using the version of socket.io from npm and the source code seems to be a bit different however looking at it, it seems to be doing the same thing except with some options injection.

[dshaw]

That doesn't include these changes. Use:
"socket.io" : "https://github.com/dshaw/socket.io/tarball/dev%2Fredis"
instead if
"socket.io" : "0.8.4"
in you package.json.

You might also want to delete and reinstall your node_modules folder, since they both identify as the same version. I'm not sure who wins in that case.