From ce27ee701b51ff829788c03e31518bc9ca3f8db3 Mon Sep 17 00:00:00 2001 From: Tim Perry <1526883+pimterry@users.noreply.github.com> Date: Wed, 18 Oct 2023 20:44:40 +0100 Subject: [PATCH] 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. PR-URL: https://github.com/nodejs/node/pull/50136 Reviewed-By: Luigi Pinca Reviewed-By: Ben Noordhuis --- lib/_tls_wrap.js | 7 +++- .../test-tls-reinitialize-listeners.js | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-tls-reinitialize-listeners.js 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()); } 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(); + }); +}));