Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal: disable default memory leak warning for AbortSignal #55816

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/api/events.md
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,10 @@ that a "possible EventEmitter memory leak" has been detected. For any single
`EventEmitter`, the `emitter.getMaxListeners()` and `emitter.setMaxListeners()`
methods can be used to temporarily avoid this warning:

`defaultMaxListeners` has no effect on `AbortSignal` instances. While it is
still possible to use [`emitter.setMaxListeners(n)`][] to set a warning limit
for individual `AbortSignal` instances, per default `AbortSignal` instances will not warn.

```mjs
import { EventEmitter } from 'node:events';
const emitter = new EventEmitter();
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/abort_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const {
kResistStopPropagation,
kWeakHandler,
} = require('internal/event_target');
const { kMaxEventTargetListeners } = require('events');
const {
customInspectSymbol,
kEmptyObject,
Expand Down Expand Up @@ -164,6 +165,7 @@ class AbortSignal extends EventTarget {
}
super();

this[kMaxEventTargetListeners] = 0;
const {
aborted = false,
reason = undefined,
Expand Down
34 changes: 23 additions & 11 deletions test/parallel/test-eventtarget-memoryleakwarning.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,22 @@ const { setTimeout } = require('timers/promises');
common.expectWarning({
MaxListenersExceededWarning: [
['Possible EventTarget memory leak detected. 3 foo listeners added to ' +
'EventTarget. MaxListeners is 2. Use events.setMaxListeners() ' +
'EventTarget. MaxListeners is 2. Use events.setMaxListeners() ' +
'to increase limit'],
['Possible EventTarget memory leak detected. 3 foo listeners added to ' +
'[MessagePort [EventTarget]]. ' +
'MaxListeners is 2. ' +
'Use events.setMaxListeners() to increase ' +
'[MessagePort [EventTarget]]. ' +
'MaxListeners is 2. ' +
'Use events.setMaxListeners() to increase ' +
'limit'],
['Possible EventTarget memory leak detected. 3 foo listeners added to ' +
'[MessagePort [EventTarget]]. ' +
'MaxListeners is 2. ' +
'Use events.setMaxListeners() to increase ' +
'[MessagePort [EventTarget]]. ' +
'MaxListeners is 2. ' +
'Use events.setMaxListeners() to increase ' +
'limit'],
['Possible EventTarget memory leak detected. 3 foo listeners added to ' +
'[AbortSignal]. ' +
'MaxListeners is 2. ' +
'Use events.setMaxListeners() to increase ' +
['Possible EventTarget memory leak detected. 2 foo listeners added to ' +
'[AbortSignal]. ' +
'MaxListeners is 1. ' +
'Use events.setMaxListeners() to increase ' +
'limit'],
],
});
Expand Down Expand Up @@ -65,13 +65,25 @@ common.expectWarning({
mc.port1.addEventListener('foo', () => {});
mc.port1.addEventListener('foo', () => {});
mc.port1.addEventListener('foo', () => {});
}

{
// No warning emitted because AbortController ignores `EventEmitter.defaultMaxListeners`
setMaxListeners(2);
const ac = new AbortController();
ac.signal.addEventListener('foo', () => {});
ac.signal.addEventListener('foo', () => {});
ac.signal.addEventListener('foo', () => {});
}

{
// Will still warn as `setMaxListeners` can still manually set a limit
const ac = new AbortController();
setMaxListeners(1, ac.signal);
ac.signal.addEventListener('foo', () => {});
ac.signal.addEventListener('foo', () => {});
}

{
// It works for EventEmitters also
const ee = new EventEmitter();
Expand Down
Loading