Skip to content

Commit 427af3b

Browse files
shudingjeysal
andauthored
Fix memory leaks of jest-worker (#11187)
Co-authored-by: Tim Seckinger <[email protected]>
1 parent 33c3b07 commit 427af3b

File tree

7 files changed

+67
-6
lines changed

7 files changed

+67
-6
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@
103103
- `[jest-resolve]` Cache reading and parsing of `package.json`s ([#11076](https://github.com/facebook/jest/pull/11076))
104104
- `[jest-runtime, jest-transform]` share `cacheFS` between runtime and transformer ([#10901](https://github.com/facebook/jest/pull/10901))
105105
- `[jest-runtime]` Load `chalk` only once per worker ([#10864](https://github.com/facebook/jest/pull/10864))
106+
- `[jest-worker]` Fix memory leak of previous task arguments while no new task is scheduled ([#11187](https://github.com/facebook/jest/pull/11187))
106107

107108
## 26.6.3
108109

packages/jest-worker/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
"@types/merge-stream": "^1.1.2",
2323
"@types/supports-color": "^7.2.0",
2424
"get-stream": "^6.0.0",
25+
"jest-leak-detector": "^27.0.0-next.3",
2526
"worker-farm": "^1.6.0"
2627
},
2728
"engines": {

packages/jest-worker/src/Farm.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,14 @@ export default class Farm {
6666
};
6767

6868
const promise: PromiseWithCustomMessage<unknown> = new Promise(
69-
(resolve, reject) => {
69+
// Bind args to this function so it won't reference to the parent scope.
70+
// This prevents a memory leak in v8, because otherwise the function will
71+
// retaine args for the closure.
72+
((
73+
args: Array<unknown>,
74+
resolve: (value: unknown) => void,
75+
reject: (reason?: any) => void,
76+
) => {
7077
const computeWorkerKey = this._computeWorkerKey;
7178
const request: ChildMessage = [CHILD_MESSAGE_CALL, false, method, args];
7279

@@ -101,7 +108,7 @@ export default class Farm {
101108
} else {
102109
this._push(task);
103110
}
104-
},
111+
}).bind(null, args),
105112
);
106113

107114
promise.UNSTABLE_onCustomMessage = addCustomMessageListener;
@@ -124,8 +131,12 @@ export default class Farm {
124131
throw new Error('Queue implementation returned processed task');
125132
}
126133

134+
// Reference the task object outside so it won't be retained by onEnd,
135+
// and other properties of the task object, such as task.request can be
136+
// garbage collected.
137+
const taskOnEnd = task.onEnd;
127138
const onEnd = (error: Error | null, result: unknown) => {
128-
task.onEnd(error, result);
139+
taskOnEnd(error, result);
129140

130141
this._unlock(workerId);
131142
this._process(workerId);
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
import {tmpdir} from 'os';
9+
import {join} from 'path';
10+
import {writeFileSync} from 'graceful-fs';
11+
import LeakDetector from 'jest-leak-detector';
12+
import {Worker} from '../..';
13+
14+
let workerFile!: string;
15+
beforeAll(() => {
16+
workerFile = join(tmpdir(), 'baz.js');
17+
writeFileSync(workerFile, `module.exports.fn = () => {};`);
18+
});
19+
20+
let worker!: Worker;
21+
beforeEach(() => {
22+
worker = new Worker(workerFile, {
23+
enableWorkerThreads: true,
24+
exposedMethods: ['fn'],
25+
});
26+
});
27+
afterEach(async () => {
28+
await worker.end();
29+
});
30+
31+
it('does not retain arguments after a task finished', async () => {
32+
let leakDetector!: LeakDetector;
33+
await new Promise(resolve => {
34+
const obj = {};
35+
leakDetector = new LeakDetector(obj);
36+
(worker as any).fn(obj).then(resolve);
37+
});
38+
39+
expect(await leakDetector.isLeaking()).toBe(false);
40+
});

packages/jest-worker/src/workers/NodeThreadsWorker.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,15 +206,21 @@ export default class ExperimentalWorker implements WorkerInterface {
206206
send(
207207
request: ChildMessage,
208208
onProcessStart: OnStart,
209-
onProcessEnd: OnEnd,
209+
onProcessEnd: OnEnd | null,
210210
onCustomMessage: OnCustomMessage,
211211
): void {
212212
onProcessStart(this);
213213
this._onProcessEnd = (...args) => {
214214
// Clean the request to avoid sending past requests to workers that fail
215215
// while waiting for a new request (timers, unhandled rejections...)
216216
this._request = null;
217-
return onProcessEnd(...args);
217+
218+
const res = onProcessEnd?.(...args);
219+
220+
// Clean up the reference so related closures can be garbage collected.
221+
onProcessEnd = null;
222+
223+
return res;
218224
};
219225

220226
this._onCustomMessage = (...arg) => onCustomMessage(...arg);

packages/jest-worker/tsconfig.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,6 @@
33
"compilerOptions": {
44
"rootDir": "src",
55
"outDir": "build"
6-
}
6+
},
7+
"references": [{"path": "../jest-leak-detector"}]
78
}

yarn.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14137,6 +14137,7 @@ fsevents@^1.2.7:
1413714137
"@types/node": "*"
1413814138
"@types/supports-color": ^7.2.0
1413914139
get-stream: ^6.0.0
14140+
jest-leak-detector: ^27.0.0-next.3
1414014141
merge-stream: ^2.0.0
1414114142
supports-color: ^8.0.0
1414214143
worker-farm: ^1.6.0

0 commit comments

Comments
 (0)