From c30ef3cbd2e42ac1d600f6bd78a601a5496b0877 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Fri, 3 May 2019 16:45:57 -0700 Subject: [PATCH] http, http2: remove default server timeout Timing out and closing the socket after two minutes have elapsed is surprising and problematic for users. This behavior was specific to Node.js, and doesn't seem to be common in other language runtimes. Fixes: https://github.com/nodejs/node/issues/27556 PR-URL: https://github.com/nodejs/node/pull/27558 Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen Reviewed-By: Rich Trott --- doc/api/http.md | 18 +++++++++++++----- doc/api/http2.md | 12 ++++++++++-- lib/_http_server.js | 2 +- lib/https.js | 2 +- lib/internal/http2/core.js | 6 ++---- test/async-hooks/test-graph.http.js | 3 --- .../test-child-process-http-socket-leak.js | 2 +- 7 files changed, 28 insertions(+), 17 deletions(-) diff --git a/doc/api/http.md b/doc/api/http.md index ccd5e4b0bd4..0588d7f408f 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -1013,9 +1013,13 @@ Limits maximum incoming headers count. If set to 0, no limit will be applied. ### server.setTimeout([msecs][, callback]) -* `msecs` {number} **Default:** `120000` (2 minutes) +* `msecs` {number} **Default:** 0 (no timeout) * `callback` {Function} * Returns: {http.Server} @@ -1026,16 +1030,20 @@ occurs. If there is a `'timeout'` event listener on the Server object, then it will be called with the timed-out socket as an argument. -By default, the Server's timeout value is 2 minutes, and sockets are -destroyed automatically if they time out. However, if a callback is assigned -to the Server's `'timeout'` event, timeouts must be handled explicitly. +By default, the Server does not timeout sockets. However, if a callback +is assigned to the Server's `'timeout'` event, timeouts must be handled +explicitly. ### server.timeout -* {number} Timeout in milliseconds. **Default:** `120000` (2 minutes). +* {number} Timeout in milliseconds. **Default:** 0 (no timeout) The number of milliseconds of inactivity before a socket is presumed to have timed out. diff --git a/doc/api/http2.md b/doc/api/http2.md index 1416c976362..5ba4943a4d9 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1722,11 +1722,15 @@ server.on('stream', (stream, headers, flags) => { #### Event: 'timeout' The `'timeout'` event is emitted when there is no activity on the Server for a given number of milliseconds set using `http2server.setTimeout()`. -**Default:** 2 minutes. +**Default:** 0 (no timeout) #### server.close([callback]) -* `msecs` {number} **Default:** `120000` (2 minutes) +* `msecs` {number} **Default:** 0 (no timeout) * `callback` {Function} * Returns: {Http2Server} diff --git a/lib/_http_server.js b/lib/_http_server.js index 9f62872e8ec..f7884d2626f 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -315,7 +315,7 @@ function Server(options, requestListener) { this.on('connection', connectionListener); - this.timeout = 2 * 60 * 1000; + this.timeout = 0; this.keepAliveTimeout = 5000; this.maxHeadersCount = null; this.headersTimeout = 40 * 1000; // 40 seconds diff --git a/lib/https.js b/lib/https.js index 4e649017312..e1fc91fd966 100644 --- a/lib/https.js +++ b/lib/https.js @@ -71,7 +71,7 @@ function Server(opts, requestListener) { conn.destroy(err); }); - this.timeout = 2 * 60 * 1000; + this.timeout = 0; this.keepAliveTimeout = 5000; this.maxHeadersCount = null; this.headersTimeout = 40 * 1000; // 40 seconds diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 58193b41cd3..6ad1668c4f5 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -171,8 +171,6 @@ const kState = Symbol('state'); const kType = Symbol('type'); const kWriteGeneric = Symbol('write-generic'); -const kDefaultSocketTimeout = 2 * 60 * 1000; - const { paddingBuffer, PADDING_BUF_FRAME_LENGTH, @@ -2680,7 +2678,7 @@ class Http2SecureServer extends TLSServer { options = initializeTLSOptions(options); super(options, connectionListener); this[kOptions] = options; - this.timeout = kDefaultSocketTimeout; + this.timeout = 0; this.on('newListener', setupCompat); if (typeof requestListener === 'function') this.on('request', requestListener); @@ -2702,7 +2700,7 @@ class Http2Server extends NETServer { constructor(options, requestListener) { super(connectionListener); this[kOptions] = initializeOptions(options); - this.timeout = kDefaultSocketTimeout; + this.timeout = 0; this.on('newListener', setupCompat); if (typeof requestListener === 'function') this.on('request', requestListener); diff --git a/test/async-hooks/test-graph.http.js b/test/async-hooks/test-graph.http.js index 55b9b055a0f..12862467a69 100644 --- a/test/async-hooks/test-graph.http.js +++ b/test/async-hooks/test-graph.http.js @@ -43,9 +43,6 @@ process.on('exit', function() { { type: 'HTTPINCOMINGMESSAGE', id: 'httpincomingmessage:1', triggerAsyncId: 'tcp:2' }, - { type: 'Timeout', - id: 'timeout:2', - triggerAsyncId: 'tcp:2' }, { type: 'SHUTDOWNWRAP', id: 'shutdown:1', triggerAsyncId: 'tcp:2' } ] diff --git a/test/parallel/test-child-process-http-socket-leak.js b/test/parallel/test-child-process-http-socket-leak.js index 553a3277532..9b284f285c4 100644 --- a/test/parallel/test-child-process-http-socket-leak.js +++ b/test/parallel/test-child-process-http-socket-leak.js @@ -46,7 +46,7 @@ server.listen(0, common.mustCall(() => { }, common.mustCall((res) => { res.on('data', () => {}); res.on('end', common.mustCall(() => { - assert.strictEqual(socket[kTimeout]._idleTimeout, -1); + assert.strictEqual(socket[kTimeout], null); assert.strictEqual(socket.parser, null); assert.strictEqual(socket._httpMessage, null); }));