http server close() doesn't end http keep-alive connections

[kanongil]

When a http client uses Connection: keep-alive, http.Server#close() doesn't end stale/free keep-alive connections, thus preventing the process from exiting. I can replicate the behavior on both 0.10.x and master.

Failing test

server.js:

var fork = require('child_process').fork;
var http = require('http');

http.createServer(function(req, res) {
  this.close();
  res.end('test\n');
}).listen(8000, '127.0.0.1', function() {
  fork('./client.js');
});

process.on('exit', function(code) {
  console.log('server exit', code);
});

client.js:

var http = require('http');
var Agent = require('forever-agent');
var agent = new Agent();

http.get({ port: 8000, agent: agent }, function(res) {
  res.pipe(process.stdout);
  setTimeout(function() {
    console.log('timeout');
    agent.sockets['localhost:8000'].forEach(function(socket) {
      socket.destroy();
    })
  }, 5000);
});

process.on('exit', function(code) {
  console.log('client exit', code);
});

Result on node 0.10.35

$ NODE_DEBUG=net node server.js 
NET: 3110 listen2 127.0.0.1 8000 4 false
NET: 3110 _listen2: create a handle
NET: 3110 bind to 127.0.0.1
NET: 3111 connect: find host localhost
NET: 3110 onconnection
NET: 3111 afterConnect
NET: 3111 _read
NET: 3110 _read
NET: 3110 Socket._read readStart
NET: 3111 Socket._read readStart
NET: 3111 afterWrite 0 { domain: null, bytes: 64, oncomplete: [Function: afterWrite] }
NET: 3111 afterWrite call cb
NET: 3110 onread undefined 0 64 64
NET: 3110 got data
NET: 3110 SERVER _emitCloseIfDrained
NET: 3110 SERVER handle? false   connections? 1
NET: 3111 onread undefined 0 123 123
NET: 3111 got data
NET: 3110 afterWrite 0 { domain: null, bytes: 118, oncomplete: [Function: afterWrite] }
NET: 3110 afterWrite call cb
NET: 3110 afterWrite 0 { domain: null, bytes: 5, oncomplete: [Function: afterWrite] }
NET: 3110 afterWrite call cb
test
timeout
NET: 3111 destroy undefined
NET: 3111 destroy
NET: 3111 close
NET: 3111 close handle
NET: 3111 emit close
NET: 3110 onread EOF undefined undefined NaN
NET: 3110 EOF
client exit 0
NET: 3110 onSocketEnd { highWaterMark: 16384,
  buffer: [],
  length: 0,
  pipes: null,
  pipesCount: 0,
  flowing: false,
  ended: true,
  endEmitted: false,
  reading: false,
  calledRead: true,
  sync: false,
  needReadable: true,
  emittedReadable: false,
  readableListening: false,
  objectMode: false,
  defaultEncoding: 'utf8',
  ranOut: false,
  awaitDrain: 0,
  readingMore: false,
  decoder: null,
  encoding: null }
NET: 3110 onSocketFinish
NET: 3110 oSF: ended, destroy { highWaterMark: 16384,
  buffer: [],
  length: 0,
  pipes: null,
  pipesCount: 0,
  flowing: false,
  ended: true,
  endEmitted: true,
  reading: false,
  calledRead: true,
  sync: false,
  needReadable: false,
  emittedReadable: true,
  readableListening: false,
  objectMode: false,
  defaultEncoding: 'utf8',
  ranOut: false,
  awaitDrain: 0,
  readingMore: false,
  decoder: null,
  encoding: null }
NET: 3110 destroy undefined
NET: 3110 destroy
NET: 3110 close
NET: 3110 close handle
NET: 3110 has server
NET: 3110 SERVER _emitCloseIfDrained
NET: 3110 SERVER: emit close
NET: 3110 emit close
server exit 0

Expected result

Server exits immediately after the client has received the response data.

Observed result

Server is kept alive until the client destroys the http keep-alive socket. Similar behavior is observed in master when using this client script:

var http = require('http');
var agent = new http.Agent({ keepAlive:true });

http.get({ port: 8000, agent: agent }, function(res) {
  res.pipe(process.stdout);
  setTimeout(function() {
    console.log('timeout');
    agent.destroy();
  }, 5000);
});

process.on('exit', function(code) {
  console.log('client exit', code);
});

[micnic]

as it is written in the docs:

Stops the server from accepting new connections and keeps existing connections. This function is asynchronous, the server is finally closed when all connections are ended and the server emits a 'close' event.

http://nodejs.org/api/net.html#net_server_close_callback

this behavior is applied to net.Server and http.Server

[kanongil]

@micnic On a pure socket level the documented functionality holds true. However, on a semantic level it doesn't really. Logically, idle http keep-alive sockets should be closed when the server is closed. As it is, any client connected with http keep-alive can continue making new requests to the closed server indefinitely. Some might even call this a security issue.

[micnic]

@kanongil , if the keep-alive connections are idle you can set a timeout to close them, this is done by server.setTimeout() or response.setTimeout(), in my web framework, for example, I use a 5000 ms delay, which can be modified to any value.

