From d71750cb3aa17430b1a1e78455cc31f37564bcd2 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 d2b510c64bbb..f3a89abccfd4 100644 --- a/lib/net.js +++ b/lib/net.js @@ -753,8 +753,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)); } @@ -1093,8 +1094,10 @@ Socket.prototype.destroySoon = function() { if (this.writableFinished) this.destroy(); - else + else if (!this._destroyingOnFinish) { + this._destroyingOnFinish = true; this.once('finish', this.destroy); + } }; From 5e0ce41d0a5d57c16d4326d9fb5edcd83c43f515 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 000000000000..89dc0d10ccad --- /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 337dc518c63261adb6cde0bf793803b1647ba92b 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 89dc0d10ccad..13795fe4bbc3 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 5e57298fa87c476de7b944cbb4a800c3c4817002 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 13795fe4bbc3..504e66221d23 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 9e5ccdefb11e161d3b02cd98a3a2ab1eb6052cab 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 504e66221d23..5d24ba5050fe 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 9d081a1078f149be01ee57666f57d3ed76ea1209 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 5d24ba5050fe..f7b94c123533 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 73dd4608b23a1f413c61f6b67c8d019f85ab0e0a 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 f7b94c123533..78131558c9f3 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 2814d99e24de50bf549fd037f089ff73b7b6ae5a 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 f3a89abccfd4..790d14e67678 100644 --- a/lib/net.js +++ b/lib/net.js @@ -753,10 +753,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 bfb8f92f702745e72ea263acb0c665327c377bb4 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 790d14e67678..8416f10de3c1 100644 --- a/lib/net.js +++ b/lib/net.js @@ -757,7 +757,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; } @@ -1099,7 +1102,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 78131558c9f3..c3a196e30bb1 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 5c1fc21506998d6b863e1cfce8365312136c70b9 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 8416f10de3c1..ca4fc4cbbb6d 100644 --- a/lib/net.js +++ b/lib/net.js @@ -755,14 +755,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) @@ -1103,8 +1096,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 c3a196e30bb1..317a97b8364c 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 c544a3a04a40ee27488d1bd52d3e464271f2d1bb 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 ca4fc4cbbb6d..5a43c52822a1 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1086,23 +1086,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 dba0ca9e6e8b8dd721abe458420d6c36e9a09f78 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 5a43c52822a1..04a037c5ce92 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1091,9 +1091,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 d2f4ab1921f5cada419a7c20cc6cb2ea2435fcf4 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 04a037c5ce92..6b3bdfe4aa4b 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1086,10 +1086,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(); @@ -1102,6 +1104,7 @@ Socket.prototype.destroySoon = function() { } }; + Socket.prototype._destroy = function(exception, cb) { debug('destroy'); From ff6cde7eeb6f8a32de322fca5c1de17cc2c0920b 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 317a97b8364c..a60ef3ace09b 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 58e07ef06cf4ae9c991313396babbda623daf394 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 a60ef3ace09b..cb2086bc547c 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 58a7f8d6bb1359cf51983a3bdb23629f7a4a052a 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 6b3bdfe4aa4b..63bb811c5bd5 100644 --- a/lib/net.js +++ b/lib/net.js @@ -172,6 +172,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'); @@ -1088,7 +1089,7 @@ function onReadableStreamEnd() { Socket.prototype.destroySoon = function() { - if (this.destroyingOnFinish) return; + if (this[kDestroyingOnFinish]) return; if (this.writable) this.end(); @@ -1096,9 +1097,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(); }); }