Skip to content

Commit

Permalink
Fix resource leak by removing abort event listeners on destroy
Browse files Browse the repository at this point in the history
Not sure that this fix is entirely correct because I may have missed clean
up points. Also, needs a test still.

Fixes #2160
  • Loading branch information
marcelmeulemans committed Oct 12, 2022
1 parent 623229f commit 49ef634
Showing 1 changed file with 18 additions and 8 deletions.
26 changes: 18 additions & 8 deletions source/core/index.ts
Expand Up @@ -176,6 +176,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
private _triggerRead: boolean;
declare private _jobs: Array<() => void>;
private _cancelTimeouts: () => void;
private readonly _removeListeners: () => void;
private _nativeResponse?: IncomingMessageWithTimings;
private _flushed: boolean;
private _aborted: boolean;
Expand All @@ -199,6 +200,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
this._unproxyEvents = noop;
this._triggerRead = false;
this._cancelTimeouts = noop;
this._removeListeners = noop;
this._jobs = [];
this._flushed = false;
this._requestInitialized = false;
Expand Down Expand Up @@ -247,14 +249,6 @@ export default class Request extends Duplex implements RequestEvents<Request> {
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;
Expand All @@ -271,6 +265,21 @@ export default class Request extends Duplex implements RequestEvents<Request> {
}
});
}

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() {
Expand Down Expand Up @@ -508,6 +517,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
// Prevent further retries
this._stopRetry();
this._cancelTimeouts();
this._removeListeners();

if (this.options) {
const {body} = this.options;
Expand Down

0 comments on commit 49ef634

Please sign in to comment.