From 43c4a9e570606eb5fe1e952fc869f85b373b896d Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Tue, 10 Oct 2023 12:56:44 +0100 Subject: [PATCH 1/2] tls: reduce TLS 'close' event listener warnings Without this, some heavy usage of TLS sockets can result in MaxListenersExceededWarning firing, from the 'this.on('close', ...)' line here. These appear to come from reinitializeHandle, which calls _wrapHandle repeatedly on the same socket instance. --- lib/_tls_wrap.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 48108710933382..9cf3fc41485080 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -713,7 +713,12 @@ TLSSocket.prototype._wrapHandle = function(wrap, handle, wrapHasActiveWriteFromP this[kRes] = res; defineHandleReading(this, handle); - this.on('close', onSocketCloseDestroySSL); + // Guard against adding multiple listeners, as this method may be called + // repeatedly on the same socket by reinitializeHandle + if (this.listenerCount('close', onSocketCloseDestroySSL) === 0) { + this.on('close', onSocketCloseDestroySSL); + } + if (wrap) { wrap.on('close', () => this.destroy()); } From f531d1573764c65adc79f86f0d6d6a02a1d7b580 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Mon, 16 Oct 2023 16:47:05 +0200 Subject: [PATCH 2/2] Add a test for the TLS reinitialization duplicate-listeners bug --- .../test-tls-reinitialize-listeners.js | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 test/parallel/test-tls-reinitialize-listeners.js diff --git a/test/parallel/test-tls-reinitialize-listeners.js b/test/parallel/test-tls-reinitialize-listeners.js new file mode 100644 index 00000000000000..ffd7c825b0f699 --- /dev/null +++ b/test/parallel/test-tls-reinitialize-listeners.js @@ -0,0 +1,42 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) { + common.skip('missing crypto'); +} + +const events = require('events'); +const fixtures = require('../common/fixtures'); +const tls = require('tls'); +const { kReinitializeHandle } = require('internal/net'); + +// Test that repeated calls to kReinitializeHandle() do not result in repeatedly +// adding new listeners on the socket (i.e. no MaxListenersExceededWarnings) + +process.on('warning', common.mustNotCall()); + +const server = tls.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem') +}); + +server.listen(0, common.mustCall(function() { + const socket = tls.connect({ + port: this.address().port, + rejectUnauthorized: false + }); + + socket.on('secureConnect', common.mustCall(function() { + for (let i = 0; i < events.defaultMaxListeners + 1; i++) { + socket[kReinitializeHandle](); + } + + socket.destroy(); + })); + + socket.on('close', function() { + server.close(); + }); +}));