sockjs / sockjs-node

WebSocket emulation - Node.js server
http://sockjs.org
MIT License
2.1k stars 309 forks source link

SockJS is not closing websockets #129

Open maxguzenski opened 11 years ago

maxguzenski commented 11 years ago

First, this issue is related to #127

SockJS is keeping websocket open (readyState == 1) even when the real connect not exists anymore for a long time (hours and days). And, sometimes, sockjs detect it and changes to readyState == 3 (closed) but NOT EMIT 'close' event (or it emit, but listeners did notice that).

I reproduced readyState == 3 bug at development side once, but I dont know how and I couldn't find a way to reproduce it again. I use chrome 27 at OSX

I solved both issue using a simple heart beat (I send a "ping" through websocket, not through a http get/post). On server side I just use a setInterval and close sockets that not sent heart beat for some long time.

I don't know if it a issue related with sockjs, nodejs, fayer or even chrome.

The point is, this issue is causing memory leak, and some applications (like my) needs to know when user leaves.

before: server memory grows from 100mb to 890mb into 2 days. now: server memory grows from 100mb to 182mb into 2 days.

HarryLyu commented 10 years ago

Hi @maxguzenski,

How do you close old sockets? Just .close()?

maxguzenski commented 10 years ago
var heatBeatTime = 1000*60*10; //10min

   setInterval(function() {
      var timeNow = new Date();
      var connIds = Object.keys(socketsList);

      connIds.forEach(function(connId) {
         var sl = socketsList[connId];

         if (!sl) {
            return true;
         }

         if (!sl.conn) {
            delete socketsList[connId];
         }

         //remove connection with long time without heatBeat
         if ((timeNow - sl.lastHeatBeat) > heatBeatTime) {
            sl.conn.close(55, 'no heartbeat');

            sl = null;
            delete socketsList[connId];

            return true;
         }
      });

      return true;

   }, heatBeatTime);
arjunmehta commented 10 years ago

Almost definitely related to #127.