[kanongil]

@micnic Using socket#setTimeout() on the server does not solve the underlying issue, it can merely hide it for common cases. A connected client will still be able to send new requests to the server indefinitely as long as it keeps sending requests before the timeout triggers.

[cepharum]

I'm stuck with code as simple as this running node 0.12.0:

var server = require( "http" ).createServer( function( req, res ) {
    res.end( "Hello World!" );
} );

server.listen( 3000 );

server.addListener( "connection", function( socket ) {
    socket.setTimeout( 0 );
} );

process.on( "SIGINT", function() {
    console.log( "got SIGINT" );
    server.close( function() {
        console.log( "closed" );
        process.exit( 0 );
    } );
} );

I tried running this from windows console (Windows 8.1). When pressing Ctrl+C w/o requesting on port 3000 the got SIGNT and closed are logged immediately. When pressing Ctrl+C after requesting at least once on port 3000 the got SIGINT is logged instantly, while the closed gets logged after 1-2 minutes.

I also tried

• server.timeout = 0; before starting to listen on port 3000 and
• res.setTimeout( 0 ); prior to sending response
  to no avail. I don't get it for this is the most simple case I can think.

What is intended way of having a nodejs-driven service application to be gracefully shut down (probably including to save state to disk after all client connections have been closed - as in my case) without any superdaemon considering service crashed/locked thus killing it no matter what data gets lost?

[CJex]

@cepharum Because timeout zero has a special meaning: https://nodejs.org/api/net.html#net_socket_settimeout_timeout_callback

By default the timeout is two minutes.

[CJex]

I can't believe the server.close() method is not designed to close server but only to stop accept new connection. Now how can I force shutdown the server? Use some npm packages like this : https://github.com/isaacs/server-destroy/blob/master/index.js ?

But it does make sense, even tasks simple like mkdir -p, you still need an npm package, why not this? Maybe someday http/https/net/fs packages will all be moved to NPM too. That's Node.js open source philosophy, isn't that? 😺

[dayuoba]

@JexCheng i think this is safer method for closing server,consider of if the server is serving for some clients.what will happen if server is forced down.

[kanongil]

@JexCheng @dayuoba I don't think that all existing connections should be closed, as server-destroy will do. The issue is that http keep-alive sockets can be idle, just waiting for another incoming request. This means that a client can continue to make new requests indefinitely.

[CJex]

@dayuoba
It's better to call this method stop, not close,or it's better to provide another destroy or close(force:Boolean) method, but it doesn't. For an analogy, I know force killing a process is not benevolent, but this doesn't imply that you should implement signal KILL same as TERM or QUIT.

So now how can I close the server immediately? No way. Oh, OK, I know, I need an NPM package.

[dayuoba]

var http = require('http');

var server = http.Server(function(req, res) {
    console.log('request got');
});

server.listen(8080);

setTimeout(function() {
    //exit if no other events in eventloop
    server.close();
    server.unref();
},5000);

@JexCheng

So now how can I close the server immediately?

how about process.exit(0)?

[CJex]

@dayuoba Hmm... I just found process.exit(0) could be used to break forEach too 😆 :

[1,3,5,23,6].forEach(function (x) {
  if (x % 2) console.log(x);
  else process.exit(0); // Just break forEach loop.
});

😆

[cepharum]

This all is missing some point. All web servers I know - Apache, nginx, cherokee - are supporting keep-alive connections. But none of them takes two minutes or more for shutting down if an administrator wants to stop it for whatever reason.

So, what is best practice for doing this in a node-base web application server. Definitely, process.exit() is not an option for it's instantly ending the process making it impossible to have shutdown handlers or similar properly releasing resources, writing back states to some persistent storage to be recovered on restarting or similar.

I know what keep-alive is good for and I'd appreciate to support it for sake of performance while the service is running. And I know why it's bad to drop all connections instantly. However, I'd love to see an easy option for lowering timeout of all currently active connections of a running server in case of I want to shut it down properly.

UPDATE: One option for achieving this is obviously given above by @micnic.

[josiasmontag]

I just published the approach I am using for this as a small npm module: https://github.com/josiasmontag/node-server-graceful-shutdown
Have a look at the source code. It uses server.close with the addition to set a socket idle timeout for all active connections, as soon as a shutdown is initiated. Using this keep alive works as normal and is only disabled if we want to shutdown the server.

[ygt-mikekchar]

I haven't looked at the node source code yet, but I'm pretty sure that the problem described by @cepharum is not to do with keep-alive connections. What's happening is that the server is set to stop accepting connections, but it is still listening. It will serve exactly 1 more connection, or it will timeout when the socket times out (and then actually close).

You can observe this behaviour yourself by calling close() on the server and then send it one more request. It will accept that request and then close immediately. If you don't send it a request it will close when the socket times out (2 minutes after the last request by default).

I think this is a bug in Node, but as I've said, I haven't looked at the code to convince myself that it isn't the desired behaviour (despite the documentation ;-) ).

[CJex]

Well, socket.setTimeout

socket.setTimeout(timeout[, callback])

