I am new to javascript world. Recently I was working on a chat application in nodejs. So I have a method called gracefulshutdown as follows.

var gracefulShutdown = function() {
    logger.info("Received kill signal, shutting down gracefully.");
    server.close();
    logger.info('Disconnecting all the socket.io clients');
    if (Object.keys(io.sockets.sockets).length == 0) process.exit();
    var _map = io.sockets.sockets,
        _socket;
    for (var _k in _map) {
        if (_map.hasOwnProperty(_k)) {
            _socket = _map[_k];
            _socket.disconnect(true);
        }
    }
    ...code here...
    setTimeout(function() {
        logger.error("Could not close connections in time, shutting down");
        process.exit();
    }, 10 * 1000);
}

Here is what is happening in the disconnect listener.The removeDisconnectedClient method simply updates an entry in the db to indicate the removed client.

socket.on('disconnect', function() { removeDisconnectedClient(socket); });

So in this case the disconnect event wasn't fired for all sockets. It was fired for only a few sockets randomly from the array. Although I was able to fix it using setTimeout(fn, 0) with the help of a teammate.

I read about it online and understood only this much that setTimeout defers the execution of of code by adding it to end of event queue. I read about javascript context, call stack, event loop. But I couldn't put together all of it in this context. I really don't understand why and how this issue occurred. Could someone explain it in detail. And what is the best way to solve or avoid them.

I am new to javascript world. Recently I was working on a chat application in nodejs. So I have a method called gracefulshutdown as follows.

var gracefulShutdown = function() {
    logger.info("Received kill signal, shutting down gracefully.");
    server.close();
    logger.info('Disconnecting all the socket.io clients');
    if (Object.keys(io.sockets.sockets).length == 0) process.exit();
    var _map = io.sockets.sockets,
        _socket;
    for (var _k in _map) {
        if (_map.hasOwnProperty(_k)) {
            _socket = _map[_k];
            _socket.disconnect(true);
        }
    }
    ...code here...
    setTimeout(function() {
        logger.error("Could not close connections in time, shutting down");
        process.exit();
    }, 10 * 1000);
}

Here is what is happening in the disconnect listener.The removeDisconnectedClient method simply updates an entry in the db to indicate the removed client.

socket.on('disconnect', function() { removeDisconnectedClient(socket); });

So in this case the disconnect event wasn't fired for all sockets. It was fired for only a few sockets randomly from the array. Although I was able to fix it using setTimeout(fn, 0) with the help of a teammate.

I read about it online and understood only this much that setTimeout defers the execution of of code by adding it to end of event queue. I read about javascript context, call stack, event loop. But I couldn't put together all of it in this context. I really don't understand why and how this issue occurred. Could someone explain it in detail. And what is the best way to solve or avoid them.

• javascript
• node.js
• sockets

• Can you provide us more context around it to help u. Are you using socket.io? what are you expecting in emitResponse?? and is the return type for socket.emit() documented somewhere the way you're expecting it.? – xdevel Commented Jun 18, 2015 at 23:53
• Yes actually I am using socket.io library. I was expecting emitResponse to ensure that the emit was complete and it had send the message to client. I have edited the question to remove that part of code, as I went through the code to realize that socket.emit returns itself. – Narendra Rajput Commented Jun 19, 2015 at 4:25
• 1 You said "randomly from the array", is _map an array or an object? If it is an object it is okay, for...in is intended for use with objects, but if it is an array, for...in shouldn't be used for arrays (read more than just the top answer, the other answers have different important info too). – Useless Code Commented Jun 21, 2015 at 8:45
• @NarendraRajput socket.emit is an async function. To get result from remote you need not the return value of emit function but rather get in a callback function, provided as a last argument to emit – Kirill Slatin Commented Jun 21, 2015 at 10:13
• @NarendraRajput check update to my answer – Kirill Slatin Commented Jun 25, 2015 at 16:35

5 Answers 5

It is hard to say for sure without a little more context about the rest of the code in gracefulShutdown but I'm surprised it is disconnecting any of the sockets at all:

_socket = _map[ _k ];
socket.disconnect(true);

It appears that you are assigning an item from _map to the variable _socket but then calling disconnect on socket, which is a different variable. I'm guessing it is a typo and you meant to call disconnect on _socket?

Some of the sockets might be disconnecting for other reasons and the appearance that your loop is disconnecting some but not all the sockets is probably just coincidence.

As far as I can tell from the code you posted, socket should be undefined and you should be getting errors about trying to call the disconnect method on undefined.

