Skip to content

Commit

Permalink
fix(zone.js): remove abort listener on a signal when actual event i…
Browse files Browse the repository at this point in the history
…s removed (#55339)

This commit updates the implementation of the `addEventListener` patcher.

We're currently creating an abort event listener on the signal (when it's provided)
and never remove it. The abort event listener creates a closure which captures `task`
(tasks capture zones and other stuff too) and prevent `task`, zones and signals from
being garbage collected.

We now store the function which removes the abort event listener when the actual event
listener is being removed. The function is stored on task data since task data is already
being used to store different types of information that's necessary to be shared between
`addEventListener` and `removeEventListener`.

Closes #54739

PR Close #55339
  • Loading branch information
arturovt authored and AndrewKushnir committed May 7, 2024
1 parent afc057e commit a9460d0
Showing 1 changed file with 23 additions and 16 deletions.
39 changes: 23 additions & 16 deletions packages/zone.js/lib/common/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,13 +427,7 @@ export function patchEventTarget(
const passive =
passiveSupported && !!passiveEvents && passiveEvents.indexOf(eventName) !== -1;
const options = buildEventListenerOptions(arguments[2], passive);
const signal =
options &&
typeof options === 'object' &&
options.signal &&
typeof options.signal === 'object'
? options.signal
: undefined;
const signal: AbortSignal | undefined = options?.signal;
if (signal?.aborted) {
// the signal is an aborted one, just return without attaching the event listener.
return;
Expand Down Expand Up @@ -528,14 +522,18 @@ export function patchEventTarget(
if (signal) {
// after task is scheduled, we need to store the signal back to task.options
taskData.options.signal = signal;
nativeListener.call(
signal,
'abort',
() => {
task.zone.cancelTask(task);
},
{once: true},
);
// Wrapping `task` in a weak reference would not prevent memory leaks. Weak references are
// primarily used for preventing strong references cycles. `onAbort` is always reachable
// as it's an event listener, so its closure retains a strong reference to the `task`.
const onAbort = () => task.zone.cancelTask(task);
nativeListener.call(signal, 'abort', onAbort, {once: true});
// We need to remove the `abort` listener when the event listener is going to be removed,
// as it creates a closure that captures `task`. This closure retains a reference to the
// `task` object even after it goes out of scope, preventing `task` from being garbage
// collected.
if (data) {
(data as any).removeAbortListener = () => signal.removeEventListener('abort', onAbort);
}
}

// should clear taskData.target to avoid memory leak
Expand Down Expand Up @@ -620,7 +618,7 @@ export function patchEventTarget(
if (symbolEventNames) {
symbolEventName = symbolEventNames[capture ? TRUE_STR : FALSE_STR];
}
const existingTasks = symbolEventName && target[symbolEventName];
const existingTasks: Task[] = symbolEventName && target[symbolEventName];
if (existingTasks) {
for (let i = 0; i < existingTasks.length; i++) {
const existingTask = existingTasks[i];
Expand All @@ -643,6 +641,15 @@ export function patchEventTarget(
target[onPropertySymbol] = null;
}
}

// Note that `removeAllListeners` would ultimately call `removeEventListener`,
// so we're safe to remove the abort listener only once here.
const taskData = existingTask.data as any;
if (taskData?.removeAbortListener) {
taskData.removeAbortListener();
taskData.removeAbortListener = null;
}

existingTask.zone.cancelTask(existingTask);
if (returnTarget) {
return target;
Expand Down

0 comments on commit a9460d0

Please sign in to comment.