http: improve testing and little nits

nodejs · Jul 3, 2023 · 7b23c64 · 7b23c64
1 parent e5c4298
commit 7b23c64
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 29 deletions.
diff --git 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,15 +559,23 @@ 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();
   }
 };
 
 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
@@ -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
@@ -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();
 }