From the method name where you use it I can suppose that application exits after attempts to disconnect all sockets. The nature of socket communication is asynchronous, so given you have a decent amount of items in _map it can occur that not all messages with disconnect will be sent before the process exits.

You can increase chances by calling exit after some timeout after disconnecting all sockets. However, why would you manually disconnect? On connection interruption remote sockets will automatically get disconnected...

UPDATE
Socket.io for Node.js doesn't have a callback to know for sure that packet with disconnect command was sent. At least in v0.9. I've debugged that and came to conclusion that without modification of sources it is not possible to catch that moment.

In file "socket.io\lib\transports\websocket\hybi-16.js" a method write is called to send the disconnect packet

WebSocket.prototype.write = function (data) {
...
   this.socket.write(buf, 'binary');
...
}

Whereas socket.write is defined in Node.js core transport "nodejs-{your-node-version}-src\core-modules-sources\lib\net.js" as

Socket.prototype.write = function(chunk, encoding, cb)
//cb is a callback to be called on writeRequest complete

However as you see this callback is not provided, so socket.io will not know about the packet having been sent.

At the same time when disconnect() is called for websocket, member disconnected is set to true, and "disconnect" event is broadcasted, indeed. But synchronously. So .on('disconnect' handler on server socket doesn't give and valuable information about whether the packet was sent or not.

Solution
I can make a general conclusion from this. If it is so critical to make sure that all clients are immediately informed (and not wait for a heartbeat timeout or if heartbeat is disabled) then this logic should be implemented manually.

You can send an ordinary message which will mean for the client that server is shutting down and call socket disconnect as soon as the message is received. At the same time server will be able to accept all acknowledgements

Server-side:

var sockets = [];
for (var _k in _map) {
    if (_map.hasOwnProperty(_k)) {
        sockets.push(_map[_k]);
    }
}
sockets.map(function (socket) {
    socket.emit('shutdown', function () {
        socket.isShutdown = true;
        var all = sockets.every(function (skt) {
            return skt.isShutdown;
        });
        if (all) {
            //wrap in timeout to let current tick finish before quitting
            setTimeout(function () {
                process.exit();
            });
        }
    })
})

Clients should behave simply

socket.on('shutdown', function () {
    socket.disconnect();
});

Thus we make sure each client has explicitly disconnected. We don't care about server. It will be shutdown shortly.

In the example code it looks like io.sockets.sockets is an Object, however, at least in the library version I am using, it is a mutable array which the socket.io library is free to modify each time you are removing a socket with disconnect(true).

Thus, when you call disconnect(true); if the currently iterated item from index i is removed, this effect like this happens:

var a = [1,2,3,4];
for( var i in a) {
   a.splice(i,1); // remove item from array
   alert(i);
}
// alerts 0,1 

Thus, the disconnect(true) call will ask the socket.io to remove the item from the array - and because you are both holding reference to the same array, the contents of the array are modified during the loop.

The solution is to create a copy of the _map with slice() before the loop:

var _map = io.sockets.sockets.slice(); // copy of the original

It would create a copy of the original array and thus should go through all the items in the array.

The reason why calling setTimeout() would also work is that it would defer the removal of the items from the array, allowing the whole loop iterate without modifying the sockets -Array.

The problem here is that sockjs and socket.io use asynchronous "disconnect" methods. IE. When you call disconnect, it is not immediately terminated. It is just a promise that it WILL be terminated. This has the following effect (assuming 3 sockets)

1. Your for loop grabs the first socket
2. The disconnect method is called on the first socket
3. Your for loop grabs the second socket
4. The disconnect method is called on the second socket
5. The disconnect method on the first socket finishes
6. Your for loop grabs the third socket
7. The disconnect method is called on the third socket
8. Program kills itself

Notice, that sockets 2 and 3 haven't necessarily finished yet. This could be for a number of reasons.

Finally, setTimeout(fn, 0) is, as you said, blocking the final call, but it may not be consistent (I haven't dug into this too much). By that I mean, you've set the final termination to be AFTER all your sockets have disconnected. The setTimeout and setInterval methods essentially act more like a queue. Your position in the queue is dictated by the timer you set. Two intervals set for 10s each, where they both run synchronously will cause one to run AFTER the other.

After Socket.io 1.0, the library does not expose you an array of the connected sockets. You can check that io.socket.sockets.length, is not equal to the open socket objects. Your best bet is that you broadcast a 'disconnect' message to all the clients that you want to off, and on.'disconnect' on the client side close the actual WebSocket.