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: #48373
PR-URL: #48383
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>

Author: Linkgoron

lib/_http_server.js

@@ -507,6 +507,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);
 
@@ -548,8 +553,7 @@ ObjectSetPrototypeOf(Server.prototype, net.Server.prototype);
 ObjectSetPrototypeOf(Server, net.Server);
 
 Server.prototype.close = function() {
-  this.closeIdleConnections();
-  clearInterval(this[kConnectionsCheckingInterval]);
+  httpServerPreClose(this);
   ReflectApply(net.Server.prototype.close, this, arguments);
 };
 
@@ -1193,4 +1197,6 @@ module.exports = {
   storeHTTPOptions,
   _connectionListener: connectionListener,
   kServerResponse,
+  httpServerPreClose,
+  kConnectionsCheckingInterval,
 };

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,
@@ -103,6 +105,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 {{

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);
+  }));
+}));

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

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);
+  }));
+}));

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