From b46a0f0bc85ab88f345163167038344722f5807a Mon Sep 17 00:00:00 2001
From: Sergey Chernyshev <schernyshev@openjdk.org>
Date: Thu, 25 Apr 2024 04:19:24 +0200
Subject: [PATCH] http: start connections checking interval on listen

Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/48611
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
---
 graal-nodejs/lib/_http_server.js              | 21 ++++++++++----
 graal-nodejs/lib/https.js                     |  3 +-
 ...t-http-server-connections-checking-leak.js | 24 +++++++++++++++
 ...-https-server-connections-checking-leak.js | 29 +++++++++++++++++++
 4 files changed, 71 insertions(+), 6 deletions(-)
 create mode 100644 graal-nodejs/test/parallel/test-http-server-connections-checking-leak.js
 create mode 100644 graal-nodejs/test/parallel/test-https-server-connections-checking-leak.js

diff --git a/graal-nodejs/lib/_http_server.js b/graal-nodejs/lib/_http_server.js
index c010d8fa96a..87a91eafd0d 100644
--- a/graal-nodejs/lib/_http_server.js
+++ b/graal-nodejs/lib/_http_server.js
@@ -493,14 +493,16 @@ function storeHTTPOptions(options) {
   }
 }
 
-function setupConnectionsTracking(server) {
+function setupConnectionsTracking() {
   // Start connection handling
-  server[kConnections] = new ConnectionsList();
+  if (!this[kConnections]) {
+    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 Server(options, requestListener) {
@@ -533,11 +535,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);
@@ -549,6 +552,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++) {
@@ -557,6 +564,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/graal-nodejs/lib/https.js b/graal-nodejs/lib/https.js
index 12453609f2c..443cb694d61 100644
--- a/graal-nodejs/lib/https.js
+++ b/graal-nodejs/lib/https.js
@@ -86,8 +86,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/graal-nodejs/test/parallel/test-http-server-connections-checking-leak.js b/graal-nodejs/test/parallel/test-http-server-connections-checking-leak.js
new file mode 100644
index 00000000000..e28cf117c65
--- /dev/null
+++ b/graal-nodejs/test/parallel/test-http-server-connections-checking-leak.js
@@ -0,0 +1,24 @@
+'use strict';
+
+// Flags: --expose-gc
+
+// Check that creating a server without listening does not leak resources.
+
+require('../common');
+const onGC = require('../common/ongc');
+const Countdown = require('../common/countdown');
+
+const http = require('http');
+const max = 100;
+
+// Note that Countdown internally calls common.mustCall, that's why it's not done here.
+const countdown = new Countdown(max, () => {});
+
+for (let i = 0; i < max; i++) {
+  const server = http.createServer((req, res) => {});
+  onGC(server, { ongc: countdown.dec.bind(countdown) });
+}
+
+setImmediate(() => {
+  global.gc();
+});
diff --git a/graal-nodejs/test/parallel/test-https-server-connections-checking-leak.js b/graal-nodejs/test/parallel/test-https-server-connections-checking-leak.js
new file mode 100644
index 00000000000..3e7c45e4660
--- /dev/null
+++ b/graal-nodejs/test/parallel/test-https-server-connections-checking-leak.js
@@ -0,0 +1,29 @@
+'use strict';
+
+// Flags: --expose-gc
+
+// Check that creating a server without listening does not leak resources.
+
+const common = require('../common');
+
+if (!common.hasCrypto) {
+  common.skip('missing crypto');
+}
+
+const onGC = require('../common/ongc');
+const Countdown = require('../common/countdown');
+
+const https = require('https');
+const max = 100;
+
+// Note that Countdown internally calls common.mustCall, that's why it's not done here.
+const countdown = new Countdown(max, () => {});
+
+for (let i = 0; i < max; i++) {
+  const server = https.createServer((req, res) => {});
+  onGC(server, { ongc: countdown.dec.bind(countdown) });
+}
+
+setImmediate(() => {
+  global.gc();
+});