Skip to content

Commit 49ef634

Browse files
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
1 parent 623229f commit 49ef634

File tree

1 file changed

+18
-8
lines changed

1 file changed

+18
-8
lines changed

source/core/index.ts

+18-8
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
176176
private _triggerRead: boolean;
177177
declare private _jobs: Array<() => void>;
178178
private _cancelTimeouts: () => void;
179+
private readonly _removeListeners: () => void;
179180
private _nativeResponse?: IncomingMessageWithTimings;
180181
private _flushed: boolean;
181182
private _aborted: boolean;
@@ -199,6 +200,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
199200
this._unproxyEvents = noop;
200201
this._triggerRead = false;
201202
this._cancelTimeouts = noop;
203+
this._removeListeners = noop;
202204
this._jobs = [];
203205
this._flushed = false;
204206
this._requestInitialized = false;
@@ -247,14 +249,6 @@ export default class Request extends Duplex implements RequestEvents<Request> {
247249
return;
248250
}
249251

250-
if (this.options.signal?.aborted) {
251-
this.destroy(new AbortError(this));
252-
}
253-
254-
this.options.signal?.addEventListener('abort', () => {
255-
this.destroy(new AbortError(this));
256-
});
257-
258252
// Important! If you replace `body` in a handler with another stream, make sure it's readable first.
259253
// The below is run only once.
260254
const {body} = this.options;
@@ -271,6 +265,21 @@ export default class Request extends Duplex implements RequestEvents<Request> {
271265
}
272266
});
273267
}
268+
269+
if (this.options.signal) {
270+
const abort = () => {
271+
this.destroy(new AbortError(this));
272+
};
273+
274+
if (this.options.signal.aborted) {
275+
abort();
276+
} else {
277+
this.options.signal.addEventListener('abort', abort);
278+
this._removeListeners = () => {
279+
this.options.signal.removeEventListener('abort', abort);
280+
};
281+
}
282+
}
274283
}
275284

276285
async flush() {
@@ -508,6 +517,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
508517
// Prevent further retries
509518
this._stopRetry();
510519
this._cancelTimeouts();
520+
this._removeListeners();
511521

512522
if (this.options) {
513523
const {body} = this.options;

0 commit comments

Comments
 (0)