From 7b23c644a633080e0e5ef93803a3b206767a850d Mon Sep 17 00:00:00 2001 From: Paolo Insogna Date: Mon, 3 Jul 2023 09:21:09 +0200 Subject: [PATCH] http: improve testing and little nits --- lib/_http_server.js | 26 +++++----- lib/https.js | 3 +- ...t-http-server-connections-checking-leak.js | 47 ++++++++++++------- 3 files changed, 47 insertions(+), 29 deletions(-) diff --git a/lib/_http_server.js b/lib/_http_server.js index b3115880796a0e1..ce3da95f177af6d 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -497,18 +497,14 @@ function storeHTTPOptions(options) { } } -function setupConnectionsTracking(server) { - if (!server) { - server = this; - } - +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) { @@ -546,7 +542,7 @@ function Server(options, requestListener) { this.httpAllowHalfOpen = false; this.on('connection', connectionListener); - this.on('listening', setupConnectionsTracking, this); + this.on('listening', setupConnectionsTracking); this.timeout = 0; this.maxHeadersCount = null; @@ -563,7 +559,11 @@ Server.prototype.close = function() { }; Server.prototype.closeAllConnections = function() { - const connections = this[kConnections]?.all() ?? []; + if (!this[kConnections]) { + return; + } + + const connections = this[kConnections].all(); for (let i = 0, l = connections.length; i < l; i++) { connections[i].socket.destroy(); @@ -571,7 +571,11 @@ Server.prototype.closeAllConnections = function() { }; Server.prototype.closeIdleConnections = function() { - const connections = this[kConnections]?.idle() ?? []; + if (!this[kConnections]) { + return; + } + + const connections = this[kConnections].idle(); for (let i = 0, l = connections.length; i < l; i++) { if (connections[i].socket._httpMessage && !connections[i].socket._httpMessage.finished) { diff --git a/lib/https.js b/lib/https.js index ca695555640a320..d2b5ee28bc52717 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 index fafb772e75870ad..71301edc84b636e 100644 --- a/test/parallel/test-http-server-connections-checking-leak.js +++ b/test/parallel/test-http-server-connections-checking-leak.js @@ -2,31 +2,44 @@ // Flags: --expose-gc -require('../common'); -const assert = require('assert'); +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; -const step = 10000; -const max = 100000; +// HTTP +{ + const gcListener = common.mustCall(max); -const usages = []; + for (let i = 0; i <= max; i++) { + if (i % 100 === 0) { + global.gc(); + } -for (let i = 0; i < max; i++) { - if (i % 100 === 0) { - global.gc(); + const server = http.createServer((req, res) => {}); + onGC(server, { ongc: gcListener }); } - if (i % step === 0) { - usages.push(process.memoryUsage()); - } - - http.createServer((req, res) => {}); + global.gc(); } -for (let i = 1; i < usages.length; i++) { - const current = usages[i].heapTotal; - const reference = usages[0].heapTotal; - assert.ok(current / reference < 1.1, `memory usage is increasing (${i}): ${current} > ${reference} * 1.1`); +// 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(); }