Sets the socket to timeout after timeout milliseconds of inactivity on the socket.

Try this script:

var http = require('http');
var server = http.createServer(function (req,res) {
  req.setTimeout(3000);
  req.on('data',function (data) { console.log(data.toString()) });
});
server.setTimeout(6000);
server.listen(9900);

var req = http.request({
  method:'POST',
  hostname:'localhost',
  port:9900
});
req.write('Start\n');
setInterval(function () {
  req.write('Never close!\n');
},1000);

setTimeout(function () {
  server.close();
  console.log("Call close");
},2000);

[ygt-mikekchar]

I think the point is that the timeout is set to a reasonable value. It's just that when you call close() on the server it doesn't actually do what the documentation suggests (i.e. stop accepting connections). It accepts exactly one more connection.

I think the easiest workaround is simply sending an http request to the server after you have closed it. This will make it work. But it is slightly inelegant.

[JoshuaWise]

If you're closing the server as part of a graceful shutdown of the process, you just need this:

var server = require('http').createServer(myFancyServerLogic);

server.on('connection', function (socket) {socket.unref();});
server.listen(80);

function myFancyServerLogic(req, res) {
    req.connection.ref();

    res.end('Hello World!', function () {
        req.connection.unref();
    });
}

Basically, the sockets that your server uses will only keep the process alive while they're actually serving a request. While they're just sitting there idly (because of a Keep-Alive connection), a call to server.close() will close the process, as long as there's nothing else keeping the process alive. If you need to do other things after the server closes, as part of your graceful shutdown, you can hook into process.on('beforeExit', callback) to finish your graceful shutdown procedures.

[ronkorving]

@JoshuaWise Your solution is the only sane answer I see in this thread, thank you. That does not mean it shouldn't be the default behavior, because it really should be.

I'm sorry to say it this bluntly, but all these hacks and workarounds (that do or don't work) should not be needed. Calling close() should close the server and all clients that are not currently in the middle of a request. It should also close all keep-alive connections that are in the middle of a request immediately after the request has been served. This is not rocket science.

[JoshuaWise]

Thanks, @ronkorving. Unfortunately, My solution is useless if you have other things keeping the process open. This solution only works because Node will close when its event loop is empty. If you have other things going on, there is no way to know when all of your sockets connections are idle (without keeping track of every single one of them manually). I find this incredibly unfortunate.

EDIT
Perhaps this is adequate:

var http = require('http');
var EventEmitter = require('events').EventEmitter;
function createServer(requestListener) {
    var state = new EventEmitter;
    state.shutdown = false;

    var server = http.createServer();
    server.shutdown = function () {
        server.close();
        state.shutdown = true;
        state.emit('shutdown');
    }
    server.on('connection', function (socket) {
        function destroy() {
            if (socket.HAS_OPEN_REQUESTS === 0) socket.destroy();
        }
        socket.HAS_OPEN_REQUESTS = 0;
        state.once('shutdown', destroy);
        socket.once('close', function () {
            state.removeListener('shutdown', destroy);
        });
    });
    server.on('request', function (req, res) {
        var socket = req.connection;
        socket.HAS_OPEN_REQUESTS++;
        res.on('finish', function () {
            socket.HAS_OPEN_REQUESTS--;
            if (state.shutdown && socket.HAS_OPEN_REQUESTS === 0) {socket.destroy();}
        });
    });
    server.on('request', requestListener);
}
var server = createServer(myFancyServerLogic);
server.listen(80);

[jasnell]

This is largely due to a number of factors but primarily the fact that Node attempts to keep the internal state management as simple as possible without having to keep track of everything that would need to be closed. This is something that would be useful as a userland module, however :-). Going to close the issue as it's not actually a bug in node, but feel free to keep discussing :-)

[ronkorving]

If it's not a bug in node, what is it a bug in? You can't have this behavior and pretend to be an HTTP server, sorry. Just because it's hard to solve does not make this a bogus issue and I for one will not stick my head in the sand. Anyway, moving on (to io.js).

[Furchin]

Does io.js solve this problem?

[ronkorving]

I doubt it, but at least the odds of it getting fixed there are higher.

[codygustafson]

Just thought I would clarify that this is a bug in node.

The following will remain open as long as the client makes a request within one second intervals making it possible that a server will never gracefully close. With the default of 2 minute timeouts a user or api client would need to stop traffic for two minutes before this will take affect. I propose closing sockets using keep alive immediately after the next request. This would also prevent needing to keep track of everything that would need to be closed while making the maximum time to gracefully shutdown fixed(and finite).

var http = require('http');

var server = http.createServer(function(req, res) {
  res.end('test\n');
  server.close()
}).listen(8000, '127.0.0.1');
server.setTimeout(1000);

[ronkorving]

Sounds good to me, thanks 👍

[jasnell]

I agree this deserves another look. Reopening the issue here, however, is not likely to get the attention it would need. Opening a new issue under nodejs/node would be helpful

[codygustafson]

Created nodejs/node#2642

[hunterloftis]

As @jasnell suggested, this can be solved in userland: nodejs/node#2642 (comment)