AbortSignal.any memory leak when circular dependencies

[CGNonofr]

Version

23.10.0

Platform

6.8.0-55-generic #57-Ubuntu SMP PREEMPT_DYNAMIC Wed Feb 12 23:42:21 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

class Test {
  private abortController = new AbortController()

  public test(cb: (abortSignal: AbortSignal) => void, abortSignal: AbortSignal) {
    const signal = AbortSignal.any([abortSignal, this.abortController.signal])

    cb(signal)
  }
}

const registry = new FinalizationRegistry((heldValue) => {
  console.log(`${heldValue} has been collected`)
})

;(() => {
  const test = new Test()
  registry.register(test, 'test')

  const abortController = new AbortController()

  test.test((abortSignal) => {
    abortSignal.addEventListener('abort', () => {
      console.log(test)
    })
  }, abortController.signal)
})()

global.gc?.()

setTimeout(() => {
  console.log('the end')
}, 2000)

run with node --expose-gc test.ts

How often does it reproduce? Is there a required condition?

Every time

What is the expected behavior? Why is that the expected behavior?

the output should be

test has been collected
the end

because the test object is not referenced anymore

What do you see instead?

output:

the end

the test object is not garbage collected

Additional information

The issues seems to be caused by the cyclic dependency: test object => abortController field => abortSignal => AbortSignal.any result => cb function => test object

AbortSignal.any seems to prevent node from garbage collecting them all at once

[jakecastelli]

@jazelly and myself have spent some time digging into this issue.

We found that when a new signal is created using AbortSignal.any it will internally create a weak ref

node/lib/internal/abort_controller.js

Line 262 in e57841f

const resultSignalWeakRef = new SafeWeakRef(resultSignal);

When you attempt to hold a reference by

console.log(test)

It would prevent each other from being collected.

Couldn't say this is a bug but happy to hear your thoughts and discuss further.

[CGNonofr]

Couldn't say this is a bug

I fail to understand how it couldn't be considered as a bug 🤔 I had a big memory leak in my app, spent hours investigating the issue, found this, had to workaround it.

It's completely unpredictable and leads to a memory leak

[gurgunday]

This is not a bug?

In your example, you never actually abort the signal, why would it get gc'd?

[CGNonofr]

I'm not sure to understand what the gc has to do with aborting the signal 🤔 aborting a signal is an exception, not the rule

My point is: I have a legit function, that doesn't assign anything globally, and just because it needs to be abortable, there is no way to prevent it from leaking every time it's called.

Do you think there is anything wrong with the code I've provided? because it was making my app go OOM in less than half a day, with nothing obvious to me about what's wrong

[gurgunday]

Oh I see the problem, sorry, I was already working on an issue about AbortSignal.any and thought this was linked to it 😆

So yeah I think this is being caused by any adding the resulting signal to gcPersistentSignals, and it can't detect that it's out of scope

[jakecastelli]

I am interested to learn what your work around is if you're willing to share of course.

What my previous comment was trying to say that nodejs internally uses weakRef to hold a reference of the signal and when you have a circular dep (at least the V8 engine) it would give up on gc, you can reproduce it with just a WeakRef and circular deps without using AbortSignal. Unlike other circular deps (that does not evlove WeakRef that V8 can detect and reclaim the memory.

I am also interested to see how other runtimes are dealing with this issue (e.g. deno, bun, chrome etc.), if they all have this memory issue, perhaps we should seek some help from the whatwg.

[CGNonofr]

at the end, my workaround is to use a simpler custom version of the AbortSignal.any:

function abortSignalAny(signals: AbortSignal[]): AbortSignal {
  return {
    get aborted() {
      return signals.some((s) => s.aborted)
    },
    addEventListener(type: 'abort', callback, options) {
      for (const signal of signals) {
        signal.addEventListener(type, callback, options)
      }
    },
    removeEventListener(type: 'abort', callback, options) {
      for (const signal of signals) {
        signal.removeEventListener(type, callback, options)
      }
    },
    dispatchEvent() {
      throw new Error('Unsupported')
    },
    onabort() {
      throw new Error('Unsupported')
    },
    get reason() {
      return signals.find((s) => s.reason != null)?.reason
    },
    throwIfAborted() {
      for (const signal of signals) {
        signal.throwIfAborted()
      }
    }
  }
}

There are some downsides, which are not really an issue for me though:

• signal.reason (and throwIfAborted) returns the reason of the first signal in the list, instead of the first triggered signal
• probably others?

[alexey-sh]

Does it mean that removing abort stuff from an app would fix unexpected memory growing?

[CGNonofr]

YES