Skip to content

Commit

Permalink
http: destroy sockets after keepAliveTimeout
Browse files Browse the repository at this point in the history
Implement server.keepAliveTimeout in addition to server.timeout to
prevent temporary socket/memory leaking in keep-alive mode.

PR-URL: #2534
Author: Timur Shemsedinov <[email protected]>
Author: Alexey Orlenko <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
  • Loading branch information
tshemsedinov authored and jasnell committed May 28, 2017
1 parent bb77cce commit a7f5c9c
Show file tree
Hide file tree
Showing 6 changed files with 252 additions and 11 deletions.
29 changes: 23 additions & 6 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -832,17 +832,33 @@ Returns `server`.
added: v0.9.12
-->

* {number} Defaults to 120000 (2 minutes).
* {number} Timeout in milliseconds. Defaults to 120000 (2 minutes).

The number of milliseconds of inactivity before a socket is presumed
to have timed out.

Note that the socket timeout logic is set up on connection, so
changing this value only affects *new* connections to the server, not
any existing connections.
A value of 0 will disable the timeout behavior on incoming connections.

Set to 0 to disable any kind of automatic timeout behavior on incoming
connections.
*Note*: The socket timeout logic is set up on connection, so changing this
value only affects new connections to the server, not any existing connections.

### server.keepAliveTimeout
<!-- YAML
added: REPLACEME
-->

* {number} Timeout in milliseconds. Defaults to 5000 (5 seconds).

The number of milliseconds of inactivity a server needs to wait for additional
incoming data, after it has finished writing the last response, before a socket
will be destroyed. If the server receives new data before the keep-alive
timeout has fired, it will reset the regular inactivity timeout, i.e.,
[`server.timeout`][].

A value of 0 will disable the keep-alive timeout behavior on incoming connections.

*Note*: The socket timeout logic is set up on connection, so changing this
value only affects new connections to the server, not any existing connections.

## Class: http.ServerResponse
<!-- YAML
Expand Down Expand Up @@ -1764,6 +1780,7 @@ There are a few special headers that should be noted.
[`response.write(data, encoding)`]: #http_response_write_chunk_encoding_callback
[`response.writeContinue()`]: #http_response_writecontinue
[`response.writeHead()`]: #http_response_writehead_statuscode_statusmessage_headers
[`server.timeout`]: #http_server_timeout
[`socket.setKeepAlive()`]: net.html#net_socket_setkeepalive_enable_initialdelay
[`socket.setNoDelay()`]: net.html#net_socket_setnodelay_nodelay
[`socket.setTimeout()`]: net.html#net_socket_settimeout_timeout_callback
Expand Down
9 changes: 9 additions & 0 deletions doc/api/https.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ added: v0.11.2

See [`http.Server#timeout`][].

### server.keepAliveTimeout
<!-- YAML
added: REPLACEME
-->
- {number} Defaults to 5000 (5 seconds).

See [`http.Server#keepAliveTimeout`][].

