From 961796b6100974f9dec7d7b0c2175722b31dd922 Mon Sep 17 00:00:00 2001 From: Alexander Shepel Date: Fri, 25 Feb 2022 15:50:51 +0300 Subject: [PATCH 1/3] Eliminate Farm work results leakage by clearing callback reference --- packages/jest-worker/src/Farm.ts | 7 +- .../src/__tests__/leak-integration.test.ts | 81 ++++++++++++++----- 2 files changed, 66 insertions(+), 22 deletions(-) diff --git a/packages/jest-worker/src/Farm.ts b/packages/jest-worker/src/Farm.ts index 9a8dcb20f9b8..e3e1a84870fd 100644 --- a/packages/jest-worker/src/Farm.ts +++ b/packages/jest-worker/src/Farm.ts @@ -127,9 +127,12 @@ export default class Farm { // Reference the task object outside so it won't be retained by onEnd, // and other properties of the task object, such as task.request can be // garbage collected. - const taskOnEnd = task.onEnd; + let taskOnEnd: OnEnd | null = task.onEnd; const onEnd: OnEnd = (error, result) => { - taskOnEnd(error, result); + if (taskOnEnd) { + taskOnEnd(error, result); + } + taskOnEnd = null; this._unlock(workerId); this._process(workerId); diff --git a/packages/jest-worker/src/__tests__/leak-integration.test.ts b/packages/jest-worker/src/__tests__/leak-integration.test.ts index f8e440e2d904..af5e827bc296 100644 --- a/packages/jest-worker/src/__tests__/leak-integration.test.ts +++ b/packages/jest-worker/src/__tests__/leak-integration.test.ts @@ -11,30 +11,71 @@ import {writeFileSync} from 'graceful-fs'; import LeakDetector from 'jest-leak-detector'; import {Worker} from '../../build/index'; -let workerFile!: string; -beforeAll(() => { - workerFile = join(tmpdir(), 'baz.js'); - writeFileSync(workerFile, 'module.exports.fn = () => {};'); -}); +describe('WorkerThreads leaks', () => { + let workerFile!: string; + beforeAll(() => { + workerFile = join(tmpdir(), 'baz.js'); + writeFileSync(workerFile, 'module.exports.fn = () => {};'); + }); -let worker!: Worker; -beforeEach(() => { - worker = new Worker(workerFile, { - enableWorkerThreads: true, - exposedMethods: ['fn'], + let worker!: Worker; + beforeEach(() => { + worker = new Worker(workerFile, { + enableWorkerThreads: true, + exposedMethods: ['fn'], + }); + }); + afterEach(async () => { + await worker.end(); + }); + + it('does not retain arguments after a task finished', async () => { + let leakDetector!: LeakDetector; + await new Promise((resolve, reject) => { + const obj = {}; + leakDetector = new LeakDetector(obj); + (worker as any).fn(obj).then(resolve, reject); + }); + + expect(await leakDetector.isLeaking()).toBe(false); }); }); -afterEach(async () => { - await worker.end(); -}); -it('does not retain arguments after a task finished', async () => { - let leakDetector!: LeakDetector; - await new Promise((resolve, reject) => { - const obj = {}; - leakDetector = new LeakDetector(obj); - (worker as any).fn(obj).then(resolve, reject); +describe('Worker leaks', () => { + let workerFile!: string; + beforeAll(() => { + workerFile = join(tmpdir(), 'baz.js'); + writeFileSync(workerFile, 'module.exports.fn = (obj) => [obj];'); + }); + + let worker!: Worker; + beforeEach(() => { + worker = new Worker(workerFile, { + enableWorkerThreads: false, + exposedMethods: ['fn'], + }); }); + afterEach(async () => { + await worker.end(); + }); + + it('does not retain result after next task call', async () => { + let leakDetector!: LeakDetector; + await new Promise((resolve, reject) => { + const obj = {}; + (worker as any) + .fn(obj) + .then((result: unknown) => { + leakDetector = new LeakDetector(result); + return result; + }) + .then(resolve, reject); + }); + await new Promise((resolve, reject) => { + const obj = {}; + (worker as any).fn(obj).then(resolve, reject); + }); - expect(await leakDetector.isLeaking()).toBe(false); + expect(await leakDetector.isLeaking()).toBe(false); + }); }); From 49549ac43739f849d207e2a733434e521fafb22a Mon Sep 17 00:00:00 2001 From: Alexander Shepel Date: Fri, 25 Feb 2022 16:12:56 +0300 Subject: [PATCH 2/3] Changelog updated --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4575a85de209..755b251ff514 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ - `[jest-jasmine2, jest-types]` [**BREAKING**] Move all `jasmine` specific types from `@jest/types` to its own package ([#12125](https://github.com/facebook/jest/pull/12125)) - `[jest-matcher-utils]` Pass maxWidth to `pretty-format` to avoid printing every element in arrays by default ([#12402](https://github.com/facebook/jest/pull/12402)) - `[jest-mock]` Fix function overloads for `spyOn` to allow more correct type inference in complex object ([#12442](https://github.com/facebook/jest/pull/12442)) +- `[jest-worker]` Fix Farm execution results leakage ([#12497](https://github.com/facebook/jest/pull/12497)) ### Chore & Maintenance From 6a8fc6a75505ed3a5cd25796ff63fcd6dd64275e Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Fri, 25 Feb 2022 14:25:45 +0100 Subject: [PATCH 3/3] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eea725d9620f..b2a3e63449ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,7 @@ - `[jest-matcher-utils]` Pass maxWidth to `pretty-format` to avoid printing every element in arrays by default ([#12402](https://github.com/facebook/jest/pull/12402)) - `[jest-mock]` Fix function overloads for `spyOn` to allow more correct type inference in complex object ([#12442](https://github.com/facebook/jest/pull/12442)) - `[jest-reporters]` Notifications generated by the `--notify` flag are no longer persistent in GNOME Shell. ([#11733](https://github.com/facebook/jest/pull/11733)) -- `[jest-worker]` Fix Farm execution results leakage ([#12497](https://github.com/facebook/jest/pull/12497)) +- `[jest-worker]` Fix `Farm` execution results memory leak ([#12497](https://github.com/facebook/jest/pull/12497)) ### Chore & Maintenance