Memory leak when using RedisStore

[gkostov]

Setup

node 0.10.12
socket.io 0.9.16
Running just one instance of the socket.io server.

Symptom

The redisSub object accumulates listeners for the "message" event and keys in the subscription_set map for each connected client and removes only some of those when the client disconnects.

Reproduce

The leak is very easy to spot using this code:

// Run the server
var ioPort=8000,
    io = require('socket.io').listen(ioPort);

var RedisStore = require('socket.io/lib/stores/redis')
  , redis  = require('socket.io/node_modules/redis')
  , pub    = redis.createClient()
  , sub    = redis.createClient()
  , client = redis.createClient();

io.set('store', new RedisStore({
  redisPub : pub
, redisSub : sub
, redisClient : client
}));

function log(){
    if(io.settings.store.sub._events.message)
        console.log('message listeners: '+io.settings.store.sub._events.message.length);
    console.log('subscription_set keys: '+Object.keys(io.settings.store.sub.subscription_set).length);
}
setInterval(log, 5000);

// and run the client
function connect(){
    log();
    var socket = require('socket.io-client').connect('localhost', {
            port: ioPort,
            'force new connection': true
        });
    socket.on('connect', function (){
        setTimeout(function (){
            socket.disconnect();
            setTimeout(connect, 1000);
        }, Math.round(Math.random()*2000)+10000);
    });
    socket.on('error', function () {
        console.log('\nsocket.io connection error');
    });
};
setTimeout(connect, 1000);

It runs the socket.io server with a RedisStore. A client is set to connect, stay connected for awhile, disconnect, then wait for one second and the cycle repeats.
In the console you'll get logs for the length of io.settings.store.sub._events.message and the number of keys in io.settings.store.sub.subscription_set.
When a client connects both grow by 5. On disconnect - "message" listeners decrease by 2 (3 remain) and subscription_set looses 3 keys (2 remain which are always like sub message:jpUygh7mf-tN1A-xulsB': true and
sub disconnect:jpUygh7mf-tN1A-xulsB': true).

I hope this will be helpfull.

[gkostov]

Ok, after digging into the code it appears that these are very old and known issues and there have even been attempts to fix them but then the fixes have been quickly reverted (for a good reason I hope).
In summary these are the same as pointed in #663 and #1064

[toblerpwn]

@gkostov - are you using this in production? any luck? we (as many others) are still seeing this issue, but unfortunately reaching the limits of using MemoryStore (or, more accurately, the limits of non-Clustered Node.js).

[gkostov]

I have managed to detect three leaks and the above commit only fixes two of them. With this update the server appears in much better shape now. It still leaks of course but survived for few days at stable load with no restarts.
Unfortunately there's something in our environment which occasionally triggers huge and stable memory leaks. I hope to discover what that is but it definitely has to deal with the Redis store because we have two instances in the cluster not registered with the load balancer. And those two leak just like every other even though they get no client connections. Apparently it's some synchronization issue where a node tells the others about some event but then later that doesn't get discarded when the event is no longer relevant (client has gone or something like that).
In fact I'm looking to get some time to dig into engine.io and to write a Redis store for it. I do not need rooms or anything from Socket.io other than a reliable message transport. The only thing that keeps me with it is the RedisStore which apparently tries to do much more than it can.

[zhaoerlei]

I think. I fix this problom. two problom exist.

1. subscribe not be unsubscribe.
   manager.js line 548. local not exist. close this logic.

   // if (local) { // comment this logic
   this.store.unsubscribe('message:' + id);
   this.store.unsubscribe('disconnect:' + id);
   // }

2. not publish "disconnect" when client disconnected.
   transport.js about line 459

   if (local) {
   this.manager.onClientDisconnect(this.id, reason, true);
   } else {
   this.store.publish('disconnect:' + this.id, reason);
   }
   //zhaoerlei add this
   this.store.publish('disconnect' , this.id);
   }
   I test it. memory not increase.

妈的 这个狗屎问题learnboost为什么不改 害的老子增加了一个月的工作量

[toblerpwn]

After a LOOOOOT of testing in our production environment (2,000-4,000 concurrent sockets blasting FREQUENT messages), #1371 has resulted in the best solution in our environment. Highly recommend checking it out and/or testing it if you have a high-ish volume application.

(We also tried #1260, which I believe attempts to solve both the problems that @zhaoerlei notes, but we still found some leaking.)

[freeman983]

I use #1260,but still leak

[toblerpwn]

@freeman983 try #1371 - it worked for us!

[freeman983]

ofpay@8c823d8

surespot/socket.io#channelfix --- add memory leak fix

[gorangajic]

@freeman983 did you test that change?

We are using surespot/socket.io#channelfix and it's much better but not perfect

[freeman983]

@Feroc1ty ,I do test for this change ,please see

https://www.evernote.com/shard/s44/sh/1f7c41f9-6563-4c24-b1b5-25e092ee5770/ff9c6a212ef601718e01128fe7ed333b

[gorangajic]

thanks I did the testing also and it's great :)
surespot/socket.io#surespot + @freeman983 commit ofpay@8c823d8
fix our socket.io memory leak

http://i.imgur.com/b9vdL93.png

[zhaoerlei]

@freeman983
sorry, I have mistake. transport.js 459, no 359

/**

• Cleans up the connection, considers the client disconnected.
  *
• @api private
  */

Transport.prototype.end = function (reason) {
if (!this.disconnected) {
this.log.info('transport end (' + reason + ')');

var local = this.manager.transports[this.id];

this.close();
this.clearTimeouts();
this.disconnected = true;

if (local) {
  this.manager.onClientDisconnect(this.id, reason, true);
} else {
  this.store.publish('disconnect:' + this.id, reason);
}
//zhaoerlei add this
this.store.publish('disconnect' , this.id);

}
};