Open jvaill opened 1 week ago
@KhafraDev
Therefore undici seems to be working properly? Since the parent AbortSignal is aborted, the dependent signal (ie. the one created for every instance of Request) can be garbage collected.
@KhafraDev - I would expect the dependent signal to also be aborted after the parent signal is aborted & before it is garbage collected. Currently, the dependent signal seems to stop working when the request itself is garbage collected, not when its parent signal is garbage collected. Perhaps I'm missing something?
Currently when a Request is collected, the signal is removed (see https://github.com/nodejs/undici/pull/1113)
must not be garbage collected while its source signals is non-empty and it has registered event listeners for its abort event or its abort algorithms is non-empty.
There is a bug here - we shouldn't be removing the signal if there are registered 'abort' event listeners. I will have a PR in a few hours unless the approach I'm thinking of doesn't work.
I believe this is a bug in node -
const ac = new AbortController();
ac.signal.addEventListener("abort", () => {
console.log("ac signal aborted");
});
const request = {
signal: (() => {
const ourAc = new AbortController()
ourAc.signal.addEventListener('abort', () => console.log('called'), { once: true })
return ourAc.signal
})()
}
request.signal.addEventListener("abort", () => {
console.log("request signal aborted");
});
setTimeout(() => {
global.gc();
ac.abort();
}, 0);
Only ac signal aborted
is printed.
@benjamingr
cc @nodejs/web-standards
Bug Description
Passing a signal to Request() and aborting it does not cause the underlying request signal to be aborted after it's been garbage collected. This leads to issues where you want to listen on the request signal even after the request itself falls out of scope - e.g., the request is instantiated with an abort controller signal and downstream code is waiting for that signal to be aborted via
request.signal
.Reproducible By
Run the following using
node --expose-gc main.js
:Expected Behavior
ac signal aborted
andrequest signal aborted
should be logged to the console.Instead, only
ac signal aborted
is logged.Environment
Latest node and undici. Tried it in some older versions as well.
Additional context
A few folks are running into this while trying to close event stream requests in the remix web framework. The underlying requests are never closed because the signal doesn't get aborted.