From 948d33294577d4c54981a455a97170df2ed30a43 Mon Sep 17 00:00:00 2001 From: tsctx <91457664+tsctx@users.noreply.github.com> Date: Sun, 28 Apr 2024 16:03:19 +0900 Subject: [PATCH 1/5] fix: don't add listeners to the same signal more than once --- lib/api/abort-signal.js | 47 +++++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/lib/api/abort-signal.js b/lib/api/abort-signal.js index 2afe747d3a1..5265c5bc270 100644 --- a/lib/api/abort-signal.js +++ b/lib/api/abort-signal.js @@ -1,8 +1,9 @@ const { addAbortListener } = require('../core/util') const { RequestAbortedError } = require('../core/errors') -const kListener = Symbol('kListener') const kSignal = Symbol('kSignal') +const kListenerList = Symbol('kListenersList') +const kHandler = Symbol('kHandler') function abort (self) { if (self.abort) { @@ -13,11 +14,26 @@ function abort (self) { removeSignal(self) } +function clearListeners (signal) { + const listenerList = signal[kListenerList] + if (!listenerList) { + return + } + if ('removeEventListener' in signal) { + signal.removeEventListener('abort', signal[kHandler]) + } else { + signal.removeListener('abort', signal[kHandler]) + } + if (listenerList.size !== 0) { + listenerList.clear() + } + signal[kListenerList] = null +} + function addSignal (self, signal) { self.reason = null self[kSignal] = null - self[kListener] = null if (!signal) { return @@ -29,26 +45,35 @@ function addSignal (self, signal) { } self[kSignal] = signal - self[kListener] = () => { - abort(self) + + if (!signal[kListenerList]) { + signal[kListenerList] = new Set() + signal[kHandler] = () => { + for (const listener of signal[kListenerList]) { + abort(listener) + } + clearListeners(signal) + } + addAbortListener(signal, signal[kHandler]) } - addAbortListener(self[kSignal], self[kListener]) + signal[kListenerList].add(self) } function removeSignal (self) { - if (!self[kSignal]) { + const signal = self[kSignal] + + if (!signal) { return } - if ('removeEventListener' in self[kSignal]) { - self[kSignal].removeEventListener('abort', self[kListener]) - } else { - self[kSignal].removeListener('abort', self[kListener]) + signal[kListenerList].delete(self) + + if (signal[kListenerList].size === 0) { + clearListeners(signal) } self[kSignal] = null - self[kListener] = null } module.exports = { From 678620cfad927011500e5f9c3a7003b7e0b14839 Mon Sep 17 00:00:00 2001 From: tsctx <91457664+tsctx@users.noreply.github.com> Date: Sun, 28 Apr 2024 16:59:01 +0900 Subject: [PATCH 2/5] refactoring --- lib/api/abort-signal.js | 47 +++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/lib/api/abort-signal.js b/lib/api/abort-signal.js index 5265c5bc270..9b29d6b00a6 100644 --- a/lib/api/abort-signal.js +++ b/lib/api/abort-signal.js @@ -1,9 +1,8 @@ const { addAbortListener } = require('../core/util') const { RequestAbortedError } = require('../core/errors') +const kListenersList = Symbol('kListenersList') const kSignal = Symbol('kSignal') -const kListenerList = Symbol('kListenersList') -const kHandler = Symbol('kHandler') function abort (self) { if (self.abort) { @@ -14,20 +13,26 @@ function abort (self) { removeSignal(self) } +function handleAbort (signal) { + for (const listener of signal[kListenersList]) { + abort(listener) + } + clearListeners(signal) +} + function clearListeners (signal) { - const listenerList = signal[kListenerList] - if (!listenerList) { + if (!signal[kListenersList]) { return } if ('removeEventListener' in signal) { - signal.removeEventListener('abort', signal[kHandler]) + signal.removeEventListener('abort', handleAbort) } else { - signal.removeListener('abort', signal[kHandler]) + signal.removeListener('abort', handleAbort) } - if (listenerList.size !== 0) { - listenerList.clear() + if (signal[kListenersList].size !== 0) { + signal[kListenersList].clear() } - signal[kListenerList] = null + signal[kListenersList] = null } function addSignal (self, signal) { @@ -46,31 +51,23 @@ function addSignal (self, signal) { self[kSignal] = signal - if (!signal[kListenerList]) { - signal[kListenerList] = new Set() - signal[kHandler] = () => { - for (const listener of signal[kListenerList]) { - abort(listener) - } - clearListeners(signal) - } - addAbortListener(signal, signal[kHandler]) + if (!signal[kListenersList]) { + signal[kListenersList] = new Set() + addAbortListener(signal, handleAbort) } - signal[kListenerList].add(self) + signal[kListenersList].add(self) } function removeSignal (self) { - const signal = self[kSignal] - - if (!signal) { + if (!self[kSignal]) { return } - signal[kListenerList].delete(self) + self[kSignal][kListenersList].delete(self) - if (signal[kListenerList].size === 0) { - clearListeners(signal) + if (self[kSignal][kListenersList].size === 0) { + clearListeners(self[kSignal]) } self[kSignal] = null From f804789cd9938b172149a0c5cfca2ea3891ce4c5 Mon Sep 17 00:00:00 2001 From: tsctx <91457664+tsctx@users.noreply.github.com> Date: Sun, 28 Apr 2024 17:08:00 +0900 Subject: [PATCH 3/5] fixup --- lib/api/abort-signal.js | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/api/abort-signal.js b/lib/api/abort-signal.js index 9b29d6b00a6..c9d505bb3d7 100644 --- a/lib/api/abort-signal.js +++ b/lib/api/abort-signal.js @@ -1,8 +1,10 @@ const { addAbortListener } = require('../core/util') const { RequestAbortedError } = require('../core/errors') +const { sign } = require('crypto') const kListenersList = Symbol('kListenersList') const kSignal = Symbol('kSignal') +const kHandle = Symbol('kHandle') function abort (self) { if (self.abort) { @@ -13,21 +15,14 @@ function abort (self) { removeSignal(self) } -function handleAbort (signal) { - for (const listener of signal[kListenersList]) { - abort(listener) - } - clearListeners(signal) -} - function clearListeners (signal) { if (!signal[kListenersList]) { return } if ('removeEventListener' in signal) { - signal.removeEventListener('abort', handleAbort) + signal.removeEventListener('abort', signal[kHandle]) } else { - signal.removeListener('abort', handleAbort) + signal.removeListener('abort', signal[kHandle]) } if (signal[kListenersList].size !== 0) { signal[kListenersList].clear() @@ -53,7 +48,13 @@ function addSignal (self, signal) { if (!signal[kListenersList]) { signal[kListenersList] = new Set() - addAbortListener(signal, handleAbort) + signal[kHandle] = () => { + for (const listener of signal[kListenersList]) { + abort(listener) + } + clearListeners(signal) + } + addAbortListener(signal, signal[kHandle]) } signal[kListenersList].add(self) From 3cee000a60025ec3985f3b8619318dc6beffd577 Mon Sep 17 00:00:00 2001 From: tsctx <91457664+tsctx@users.noreply.github.com> Date: Sun, 28 Apr 2024 17:39:33 +0900 Subject: [PATCH 4/5] add test --- test/issue-3131.js | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 test/issue-3131.js diff --git a/test/issue-3131.js b/test/issue-3131.js new file mode 100644 index 00000000000..02c1f501b11 --- /dev/null +++ b/test/issue-3131.js @@ -0,0 +1,43 @@ +'use strict' + +const { createServer } = require('node:http') +const { EventEmitter, once } = require('node:events') +const { request } = require('../') +const { test } = require('node:test') +const { closeServerAsPromise } = require('./utils/node-http') +const { strictEqual } = require('node:assert') + +test('issue #3131', async (t) => { + const emitter = new EventEmitter() + + const server = createServer((req, res) => { + res.end('Hi') + }) + + server.listen(0) + + await once(server, 'listening') + + t.after(closeServerAsPromise(server)) + + const url = `http://localhost:${server.address().port}` + + let warningEmitted = false + function onWarning () { + warningEmitted = true + } + process.on('warning', onWarning) + t.after(() => { + process.off('warning', onWarning) + }) + + const promises = new Array(50) + + for (let i = 0; i < promises.length; ++i) { + promises[i] = request(url, { signal: emitter }) + } + + await Promise.all(promises) + + strictEqual(warningEmitted, false) +}) From 5009498c783d1a5be51b4a929296da0246d31370 Mon Sep 17 00:00:00 2001 From: tsctx <91457664+tsctx@users.noreply.github.com> Date: Sun, 28 Apr 2024 17:41:26 +0900 Subject: [PATCH 5/5] lint fix --- lib/api/abort-signal.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/api/abort-signal.js b/lib/api/abort-signal.js index c9d505bb3d7..0cd020ba200 100644 --- a/lib/api/abort-signal.js +++ b/lib/api/abort-signal.js @@ -1,6 +1,5 @@ const { addAbortListener } = require('../core/util') const { RequestAbortedError } = require('../core/errors') -const { sign } = require('crypto') const kListenersList = Symbol('kListenersList') const kSignal = Symbol('kSignal')