From 6e33069d0f8d0d57b3571fc22fb3345f2d5fbd35 Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Tue, 28 Oct 2025 22:09:25 +0100 Subject: [PATCH 01/16] net: do not duplicate `connect` and `finish` listners on `destroySoon` --- lib/net.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/net.js b/lib/net.js index a391e9da30f861..7829462f3af9d8 100644 --- a/lib/net.js +++ b/lib/net.js @@ -529,8 +529,9 @@ Socket.prototype._unrefTimer = function _unrefTimer() { // sent out to the other side. Socket.prototype._final = function(cb) { // If still connecting - defer handling `_final` until 'connect' will happen - if (this.connecting) { + if (this.connecting && !this._finalizingOnConnect) { debug('_final: not yet connected'); + this._finalizingOnConnect = true; return this.once('connect', () => this._final(cb)); } @@ -800,8 +801,10 @@ Socket.prototype.destroySoon = function() { if (this.writableFinished) this.destroy(); - else + else if (!this._destroyingOnFinish) { + this._destroyingOnFinish = true; this.once('finish', this.destroy); + } }; From 9905cfbf73b473a0ce843a53d9ac0064e367cfe8 Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Tue, 28 Oct 2025 22:49:09 +0100 Subject: [PATCH 02/16] unit test for not duplicating event listeners --- ...socket-not-duplicates-destroy-soon-listeners.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js diff --git a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js new file mode 100644 index 00000000000000..89dc0d10ccad00 --- /dev/null +++ b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js @@ -0,0 +1,14 @@ +'use strict'; +const assert = require('assert'); +const net = require('net'); + +const socket = new net.Socket(); +socket.on('error', () => {}); +socket.connect({ host: 'non-existing.domain', port: 1234 }); +socket.destroySoon(); +socket.connect({ host: 'non-existing.domain', port: 1234 }); +socket.destroySoon(); +const finishListenersCount = socket.listeners('finish').length; +const connectListenersCount = socket.listeners('connect').length; +assert.equal(finishListenersCount, 1); +assert.equal(connectListenersCount, 1); From afe8307f9c7b2a93555a51fe777294f6743d058e Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Tue, 28 Oct 2025 22:55:23 +0100 Subject: [PATCH 03/16] add messages to test assertions --- ...ket-not-duplicates-destroy-soon-listeners.js | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js index 89dc0d10ccad00..13795fe4bbc3cc 100644 --- a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js +++ b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js @@ -3,12 +3,21 @@ const assert = require('assert'); const net = require('net'); const socket = new net.Socket(); -socket.on('error', () => {}); +socket.on('error', () => { + // noop +}); socket.connect({ host: 'non-existing.domain', port: 1234 }); socket.destroySoon(); -socket.connect({ host: 'non-existing.domain', port: 1234 }); socket.destroySoon(); const finishListenersCount = socket.listeners('finish').length; const connectListenersCount = socket.listeners('connect').length; -assert.equal(finishListenersCount, 1); -assert.equal(connectListenersCount, 1); +assert.equal( + finishListenersCount, + 1, + '"finish" event listeners should not be duplicated for multiple "Socket.destroySoon" calls' +); +assert.equal( + connectListenersCount, + 1, + '"connect" event listeners should not be duplicated for multiple "Socket.destroySoon" calls' +); From 565e2c26642341063edbe1ac52acb6f3be399456 Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Tue, 28 Oct 2025 23:08:44 +0100 Subject: [PATCH 04/16] use `assert.strictEqual` instead of `assert.equal` --- .../test-net-socket-not-duplicates-destroy-soon-listeners.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js index 13795fe4bbc3cc..504e66221d23ab 100644 --- a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js +++ b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js @@ -11,12 +11,12 @@ socket.destroySoon(); socket.destroySoon(); const finishListenersCount = socket.listeners('finish').length; const connectListenersCount = socket.listeners('connect').length; -assert.equal( +assert.strictEqual( finishListenersCount, 1, '"finish" event listeners should not be duplicated for multiple "Socket.destroySoon" calls' ); -assert.equal( +assert.strictEqual( connectListenersCount, 1, '"connect" event listeners should not be duplicated for multiple "Socket.destroySoon" calls' From 94edac0f243b740b8783bab691ae1f32bba8fbec Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Tue, 28 Oct 2025 23:12:54 +0100 Subject: [PATCH 05/16] require `common` module --- .../test-net-socket-not-duplicates-destroy-soon-listeners.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js index 504e66221d23ab..5d24ba5050fe42 100644 --- a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js +++ b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js @@ -1,4 +1,6 @@ 'use strict'; +require('../common'); + const assert = require('assert'); const net = require('net'); From e5ec2a4e09806aa7f1e5827498ed8bf9fbeecd85 Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Tue, 28 Oct 2025 23:19:27 +0100 Subject: [PATCH 06/16] remove third argument on `strictEqual` call --- ...t-socket-not-duplicates-destroy-soon-listeners.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js index 5d24ba5050fe42..f7b94c12353375 100644 --- a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js +++ b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js @@ -13,13 +13,5 @@ socket.destroySoon(); socket.destroySoon(); const finishListenersCount = socket.listeners('finish').length; const connectListenersCount = socket.listeners('connect').length; -assert.strictEqual( - finishListenersCount, - 1, - '"finish" event listeners should not be duplicated for multiple "Socket.destroySoon" calls' -); -assert.strictEqual( - connectListenersCount, - 1, - '"connect" event listeners should not be duplicated for multiple "Socket.destroySoon" calls' -); +assert.strictEqual(finishListenersCount, 1); +assert.strictEqual(connectListenersCount, 1); From ae5739e48509d66a20b0019c9d9aaa3cb481549f Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Tue, 28 Oct 2025 23:41:40 +0100 Subject: [PATCH 07/16] use `addresses.INVALID_HOST` in test --- .../test-net-socket-not-duplicates-destroy-soon-listeners.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js index f7b94c12353375..78131558c9f37c 100644 --- a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js +++ b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js @@ -1,5 +1,6 @@ 'use strict'; require('../common'); +const { addresses } = require('../common/internet'); const assert = require('assert'); const net = require('net'); @@ -8,7 +9,7 @@ const socket = new net.Socket(); socket.on('error', () => { // noop }); -socket.connect({ host: 'non-existing.domain', port: 1234 }); +socket.connect({ host: addresses.INVALID_HOST, port: 1234 }); socket.destroySoon(); socket.destroySoon(); const finishListenersCount = socket.listeners('finish').length; From a70ef63631c339a3c6eb050366d503271281850c Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Wed, 29 Oct 2025 23:07:43 +0100 Subject: [PATCH 08/16] `return` from `_final` when `connecting` even if no listeners added --- lib/net.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/net.js b/lib/net.js index 7829462f3af9d8..39da60773c9989 100644 --- a/lib/net.js +++ b/lib/net.js @@ -529,10 +529,13 @@ Socket.prototype._unrefTimer = function _unrefTimer() { // sent out to the other side. Socket.prototype._final = function(cb) { // If still connecting - defer handling `_final` until 'connect' will happen - if (this.connecting && !this._finalizingOnConnect) { + if (this.connecting) { debug('_final: not yet connected'); - this._finalizingOnConnect = true; - return this.once('connect', () => this._final(cb)); + if (!this._finalizingOnConnect) { + this._finalizingOnConnect = true; + this.once('connect', () => this._final(cb)); + } + return; } if (!this._handle) From a63fb3799a05e4aa663d4581d768489648ac5159 Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Thu, 30 Oct 2025 19:53:23 +0100 Subject: [PATCH 09/16] rework test to better reproduce mentioned issue --- lib/net.js | 10 ++++++++-- ...et-not-duplicates-destroy-soon-listeners.js | 18 +++++++++++------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/lib/net.js b/lib/net.js index 39da60773c9989..06da8b146fbe3c 100644 --- a/lib/net.js +++ b/lib/net.js @@ -533,7 +533,10 @@ Socket.prototype._final = function(cb) { debug('_final: not yet connected'); if (!this._finalizingOnConnect) { this._finalizingOnConnect = true; - this.once('connect', () => this._final(cb)); + this.once('connect', () => { + this._final(cb); + this._finalizingOnConnect = false; + }); } return; } @@ -806,7 +809,10 @@ Socket.prototype.destroySoon = function() { this.destroy(); else if (!this._destroyingOnFinish) { this._destroyingOnFinish = true; - this.once('finish', this.destroy); + this.once('finish', () => { + this.destroy(); + this._destroyingOnFinish = false; + }); } }; diff --git a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js index 78131558c9f37c..c3a196e30bb1cb 100644 --- a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js +++ b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js @@ -9,10 +9,14 @@ const socket = new net.Socket(); socket.on('error', () => { // noop }); -socket.connect({ host: addresses.INVALID_HOST, port: 1234 }); -socket.destroySoon(); -socket.destroySoon(); -const finishListenersCount = socket.listeners('finish').length; -const connectListenersCount = socket.listeners('connect').length; -assert.strictEqual(finishListenersCount, 1); -assert.strictEqual(connectListenersCount, 1); +const connectOptions = { host: addresses.INVALID_HOST, port: 1234 }; + +socket.connect(connectOptions); +socket.destroySoon(); // Adds "connect" and "finish" event listeners when socket has "writable" state +socket.destroy(); // Makes imideditly socket again "writable" + +socket.connect(connectOptions); +socket.destroySoon(); // Should not duplicate "connect" and "finish" event listeners + +assert.strictEqual(socket.listeners('finish').length, 1); +assert.strictEqual(socket.listeners('connect').length, 1); From 40ef4553497d7d79680c05af2122ebd214bb5c5f Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Fri, 31 Oct 2025 22:39:21 +0100 Subject: [PATCH 10/16] revert changes in `_final` --- lib/net.js | 11 ++--------- ...ket-not-duplicates-destroy-soon-listeners.js | 17 +++++------------ 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/lib/net.js b/lib/net.js index 06da8b146fbe3c..b46fb3594c94cf 100644 --- a/lib/net.js +++ b/lib/net.js @@ -531,14 +531,7 @@ Socket.prototype._final = function(cb) { // If still connecting - defer handling `_final` until 'connect' will happen if (this.connecting) { debug('_final: not yet connected'); - if (!this._finalizingOnConnect) { - this._finalizingOnConnect = true; - this.once('connect', () => { - this._final(cb); - this._finalizingOnConnect = false; - }); - } - return; + return this.once('connect', () => this._final(cb)); } if (!this._handle) @@ -810,8 +803,8 @@ Socket.prototype.destroySoon = function() { else if (!this._destroyingOnFinish) { this._destroyingOnFinish = true; this.once('finish', () => { - this.destroy(); this._destroyingOnFinish = false; + this.destroy(); }); } }; diff --git a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js index c3a196e30bb1cb..317a97b8364c48 100644 --- a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js +++ b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js @@ -3,20 +3,13 @@ require('../common'); const { addresses } = require('../common/internet'); const assert = require('assert'); -const net = require('net'); +const { Socket } = require('net'); -const socket = new net.Socket(); +const socket = new Socket(); socket.on('error', () => { // noop }); -const connectOptions = { host: addresses.INVALID_HOST, port: 1234 }; - -socket.connect(connectOptions); -socket.destroySoon(); // Adds "connect" and "finish" event listeners when socket has "writable" state -socket.destroy(); // Makes imideditly socket again "writable" - -socket.connect(connectOptions); -socket.destroySoon(); // Should not duplicate "connect" and "finish" event listeners - +socket.connect({ host: addresses.INVALID_HOST, port: 1234 }); +socket.destroySoon(); +socket.destroySoon(); assert.strictEqual(socket.listeners('finish').length, 1); -assert.strictEqual(socket.listeners('connect').length, 1); From c2e85a040e1106c009c653a99f9123f0b8592f1a Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Sat, 1 Nov 2025 23:45:10 +0100 Subject: [PATCH 11/16] make `destroySoon` noop after first call --- lib/net.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/net.js b/lib/net.js index b46fb3594c94cf..bf35bd7d8d4909 100644 --- a/lib/net.js +++ b/lib/net.js @@ -793,23 +793,22 @@ function onReadableStreamEnd() { } } - Socket.prototype.destroySoon = function() { - if (this.writable) - this.end(); + if (this.destroyingOnFinish) return; + + if (this.writable) this.end(); - if (this.writableFinished) + if (this.writableFinished) { this.destroy(); - else if (!this._destroyingOnFinish) { - this._destroyingOnFinish = true; + } else { + this.destroyingOnFinish = true; this.once('finish', () => { - this._destroyingOnFinish = false; + this.destroyingOnFinish = false; this.destroy(); }); } }; - Socket.prototype._destroy = function(exception, cb) { debug('destroy'); From 3d238a0b31db3858b019956e55f6514d0192c7cc Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Sat, 1 Nov 2025 23:53:35 +0100 Subject: [PATCH 12/16] remove unwanted brackets --- lib/net.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/net.js b/lib/net.js index bf35bd7d8d4909..3f56049dac3ac1 100644 --- a/lib/net.js +++ b/lib/net.js @@ -798,9 +798,9 @@ Socket.prototype.destroySoon = function() { if (this.writable) this.end(); - if (this.writableFinished) { + if (this.writableFinished) this.destroy(); - } else { + else { this.destroyingOnFinish = true; this.once('finish', () => { this.destroyingOnFinish = false; From 81ac7529935469585b4c17811e2fe776fcd87de4 Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Sun, 2 Nov 2025 07:32:13 +0100 Subject: [PATCH 13/16] remove unwanted formatting changes --- lib/net.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/net.js b/lib/net.js index 3f56049dac3ac1..42dfcf7f93b874 100644 --- a/lib/net.js +++ b/lib/net.js @@ -793,10 +793,12 @@ function onReadableStreamEnd() { } } + Socket.prototype.destroySoon = function() { if (this.destroyingOnFinish) return; - if (this.writable) this.end(); + if (this.writable) + this.end(); if (this.writableFinished) this.destroy(); @@ -809,6 +811,7 @@ Socket.prototype.destroySoon = function() { } }; + Socket.prototype._destroy = function(exception, cb) { debug('destroy'); From f703403e2b0683c7bc5116bebdf637fd9545b004 Mon Sep 17 00:00:00 2001 From: Artur Gawlik Date: Sun, 2 Nov 2025 23:34:30 +0100 Subject: [PATCH 14/16] Update test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js Co-authored-by: Luigi Pinca --- .../test-net-socket-not-duplicates-destroy-soon-listeners.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js index 317a97b8364c48..a60ef3ace09ba9 100644 --- a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js +++ b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js @@ -1,6 +1,5 @@ 'use strict'; require('../common'); -const { addresses } = require('../common/internet'); const assert = require('assert'); const { Socket } = require('net'); From 7b5a01ab4af4e4ef4232ce8c049e2ed01fa8dadc Mon Sep 17 00:00:00 2001 From: Artur Gawlik Date: Sun, 2 Nov 2025 23:34:42 +0100 Subject: [PATCH 15/16] Update test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js Co-authored-by: Luigi Pinca --- .../test-net-socket-not-duplicates-destroy-soon-listeners.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js index a60ef3ace09ba9..cb2086bc547ccf 100644 --- a/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js +++ b/test/parallel/test-net-socket-not-duplicates-destroy-soon-listeners.js @@ -5,10 +5,6 @@ const assert = require('assert'); const { Socket } = require('net'); const socket = new Socket(); -socket.on('error', () => { - // noop -}); -socket.connect({ host: addresses.INVALID_HOST, port: 1234 }); socket.destroySoon(); socket.destroySoon(); assert.strictEqual(socket.listeners('finish').length, 1); From c9a2c5e6f14887d0d0fbfe1888060ebe67a3fa6c Mon Sep 17 00:00:00 2001 From: arturgawlik Date: Mon, 3 Nov 2025 16:19:25 +0100 Subject: [PATCH 16/16] use symbol for storing `destroyingOnFinish` flag --- lib/net.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/net.js b/lib/net.js index 42dfcf7f93b874..36b0863fa32a5d 100644 --- a/lib/net.js +++ b/lib/net.js @@ -152,6 +152,7 @@ const DEFAULT_IPV6_ADDR = '::'; const noop = () => {}; const kPerfHooksNetConnectContext = Symbol('kPerfHooksNetConnectContext'); +const kDestroyingOnFinish = Symbol('kDestroyingOnFinish'); const dc = require('diagnostics_channel'); const netClientSocketChannel = dc.channel('net.client.socket'); @@ -795,7 +796,7 @@ function onReadableStreamEnd() { Socket.prototype.destroySoon = function() { - if (this.destroyingOnFinish) return; + if (this[kDestroyingOnFinish]) return; if (this.writable) this.end(); @@ -803,9 +804,9 @@ Socket.prototype.destroySoon = function() { if (this.writableFinished) this.destroy(); else { - this.destroyingOnFinish = true; + this[kDestroyingOnFinish] = true; this.once('finish', () => { - this.destroyingOnFinish = false; + this[kDestroyingOnFinish] = false; this.destroy(); }); }