AbortSignal.any() leaks when any of the provided signal is long-lived

[nktpro]

Version: Deno 1.45.5

The current implementation of AbortSignal.any([...]) will lead to memory leaks if any of the provided signals are long-lived and retained in memory. A typical real-world example is a global shared signal that is only aborted when the process receives an interruption signal (i.e. SIGTERM or SIGINT).

Here's a simple reproducer:

import { format } from "jsr:@std/fmt/bytes";

const ac = new AbortController();
let count = 0;

setInterval(() => {
  console.log("rss", format(Deno.memoryUsage().rss), "iterations", count);
}, 1000);

setInterval(() => {
  for (let i = 0; i < 10000; i++) {
    const anySignal = AbortSignal.any([ac.signal]);
    const onAbort = () => {};
    anySignal.addEventListener("abort", onAbort);
    anySignal.removeEventListener("abort", onAbort);
    count++;
  }
}, 0);

Running this will show a clear memory leak pattern of RSS growing rapidly:

rss 121 MB iterations 910000
rss 151 MB iterations 1720000
rss 180 MB iterations 2510000
rss 211 MB iterations 3280000
rss 237 MB iterations 4060000
rss 274 MB iterations 4800000
rss 296 MB iterations 5590000
rss 320 MB iterations 6340000
rss 360 MB iterations 7070000
rss 379 MB iterations 7870000
rss 404 MB iterations 8700000
rss 435 MB iterations 9460000
rss 466 MB iterations 10110000
rss 503 MB iterations 10940000
rss 524 MB iterations 11530000
rss 541 MB iterations 12300000
rss 569 MB iterations 13000000
rss 584 MB iterations 13780000
rss 600 MB iterations 14540000
rss 648 MB iterations 15010000
rss 697 MB iterations 15710000
...

2 things could be observed:

1. AbortSignal.any seems to eagerly add a listener to each provided signal, regardless of whether the combined signal itself has any listener at all. As a consequence, the leak can also be observed with just this:

while (true) {
  // ...
  AbortSignal.any([ac.signal])
  // ...
}

2. AbortSignal.any does not remove the listener from all provided signals upon removeEventListener being called on the combined signal.

I believe one way to address this is to:

1. Lazily add listeners to the underlying signals only when the first addEventListener is called on the combined signal
2. Remove all listeners to the underlying signals when there are no remaining listeners to the combined signal.

As an example, here's a suggested implementation:

class AnyAbortSignal implements AbortSignal {
  #listenerSet = new Set<EventListenerOrEventListenerObject>();
  #isListening = false;
  // deno-lint-ignore no-explicit-any
  #onabort: ((this: AbortSignal, ev: Event) => any) | null = null;

  get aborted(): boolean {
    for (const signal of this.signals) {
      if (signal.aborted) {
        return true;
      }
    }
    return false;
  }

  // deno-lint-ignore no-explicit-any
  get reason(): any {
    for (const signal of this.signals) {
      if (signal.aborted) {
        return signal.reason;
      }
    }

    return undefined;
  }

  constructor(private signals: AbortSignal[]) {
    this.onAbort = this.onAbort.bind(this);
  }

  // deno-lint-ignore no-explicit-any
  set onabort(value: ((this: AbortSignal, ev: Event) => any) | null) {
    this.#onabort = value;
    this.#startListening();
  }

  throwIfAborted(): void {
    for (const signal of this.signals) {
      signal.throwIfAborted();
    }
  }

  protected onAbort(event: Event) {
    this.#stopListening();

    this.#onabort?.call(this, event);

    for (const listener of this.#listenerSet) {
      if (typeof listener === "function") {
        listener(event);
      } else {
        listener.handleEvent(event);
      }
    }

    this.#listenerSet.clear();
  }

  #startListening() {
    if (!this.#isListening) {
      this.#isListening = true;
      for (const signal of this.signals) {
        signal.addEventListener("abort", this.onAbort);
      }
    }
  }

  #stopListening() {
    if (this.#isListening) {
      this.#isListening = false;
      for (const signal of this.signals) {
        signal.removeEventListener("abort", this.onAbort);
      }
    }
  }

  addEventListener(
    _type: "abort",
    listener: EventListenerOrEventListenerObject,
    _options?: boolean | AddEventListenerOptions,
  ): void {
    if (!this.aborted) {
      this.#listenerSet.add(listener);
      this.#startListening();
    }
  }

  removeEventListener(
    _type: "abort",
    listener: EventListenerOrEventListenerObject,
    _options?: boolean | EventListenerOptions,
  ): void {
    this.#listenerSet.delete(listener);
    if (this.#listenerSet.size === 0 && this.#isListening) {
      this.#stopListening();
    }
  }

  dispatchEvent(_event: Event): boolean {
    return false;
  }
}

[lucacasonato]

The suggested fix looks good. Do you want to raise a PR?

[nktpro]

I'm afraid this is more of a spec/ecosystem-wide issue than just Deno's implementation...

As I dug up a bit more, the nodejs's implementation is practically the same as deno's. Both follow the algorithm laid out from this whatwg/dom PR.

Here's the result of running the same reproducer in nodejs 22.4.1, which suffers the same issue:

rss 679 MB iterations 2390000
rss 1.07 GB iterations 4430000
rss 1.72 GB iterations 6420000
rss 2.31 GB iterations 8390000
rss 2.14 GB iterations 10330000
rss 2.46 GB iterations 12400000
rss 2.71 GB iterations 14000000
rss 2.73 GB iterations 15730000
node:internal/abort_controller:244
        signal[kDependantSignals].add(resultSignalWeakRef);
                                  ^

RangeError: Set maximum size exceeded
    at Set.add (<anonymous>)
    at AbortSignal.any (node:internal/abort_controller:244:35)
    at ...abort_signal_any_leak.js:15:42

Node.js v22.4.1

Others seemed to also have pointed out this exact issue in the past.

Perhaps we either accept that this is by design with AbortSignal.any, where it must not be used with any long-lived signal, or we should address this in the whatwg/dom proposed algorithm such that all downstream implementations correct this.

[jazelly]

I think this fix can be applied as the spec is not very specific on that. All I can find about the implementation details of AbortSignal.any() is:

The static any(signals) method steps are to return the result of creating a dependent abort signal from signals using AbortSignal and the current realm.