From 4fd8cd69c0f4e050c8d466b60f319640fa6398e5 Mon Sep 17 00:00:00 2001 From: linkgoron Date: Mon, 12 Jun 2023 10:47:25 +0300 Subject: [PATCH] https: fix connection checking interval not clearing on server close The connection interval should close when httpsServer.close is called similarly to how it gets cleared when httpServer.close is called. fixes: https://github.com/nodejs/node/issues/48373 PR-URL: https://github.com/nodejs/node/pull/48383 Reviewed-By: Moshe Atlow Reviewed-By: Paolo Insogna Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca Reviewed-By: Marco Ippolito Reviewed-By: Minwoo Jung --- lib/_http_server.js | 9 ++++++- lib/https.js | 7 ++++++ .../test-http-server-close-destroy-timeout.js | 13 ++++++++++ test/parallel/test-http-server-close-idle.js | 1 - ...test-https-server-close-destroy-timeout.js | 24 +++++++++++++++++++ test/parallel/test-https-server-close-idle.js | 1 - 6 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-http-server-close-destroy-timeout.js create mode 100644 test/parallel/test-https-server-close-destroy-timeout.js diff --git a/lib/_http_server.js b/lib/_http_server.js index 1d23ea0e86ab22..2d0ff8b7a64357 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -508,6 +508,11 @@ function setupConnectionsTracking(server) { setInterval(checkConnections.bind(server), server.connectionsCheckingInterval).unref(); } +function httpServerPreClose(server) { + server.closeIdleConnections(); + clearInterval(server[kConnectionsCheckingInterval]); +} + function Server(options, requestListener) { if (!(this instanceof Server)) return new Server(options, requestListener); @@ -549,7 +554,7 @@ ObjectSetPrototypeOf(Server.prototype, net.Server.prototype); ObjectSetPrototypeOf(Server, net.Server); Server.prototype.close = function() { - clearInterval(this[kConnectionsCheckingInterval]); + httpServerPreClose(this); ReflectApply(net.Server.prototype.close, this, arguments); }; @@ -1188,4 +1193,6 @@ module.exports = { storeHTTPOptions, _connectionListener: connectionListener, kServerResponse, + httpServerPreClose, + kConnectionsCheckingInterval, }; diff --git a/lib/https.js b/lib/https.js index 12453609f2c452..a6b7b971f44ee7 100644 --- a/lib/https.js +++ b/lib/https.js @@ -31,6 +31,7 @@ const { JSONStringify, ObjectAssign, ObjectSetPrototypeOf, + ReflectApply, ReflectConstruct, } = primordials; @@ -43,6 +44,7 @@ assertCrypto(); const tls = require('tls'); const { Agent: HttpAgent } = require('_http_agent'); const { + httpServerPreClose, Server: HttpServer, setupConnectionsTracking, storeHTTPOptions, @@ -97,6 +99,11 @@ Server.prototype.closeIdleConnections = HttpServer.prototype.closeIdleConnection Server.prototype.setTimeout = HttpServer.prototype.setTimeout; +Server.prototype.close = function() { + httpServerPreClose(this); + ReflectApply(tls.Server.prototype.close, this, arguments); +}; + /** * Creates a new `https.Server` instance. * @param {{ diff --git a/test/parallel/test-http-server-close-destroy-timeout.js b/test/parallel/test-http-server-close-destroy-timeout.js new file mode 100644 index 00000000000000..b1138ee36d5a90 --- /dev/null +++ b/test/parallel/test-http-server-close-destroy-timeout.js @@ -0,0 +1,13 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { createServer } = require('http'); +const { kConnectionsCheckingInterval } = require('_http_server'); + +const server = createServer(function(req, res) {}); +server.listen(0, common.mustCall(function() { + assert.strictEqual(server[kConnectionsCheckingInterval]._destroyed, false); + server.close(common.mustCall(() => { + assert(server[kConnectionsCheckingInterval]._destroyed); + })); +})); diff --git a/test/parallel/test-http-server-close-idle.js b/test/parallel/test-http-server-close-idle.js index 361ccf990fabcc..36e9752d36b528 100644 --- a/test/parallel/test-http-server-close-idle.js +++ b/test/parallel/test-http-server-close-idle.js @@ -42,7 +42,6 @@ server.listen(0, function() { assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive')); assert.strictEqual(connections, 2); - server.closeIdleConnections(); server.close(common.mustCall()); // Check that only the idle connection got closed diff --git a/test/parallel/test-https-server-close-destroy-timeout.js b/test/parallel/test-https-server-close-destroy-timeout.js new file mode 100644 index 00000000000000..e876721f610964 --- /dev/null +++ b/test/parallel/test-https-server-close-destroy-timeout.js @@ -0,0 +1,24 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +if (!common.hasCrypto) { + common.skip('missing crypto'); +} + +const { createServer } = require('https'); +const { kConnectionsCheckingInterval } = require('_http_server'); + +const fixtures = require('../common/fixtures'); + +const options = { + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem') +}; + +const server = createServer(options, function(req, res) {}); +server.listen(0, common.mustCall(function() { + assert.strictEqual(server[kConnectionsCheckingInterval]._destroyed, false); + server.close(common.mustCall(() => { + assert(server[kConnectionsCheckingInterval]._destroyed); + })); +})); diff --git a/test/parallel/test-https-server-close-idle.js b/test/parallel/test-https-server-close-idle.js index 7f093c47cd8609..49b525dd05f117 100644 --- a/test/parallel/test-https-server-close-idle.js +++ b/test/parallel/test-https-server-close-idle.js @@ -52,7 +52,6 @@ server.listen(0, function() { assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive')); assert.strictEqual(connections, 2); - server.closeIdleConnections(); server.close(common.mustCall()); // Check that only the idle connection got closed