From 49ef634f860868d752a47a6818ff60d3f9d1b4d4 Mon Sep 17 00:00:00 2001 From: Marcel Meulemans Date: Mon, 10 Oct 2022 21:38:04 +1300 Subject: [PATCH] Fix resource leak by removing abort event listeners on destroy Not sure that this fix is entirely correct because I may have missed clean up points. Also, needs a test still. Fixes #2160 --- source/core/index.ts | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/source/core/index.ts b/source/core/index.ts index 0a054fd64..bb3c59a57 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -176,6 +176,7 @@ export default class Request extends Duplex implements RequestEvents { private _triggerRead: boolean; declare private _jobs: Array<() => void>; private _cancelTimeouts: () => void; + private readonly _removeListeners: () => void; private _nativeResponse?: IncomingMessageWithTimings; private _flushed: boolean; private _aborted: boolean; @@ -199,6 +200,7 @@ export default class Request extends Duplex implements RequestEvents { this._unproxyEvents = noop; this._triggerRead = false; this._cancelTimeouts = noop; + this._removeListeners = noop; this._jobs = []; this._flushed = false; this._requestInitialized = false; @@ -247,14 +249,6 @@ export default class Request extends Duplex implements RequestEvents { return; } - if (this.options.signal?.aborted) { - this.destroy(new AbortError(this)); - } - - this.options.signal?.addEventListener('abort', () => { - this.destroy(new AbortError(this)); - }); - // Important! If you replace `body` in a handler with another stream, make sure it's readable first. // The below is run only once. const {body} = this.options; @@ -271,6 +265,21 @@ export default class Request extends Duplex implements RequestEvents { } }); } + + if (this.options.signal) { + const abort = () => { + this.destroy(new AbortError(this)); + }; + + if (this.options.signal.aborted) { + abort(); + } else { + this.options.signal.addEventListener('abort', abort); + this._removeListeners = () => { + this.options.signal.removeEventListener('abort', abort); + }; + } + } } async flush() { @@ -508,6 +517,7 @@ export default class Request extends Duplex implements RequestEvents { // Prevent further retries this._stopRetry(); this._cancelTimeouts(); + this._removeListeners(); if (this.options) { const {body} = this.options;