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 10, 2022
1 parent 623229f commit 1bc4982
Showing 1 changed file with 16 additions and 6 deletions.
22 changes: 16 additions & 6 deletions source/core/index.ts
Original file line number Diff line number Diff line change
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,13 +249,20 @@ export default class Request extends Duplex implements RequestEvents<Request> {
return;
}

if (this.options.signal?.aborted) {
this.destroy(new AbortError(this));
}
if (this.options.signal) {
if (this.options.signal.aborted) {
this.destroy(new AbortError(this));
}

this.options.signal?.addEventListener('abort', () => {
this.destroy(new AbortError(this));
});
const abort = () => {
this.destroy(new AbortError(this));
};

this.options.signal.addEventListener('abort', abort);
this._removeListeners = () => {
this.options.signal.removeEventListener('abort', abort);
};
}

// Important! If you replace `body` in a handler with another stream, make sure it's readable first.
// The below is run only once.
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 1bc4982

Please sign in to comment.