From 94b4764b5027e8214327db1addd4f623b94414af Mon Sep 17 00:00:00 2001 From: Paolo Insogna Date: Fri, 30 Jun 2023 16:14:44 +0200 Subject: [PATCH] http: start connections checking interval on listen Fixes: https://github.com/nodejs/node/issues/48604. --- lib/_http_server.js | 19 +++++--- lib/https.js | 3 +- ...t-http-server-connections-checking-leak.js | 45 +++++++++++++++++++ 3 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-http-server-connections-checking-leak.js diff --git a/lib/_http_server.js b/lib/_http_server.js index d72faed56e6056..ce3da95f177af6 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -497,14 +497,14 @@ function storeHTTPOptions(options) { } } -function setupConnectionsTracking(server) { +function setupConnectionsTracking() { // Start connection handling - server[kConnections] = new ConnectionsList(); + this[kConnections] = new ConnectionsList(); // This checker is started without checking whether any headersTimeout or requestTimeout is non zero // otherwise it would not be started if such timeouts are modified after createServer. - server[kConnectionsCheckingInterval] = - setInterval(checkConnections.bind(server), server.connectionsCheckingInterval).unref(); + this[kConnectionsCheckingInterval] = + setInterval(checkConnections.bind(this), this.connectionsCheckingInterval).unref(); } function httpServerPreClose(server) { @@ -542,11 +542,12 @@ function Server(options, requestListener) { this.httpAllowHalfOpen = false; this.on('connection', connectionListener); + this.on('listening', setupConnectionsTracking); this.timeout = 0; this.maxHeadersCount = null; this.maxRequestsPerSocket = 0; - setupConnectionsTracking(this); + this[kUniqueHeaders] = parseUniqueHeadersOption(options.uniqueHeaders); } ObjectSetPrototypeOf(Server.prototype, net.Server.prototype); @@ -558,6 +559,10 @@ Server.prototype.close = function() { }; Server.prototype.closeAllConnections = function() { + if (!this[kConnections]) { + return; + } + const connections = this[kConnections].all(); for (let i = 0, l = connections.length; i < l; i++) { @@ -566,6 +571,10 @@ Server.prototype.closeAllConnections = function() { }; Server.prototype.closeIdleConnections = function() { + if (!this[kConnections]) { + return; + } + const connections = this[kConnections].idle(); for (let i = 0, l = connections.length; i < l; i++) { diff --git a/lib/https.js b/lib/https.js index ca695555640a32..d2b5ee28bc5271 100644 --- a/lib/https.js +++ b/lib/https.js @@ -94,8 +94,9 @@ function Server(opts, requestListener) { this.timeout = 0; this.maxHeadersCount = null; - setupConnectionsTracking(this); + this.on('listening', setupConnectionsTracking); } + ObjectSetPrototypeOf(Server.prototype, tls.Server.prototype); ObjectSetPrototypeOf(Server, tls.Server); diff --git a/test/parallel/test-http-server-connections-checking-leak.js b/test/parallel/test-http-server-connections-checking-leak.js new file mode 100644 index 00000000000000..71301edc84b636 --- /dev/null +++ b/test/parallel/test-http-server-connections-checking-leak.js @@ -0,0 +1,45 @@ +'use strict'; + +// Flags: --expose-gc + +const common = require('../common'); +if (!common.hasCrypto) { common.skip('missing crypto'); } +const onGC = require('../common/ongc'); +const http = require('http'); +const https = require('https'); + +// Check that creating a server without listening is not leaking resources. +const max = 100; + +// HTTP +{ + const gcListener = common.mustCall(max); + + for (let i = 0; i <= max; i++) { + if (i % 100 === 0) { + global.gc(); + } + + const server = http.createServer((req, res) => {}); + onGC(server, { ongc: gcListener }); + } + + global.gc(); +} + +// HTTPS + +{ + const gcListener = common.mustCall(max); + + for (let i = 0; i <= max; i++) { + if (i % 100 === 0) { + global.gc(); + } + + const server = https.createServer((req, res) => {}); + onGC(server, { ongc: gcListener }); + } + + global.gc(); +}