## https.createServer(options[, requestListener])
<!-- YAML
added: v0.3.4
Expand Down Expand Up @@ -229,6 +237,7 @@ const req = https.request(options, (res) => {

[`Agent`]: #https_class_https_agent
[`http.Agent`]: http.html#http_class_http_agent
[`http.Server#keepAliveTimeout`]: http.html#http_server_keepalivetimeout
[`http.Server#setTimeout()`]: http.html#http_server_settimeout_msecs_callback
[`http.Server#timeout`]: http.html#http_server_timeout
[`http.Server`]: http.html#http_class_http_server
Expand Down
26 changes: 21 additions & 5 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ function Server(requestListener) {
this.on('connection', connectionListener);

this.timeout = 2 * 60 * 1000;

this.keepAliveTimeout = 5000;
this._pendingResponseData = 0;
this.maxHeadersCount = null;
}
Expand Down Expand Up @@ -323,7 +323,8 @@ function connectionListener(socket) {
// inactive responses. If more data than the high watermark is queued - we
// need to pause TCP socket/HTTP parser, and wait until the data will be
// sent to the client.
outgoingData: 0
outgoingData: 0,
keepAliveTimeoutSet: false
};
state.onData = socketOnData.bind(undefined, this, socket, parser, state);
state.onEnd = socketOnEnd.bind(undefined, this, socket, parser, state);
Expand Down Expand Up @@ -431,8 +432,16 @@ function socketOnEnd(server, socket, parser, state) {
function socketOnData(server, socket, parser, state, d) {
assert(!socket._paused);
debug('SERVER socketOnData %d', d.length);
var ret = parser.execute(d);

if (state.keepAliveTimeoutSet) {
socket.setTimeout(0);
if (server.timeout) {
socket.setTimeout(server.timeout);
}
state.keepAliveTimeoutSet = false;
}

var ret = parser.execute(d);
onParserExecuteCommon(server, socket, parser, state, ret, d);
}

Expand Down Expand Up @@ -496,7 +505,7 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
}
}

function resOnFinish(req, res, socket, state) {
function resOnFinish(req, res, socket, state, server) {
// Usually the first incoming element should be our request. it may
// be that in the case abortIncoming() was called that the incoming
// array will be empty.
Expand All @@ -514,6 +523,12 @@ function resOnFinish(req, res, socket, state) {

if (res._last) {
socket.destroySoon();
} else if (state.outgoing.length === 0) {
if (server.keepAliveTimeout) {
socket.setTimeout(0);
socket.setTimeout(server.keepAliveTimeout);
state.keepAliveTimeoutSet = true;
}
} else {
// start sending the next message
var m = state.outgoing.shift();
Expand Down Expand Up @@ -560,7 +575,8 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {

// When we're finished writing the response, check if this is the last
// response, if so destroy the socket.
res.on('finish', resOnFinish.bind(undefined, req, res, socket, state));
res.on('finish',
resOnFinish.bind(undefined, req, res, socket, state, server));

if (req.headers.expect !== undefined &&
(req.httpVersionMajor === 1 && req.httpVersionMinor === 1)) {
Expand Down
1 change: 1 addition & 0 deletions lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ function Server(opts, requestListener) {
});

this.timeout = 2 * 60 * 1000;
this.keepAliveTimeout = 5000;
}
inherits(Server, tls.Server);
exports.Server = Server;
Expand Down
95 changes: 95 additions & 0 deletions test/parallel/test-http-server-keep-alive-timeout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const http = require('http');
const net = require('net');

const tests = [];

function test(fn) {
if (!tests.length) {
process.nextTick(run);
}
tests.push(fn);
}

function run() {
const fn = tests.shift();
if (fn) fn(run);
}

test(function serverEndKeepAliveTimeoutWithPipeline(cb) {
let socket;
let destroyedSockets = 0;
let timeoutCount = 0;
let requestCount = 0;
process.on('exit', () => {
assert.strictEqual(timeoutCount, 1);
assert.strictEqual(requestCount, 3);
assert.strictEqual(destroyedSockets, 1);
});
const server = http.createServer((req, res) => {
socket = req.socket;
requestCount++;
res.end();
});
server.setTimeout(200, (socket) => {
timeoutCount++;
socket.destroy();
});
server.keepAliveTimeout = 50;
server.listen(0, common.mustCall(() => {
const options = {
port: server.address().port,
allowHalfOpen: true
};
const c = net.connect(options, () => {
c.write('GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n');
c.write('GET /2 HTTP/1.1\r\nHost: localhost\r\n\r\n');
c.write('GET /3 HTTP/1.1\r\nHost: localhost\r\n\r\n');
});
setTimeout(() => {
server.close();
if (socket.destroyed) destroyedSockets++;
cb();
}, 1000);
}));
});

test(function serverNoEndKeepAliveTimeoutWithPipeline(cb) {
let socket;
let destroyedSockets = 0;
let timeoutCount = 0;
let requestCount = 0;
process.on('exit', () => {
assert.strictEqual(timeoutCount, 1);
assert.strictEqual(requestCount, 3);
assert.strictEqual(destroyedSockets, 1);
});
const server = http.createServer((req, res) => {
socket = req.socket;
requestCount++;
});
server.setTimeout(200, (socket) => {
timeoutCount++;
socket.destroy();
});
server.keepAliveTimeout = 50;
server.listen(0, common.mustCall(() => {
const options = {
port: server.address().port,
allowHalfOpen: true
};
const c = net.connect(options, () => {
c.write('GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n');
c.write('GET /2 HTTP/1.1\r\nHost: localhost\r\n\r\n');
c.write('GET /3 HTTP/1.1\r\nHost: localhost\r\n\r\n');
});
setTimeout(() => {
server.close();
if (socket.destroyed) destroyedSockets++;
cb();
}, 1000);
}));
});
103 changes: 103 additions & 0 deletions test/parallel/test-https-server-keep-alive-timeout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const https = require('https');
const tls = require('tls');
const fs = require('fs');

const tests = [];

const serverOptions = {
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
};

function test(fn) {
if (!tests.length) {
process.nextTick(run);
}
tests.push(fn);
}

function run() {
const fn = tests.shift();
if (fn) fn(run);
}

test(function serverKeepAliveTimeoutWithPipeline(cb) {
let socket;
let destroyedSockets = 0;
let timeoutCount = 0;
let requestCount = 0;
process.on('exit', function() {
assert.strictEqual(timeoutCount, 1);
assert.strictEqual(requestCount, 3);
assert.strictEqual(destroyedSockets, 1);
});
const server = https.createServer(serverOptions, (req, res) => {
socket = req.socket;
requestCount++;
res.end();
});
server.setTimeout(200, (socket) => {
timeoutCount++;
socket.destroy();
});
server.keepAliveTimeout = 50;
server.listen(0, common.mustCall(() => {
const options = {
port: server.address().port,
allowHalfOpen: true,
rejectUnauthorized: false
};
const c = tls.connect(options, () => {
c.write('GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n');
c.write('GET /2 HTTP/1.1\r\nHost: localhost\r\n\r\n');
c.write('GET /3 HTTP/1.1\r\nHost: localhost\r\n\r\n');
});
setTimeout(() => {
server.close();
if (socket.destroyed) destroyedSockets++;
cb();
}, 1000);
}));
});

test(function serverNoEndKeepAliveTimeoutWithPipeline(cb) {
let socket;
let destroyedSockets = 0;
let timeoutCount = 0;
let requestCount = 0;
process.on('exit', () => {
assert.strictEqual(timeoutCount, 1);
assert.strictEqual(requestCount, 3);
assert.strictEqual(destroyedSockets, 1);
});
const server = https.createServer(serverOptions, (req, res) => {
socket = req.socket;
requestCount++;
});
server.setTimeout(200, (socket) => {
timeoutCount++;
socket.destroy();
});
server.keepAliveTimeout = 50;
server.listen(0, common.mustCall(() => {
const options = {
port: server.address().port,
allowHalfOpen: true,
rejectUnauthorized: false
};
const c = tls.connect(options, () => {
c.write('GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n');
c.write('GET /2 HTTP/1.1\r\nHost: localhost\r\n\r\n');
c.write('GET /3 HTTP/1.1\r\nHost: localhost\r\n\r\n');
});
setTimeout(() => {
server.close();
if (socket && socket.destroyed) destroyedSockets++;
cb();
}, 1000);
}));
});

0 comments on commit a7f5c9c

Please sign in to comment.