Skip to content

Commit e48386e

Browse files
sebinsuasholladay
andauthored
Fix memory leak caused by shared abort signals (#683)
Co-authored-by: Seth Holladay <[email protected]>
1 parent b49cd03 commit e48386e

File tree

3 files changed

+39
-9
lines changed

3 files changed

+39
-9
lines changed

package.json

+1
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
"delay": "^6.0.0",
6868
"expect-type": "^0.19.0",
6969
"express": "^4.18.2",
70+
"jest-leak-detector": "^29.7.0",
7071
"pify": "^6.1.0",
7172
"playwright": "^1.45.3",
7273
"raw-body": "^2.5.2",

source/core/Ky.ts

+2-9
Original file line numberDiff line numberDiff line change
@@ -168,16 +168,9 @@ export class Ky {
168168
}
169169

170170
if (supportsAbortController) {
171-
this.abortController = new globalThis.AbortController();
172171
const originalSignal = this._options.signal ?? (this._input as Request).signal;
173-
if (originalSignal?.aborted) {
174-
this.abortController.abort(originalSignal?.reason);
175-
}
176-
177-
originalSignal?.addEventListener('abort', () => {
178-
this.abortController!.abort(originalSignal.reason);
179-
});
180-
this._options.signal = this.abortController.signal;
172+
this.abortController = new globalThis.AbortController();
173+
this._options.signal = originalSignal ? AbortSignal.any([originalSignal, this.abortController.signal]) : this.abortController.signal;
181174
}
182175

183176
if (supportsRequestStreams) {

test/memory-leak.ts

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import test from 'ava';
2+
import _LeakDetector from 'jest-leak-detector';
3+
import ky, {type KyInstance} from '../source/index.js';
4+
import {createHttpTestServer} from './helpers/create-http-test-server.js';
5+
6+
const LeakDetector = _LeakDetector.default as typeof _LeakDetector;
7+
8+
test('shared abort signal must not cause memory leak of input', async t => {
9+
const server = await createHttpTestServer();
10+
server.get('/', (_request, response) => {
11+
response.end('ok');
12+
});
13+
14+
async function isKyLeaking(api: KyInstance) {
15+
let url: URL | undefined = new URL(
16+
`${server.url.toString()}?id=${Math.random()}`,
17+
);
18+
const detector = new LeakDetector(url);
19+
20+
await api.get(url);
21+
22+
url = undefined;
23+
24+
return detector.isLeaking();
25+
}
26+
27+
const abortController = new AbortController();
28+
29+
try {
30+
t.false(await isKyLeaking(ky.extend({})));
31+
t.false(await isKyLeaking(ky.extend({signal: abortController.signal})));
32+
} finally {
33+
abortController.abort();
34+
await server.close();
35+
}
36+
});

0 commit comments

Comments
 (0)