GeezerVison is showing github.com — https: fix connection checking interval not clearing on server close · nodejs/node@2969722
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/_http_server.js +8-1Lines changed: 8 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -505,6 +505,11 @@ function setupConnectionsTracking() { | 505 | 505 | setInterval(checkConnections.bind(this), this.connectionsCheckingInterval).unref(); | 506 | 506 | } | 507 | 507 |
| | 508 | +function httpServerPreClose(server) { | | 509 | + server.closeIdleConnections(); | | 510 | + clearInterval(server[kConnectionsCheckingInterval]); | | 511 | +} | | 512 | + | 508 | 513 | function Server(options, requestListener) { | 509 | 514 | if (!(this instanceof Server)) return new Server(options, requestListener); | 510 | 515 |
| @@ -547,7 +552,7 @@ ObjectSetPrototypeOf(Server.prototype, net.Server.prototype); | 547 | 552 | ObjectSetPrototypeOf(Server, net.Server); | 548 | 553 |
| 549 | 554 | Server.prototype.close = function() { | 550 | | - clearInterval(this[kConnectionsCheckingInterval]); | | 555 | + httpServerPreClose(this); | 551 | 556 | ReflectApply(net.Server.prototype.close, this, arguments); | 552 | 557 | }; | 553 | 558 |
| @@ -1190,4 +1195,6 @@ module.exports = { | 1190 | 1195 | storeHTTPOptions, | 1191 | 1196 | _connectionListener: connectionListener, | 1192 | 1197 | kServerResponse, | | 1198 | + httpServerPreClose, | | 1199 | + kConnectionsCheckingInterval, | 1193 | 1200 | }; |
Collapse file Copy file name to clipboardExpand all lines: lib/https.js +7Lines changed: 7 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -31,6 +31,7 @@ const { | 31 | 31 | JSONStringify, | 32 | 32 | ObjectAssign, | 33 | 33 | ObjectSetPrototypeOf, | | 34 | + ReflectApply, | 34 | 35 | ReflectConstruct, | 35 | 36 | } = primordials; | 36 | 37 |
| @@ -43,6 +44,7 @@ assertCrypto(); | 43 | 44 | const tls = require('tls'); | 44 | 45 | const { Agent: HttpAgent } = require('_http_agent'); | 45 | 46 | const { | | 47 | + httpServerPreClose, | 46 | 48 | Server: HttpServer, | 47 | 49 | setupConnectionsTracking, | 48 | 50 | storeHTTPOptions, | @@ -98,6 +100,11 @@ Server.prototype.closeIdleConnections = HttpServer.prototype.closeIdleConnection | 98 | 100 |
| 99 | 101 | Server.prototype.setTimeout = HttpServer.prototype.setTimeout; | 100 | 102 |
| | 103 | +Server.prototype.close = function() { | | 104 | + httpServerPreClose(this); | | 105 | + ReflectApply(tls.Server.prototype.close, this, arguments); | | 106 | +}; | | 107 | + | 101 | 108 | /** | 102 | 109 | * Creates a new `https.Server` instance. | 103 | 110 | * @param {{ | |
Collapse file Copy file name to clipboard +13Lines changed: 13 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +'use strict'; | | 2 | +const common = require('../common'); | | 3 | +const assert = require('assert'); | | 4 | +const { createServer } = require('http'); | | 5 | +const { kConnectionsCheckingInterval } = require('_http_server'); | | 6 | + | | 7 | +const server = createServer(function(req, res) {}); | | 8 | +server.listen(0, common.mustCall(function() { | | 9 | + assert.strictEqual(server[kConnectionsCheckingInterval]._destroyed, false); | | 10 | + server.close(common.mustCall(() => { | | 11 | + assert(server[kConnectionsCheckingInterval]._destroyed); | | 12 | + })); | | 13 | +})); |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-http-server-close-idle.js -1Lines changed: 0 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -42,7 +42,6 @@ server.listen(0, function() { | 42 | 42 | assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive')); | 43 | 43 | assert.strictEqual(connections, 2); | 44 | 44 |
| 45 | | - server.closeIdleConnections(); | 46 | 45 | server.close(common.mustCall()); | 47 | 46 |
| 48 | 47 | // Check that only the idle connection got closed | |
Collapse file Copy file name to clipboard +24Lines changed: 24 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +'use strict'; | | 2 | +const common = require('../common'); | | 3 | +const assert = require('assert'); | | 4 | +if (!common.hasCrypto) { | | 5 | + common.skip('missing crypto'); | | 6 | +} | | 7 | + | | 8 | +const { createServer } = require('https'); | | 9 | +const { kConnectionsCheckingInterval } = require('_http_server'); | | 10 | + | | 11 | +const fixtures = require('../common/fixtures'); | | 12 | + | | 13 | +const options = { | | 14 | + key: fixtures.readKey('agent1-key.pem'), | | 15 | + cert: fixtures.readKey('agent1-cert.pem') | | 16 | +}; | | 17 | + | | 18 | +const server = createServer(options, function(req, res) {}); | | 19 | +server.listen(0, common.mustCall(function() { | | 20 | + assert.strictEqual(server[kConnectionsCheckingInterval]._destroyed, false); | | 21 | + server.close(common.mustCall(() => { | | 22 | + assert(server[kConnectionsCheckingInterval]._destroyed); | | 23 | + })); | | 24 | +})); |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-https-server-close-idle.js -1Lines changed: 0 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -52,7 +52,6 @@ server.listen(0, function() { | 52 | 52 | assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive')); | 53 | 53 | assert.strictEqual(connections, 2); | 54 | 54 |
| 55 | | - server.closeIdleConnections(); | 56 | 55 | server.close(common.mustCall()); | 57 | 56 |
| 58 | 57 | // Check that only the idle connection got closed | |
|