From 71d0080375f6b5d5612852d2cf2d49ea0088354f Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Mon, 8 Aug 2022 23:18:30 +0100 Subject: [PATCH 01/52] test: ad integration test case for worker not executing test --- e2e/worker-restarting/__tests__/test1.js | 3 +++ e2e/worker-restarting/__tests__/test2.js | 3 +++ e2e/worker-restarting/__tests__/test3.js | 3 +++ e2e/worker-restarting/package.json | 6 ++++++ packages/jest-config/src/index.ts | 1 + 5 files changed, 16 insertions(+) create mode 100644 e2e/worker-restarting/__tests__/test1.js create mode 100644 e2e/worker-restarting/__tests__/test2.js create mode 100644 e2e/worker-restarting/__tests__/test3.js create mode 100644 e2e/worker-restarting/package.json diff --git a/e2e/worker-restarting/__tests__/test1.js b/e2e/worker-restarting/__tests__/test1.js new file mode 100644 index 000000000000..36b84acd0947 --- /dev/null +++ b/e2e/worker-restarting/__tests__/test1.js @@ -0,0 +1,3 @@ +test('basic test', () => { + expect(true).toBeTruthy(); +}); diff --git a/e2e/worker-restarting/__tests__/test2.js b/e2e/worker-restarting/__tests__/test2.js new file mode 100644 index 000000000000..36b84acd0947 --- /dev/null +++ b/e2e/worker-restarting/__tests__/test2.js @@ -0,0 +1,3 @@ +test('basic test', () => { + expect(true).toBeTruthy(); +}); diff --git a/e2e/worker-restarting/__tests__/test3.js b/e2e/worker-restarting/__tests__/test3.js new file mode 100644 index 000000000000..36b84acd0947 --- /dev/null +++ b/e2e/worker-restarting/__tests__/test3.js @@ -0,0 +1,3 @@ +test('basic test', () => { + expect(true).toBeTruthy(); +}); diff --git a/e2e/worker-restarting/package.json b/e2e/worker-restarting/package.json new file mode 100644 index 000000000000..3dc2bb57749f --- /dev/null +++ b/e2e/worker-restarting/package.json @@ -0,0 +1,6 @@ +{ + "jest": { + "maxWorkers": 2, + "workerIdleMemoryLimit": "1MB" + } +} diff --git a/packages/jest-config/src/index.ts b/packages/jest-config/src/index.ts index 5728f473e5cc..ba9afa5e100b 100644 --- a/packages/jest-config/src/index.ts +++ b/packages/jest-config/src/index.ts @@ -171,6 +171,7 @@ const groupOptions = ( watchAll: options.watchAll, watchPlugins: options.watchPlugins, watchman: options.watchman, + workerIdleMemoryLimit: options.workerIdleMemoryLimit, }), projectConfig: Object.freeze({ automock: options.automock, From 8c6ab73e888ac4781159ec26d743f7fa1c550007 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 14:02:32 +0100 Subject: [PATCH 02/52] test: add test case for worker being killed just once --- packages/jest-worker/src/types.ts | 1 + .../jest-worker/src/workers/ChildProcessWorker.ts | 6 +++++- .../jest-worker/src/workers/NodeThreadsWorker.ts | 4 ++++ .../src/workers/__tests__/WorkerEdgeCases.test.js | 14 +++++++++----- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/jest-worker/src/types.ts b/packages/jest-worker/src/types.ts index 302771e10b0a..61c7014bab87 100644 --- a/packages/jest-worker/src/types.ts +++ b/packages/jest-worker/src/types.ts @@ -83,6 +83,7 @@ export interface WorkerInterface { */ getWorkerSystemId(): number; getMemoryUsage(): Promise; + isWorkerRunning(): boolean; } export type PoolExitResult = { diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 4ef4d51b87cc..c6f281539db3 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -33,7 +33,7 @@ const SIGKILL_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 9; const SIGTERM_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 15; // How long to wait after SIGTERM before sending SIGKILL -const SIGKILL_DELAY = 500; +export const SIGKILL_DELAY = 500; /** * This class wraps the child process and provides a nice interface to @@ -466,6 +466,10 @@ export default class ChildProcessWorker implements WorkerInterface { } } + isWorkerRunning(): boolean { + return this._child.connected && !this._child.killed; + } + private _getFakeStream() { if (!this._fakeStream) { this._fakeStream = new PassThrough(); diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index ad1bb822cd56..33d0d2585e07 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -410,4 +410,8 @@ export default class ExperimentalWorker implements WorkerInterface { } return this._fakeStream; } + + isWorkerRunning(): boolean { + return !!this._worker.threadId; + } } diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 8005048f48f4..145f841f66c9 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -14,13 +14,9 @@ import { WorkerInterface, WorkerOptions, } from '../../types'; -import ChildProcessWorker from '../ChildProcessWorker'; +import ChildProcessWorker, {SIGKILL_DELAY} from '../ChildProcessWorker'; import ThreadsWorker from '../NodeThreadsWorker'; -// These tests appear to be slow/flaky. Allowing it to retry quite a few times -// will cut down on this noise and they're fast tests anyway. -jest.retryTimes(30); - const root = join('../../'); const filesToBuild = ['workers/processChild', 'workers/threadChild', 'types']; const writeDestination = join(__dirname, '__temp__'); @@ -169,6 +165,14 @@ describe.each([ const endPid = worker.getWorkerSystemId(); expect(endPid).toBeGreaterThanOrEqual(0); expect(endPid).not.toEqual(startPid); + expect(worker.isWorkerRunning()).toBeTruthy(); + + await new Promise(resolve => { + setTimeout(resolve, SIGKILL_DELAY + 100); + }); + + expect(worker.isWorkerRunning()).toBeTruthy(); + expect(worker.getWorkerSystemId()).toEqual(endPid); }, 10000); test('should cleanly exit on crash', async () => { From 86acdf155dce25de8f33b65a5093ef69c157365b Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 14:07:15 +0100 Subject: [PATCH 03/52] fix: worker being killed after spawning --- packages/jest-worker/src/workers/ChildProcessWorker.ts | 8 ++++++-- packages/jest-worker/src/workers/NodeThreadsWorker.ts | 2 +- .../src/workers/__tests__/WorkerEdgeCases.test.js | 7 ++++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index c6f281539db3..a9944249d85e 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -377,8 +377,12 @@ export default class ChildProcessWorker implements WorkerInterface { } killChild(): NodeJS.Timeout { - this._child.kill('SIGTERM'); - return setTimeout(() => this._child.kill('SIGKILL'), SIGKILL_DELAY); + // We store a reference so that there's no way we can accidentally + // kill a new worker that has been spawned. + const childToKill = this._child; + + childToKill.kill('SIGTERM'); + return setTimeout(() => childToKill.kill('SIGKILL'), SIGKILL_DELAY); } forceExit(): void { diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 33d0d2585e07..64276fba1b75 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -412,6 +412,6 @@ export default class ExperimentalWorker implements WorkerInterface { } isWorkerRunning(): boolean { - return !!this._worker.threadId; + return this._worker.threadId >= 0; } } diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 145f841f66c9..53cc06b0a35b 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -134,6 +134,12 @@ describe.each([ const systemId = worker.getWorkerSystemId(); expect(systemId).toBeGreaterThanOrEqual(0); expect(systemId).not.toEqual(startSystemId); + + await new Promise(resolve => { + setTimeout(resolve, SIGKILL_DELAY + 100); + }); + + expect(worker.isWorkerRunning()).toBeTruthy(); }); test('should automatically recycle on idle limit breach', async () => { @@ -172,7 +178,6 @@ describe.each([ }); expect(worker.isWorkerRunning()).toBeTruthy(); - expect(worker.getWorkerSystemId()).toEqual(endPid); }, 10000); test('should cleanly exit on crash', async () => { From de0c0e8047ed3ead553302177f4a7af730955e71 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 14:41:51 +0100 Subject: [PATCH 04/52] fix: missing license --- e2e/worker-restarting/__tests__/test1.js | 7 +++++++ e2e/worker-restarting/__tests__/test2.js | 7 +++++++ e2e/worker-restarting/__tests__/test3.js | 7 +++++++ 3 files changed, 21 insertions(+) diff --git a/e2e/worker-restarting/__tests__/test1.js b/e2e/worker-restarting/__tests__/test1.js index 36b84acd0947..3e394ec4ec61 100644 --- a/e2e/worker-restarting/__tests__/test1.js +++ b/e2e/worker-restarting/__tests__/test1.js @@ -1,3 +1,10 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + test('basic test', () => { expect(true).toBeTruthy(); }); diff --git a/e2e/worker-restarting/__tests__/test2.js b/e2e/worker-restarting/__tests__/test2.js index 36b84acd0947..3e394ec4ec61 100644 --- a/e2e/worker-restarting/__tests__/test2.js +++ b/e2e/worker-restarting/__tests__/test2.js @@ -1,3 +1,10 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + test('basic test', () => { expect(true).toBeTruthy(); }); diff --git a/e2e/worker-restarting/__tests__/test3.js b/e2e/worker-restarting/__tests__/test3.js index 36b84acd0947..3e394ec4ec61 100644 --- a/e2e/worker-restarting/__tests__/test3.js +++ b/e2e/worker-restarting/__tests__/test3.js @@ -1,3 +1,10 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + test('basic test', () => { expect(true).toBeTruthy(); }); From d83aa84ff6d9455c4f69d7a5d1af81324388e082 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 15:27:52 +0100 Subject: [PATCH 05/52] fix: flaky tests --- packages/jest-worker/src/types.ts | 11 ++++++ .../src/workers/ChildProcessWorker.ts | 34 +++++++++++++++++++ .../src/workers/NodeThreadsWorker.ts | 34 +++++++++++++++++++ .../workers/__tests__/WorkerEdgeCases.test.js | 9 +++++ 4 files changed, 88 insertions(+) diff --git a/packages/jest-worker/src/types.ts b/packages/jest-worker/src/types.ts index 61c7014bab87..e4a0a357b503 100644 --- a/packages/jest-worker/src/types.ts +++ b/packages/jest-worker/src/types.ts @@ -83,7 +83,18 @@ export interface WorkerInterface { */ getWorkerSystemId(): number; getMemoryUsage(): Promise; + /** + * Checks to see if the child worker is actually running. + */ isWorkerRunning(): boolean; + /** + * When the worker child is started and ready to start handling requests. + * + * @remarks + * This mostly exists to help with testing so that you don't check the status + * of things like isWorkerRunning before it actually is. + */ + waitForWorkerReady(): Promise; } export type PoolExitResult = { diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index a9944249d85e..ffae02c27109 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -72,6 +72,9 @@ export default class ChildProcessWorker implements WorkerInterface { private _memoryUsagePromise: Promise | undefined; private _resolveMemoryUsage: ((arg0: number) => void) | undefined; + private _workerReadyPromise: Promise | undefined; + private _resolveWorkerReady: (() => void) | undefined; + private _childIdleMemoryUsage: number | null; private _childIdleMemoryUsageLimit: number | null; private _memoryUsageCheck = false; @@ -189,6 +192,10 @@ export default class ChildProcessWorker implements WorkerInterface { // Clear the request so we don't keep executing it. this._request = null; } + + if (this._resolveWorkerReady) { + this._resolveWorkerReady(); + } } private _detectOutOfMemoryCrash(child: ChildProcess): void { @@ -319,6 +326,9 @@ export default class ChildProcessWorker implements WorkerInterface { } private _onExit(exitCode: number | null) { + this._workerReadyPromise = undefined; + this._resolveWorkerReady = undefined; + if (exitCode !== 0 && this._state === WorkerStates.OUT_OF_MEMORY) { this._onProcessEnd( new Error('Jest worker ran out of memory and crashed'), @@ -474,6 +484,30 @@ export default class ChildProcessWorker implements WorkerInterface { return this._child.connected && !this._child.killed; } + waitForWorkerReady(): Promise { + if (!this._workerReadyPromise) { + this._workerReadyPromise = new Promise((resolve, reject) => { + if ( + this._state === WorkerStates.OUT_OF_MEMORY || + this._state === WorkerStates.SHUTTING_DOWN || + this._state === WorkerStates.SHUT_DOWN + ) { + reject( + new Error( + `Worker state means it will never be ready: ${this._state}`, + ), + ); + } else if (this.isWorkerRunning()) { + resolve(); + } else { + this._resolveWorkerReady = resolve; + } + }); + } + + return this._workerReadyPromise; + } + private _getFakeStream() { if (!this._fakeStream) { this._fakeStream = new PassThrough(); diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 64276fba1b75..ee585b4bf62c 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -46,6 +46,9 @@ export default class ExperimentalWorker implements WorkerInterface { private _memoryUsagePromise: Promise | undefined; private _resolveMemoryUsage: ((arg0: number) => void) | undefined; + private _workerReadyPromise: Promise | undefined; + private _resolveWorkerReady: (() => void) | undefined; + private _childWorkerPath: string; private _childIdleMemoryUsage: number | null; @@ -157,6 +160,10 @@ export default class ExperimentalWorker implements WorkerInterface { {type: 'WorkerError'}, ]); } + + if (this._resolveWorkerReady) { + this._resolveWorkerReady(); + } } private _shutdown() { @@ -238,6 +245,9 @@ export default class ExperimentalWorker implements WorkerInterface { } private _onExit(exitCode: number) { + this._workerReadyPromise = undefined; + this._resolveWorkerReady = undefined; + if (exitCode !== 0 && this._state === WorkerStates.OUT_OF_MEMORY) { this._onProcessEnd( new Error('Jest worker ran out of memory and crashed'), @@ -404,6 +414,30 @@ export default class ExperimentalWorker implements WorkerInterface { return this._worker.threadId; } + waitForWorkerReady(): Promise { + if (!this._workerReadyPromise) { + this._workerReadyPromise = new Promise((resolve, reject) => { + if ( + this._state === WorkerStates.OUT_OF_MEMORY || + this._state === WorkerStates.SHUTTING_DOWN || + this._state === WorkerStates.SHUT_DOWN + ) { + reject( + new Error( + `Worker state means it will never be ready: ${this._state}`, + ), + ); + } else if (this.isWorkerRunning()) { + resolve(); + } else { + this._resolveWorkerReady = resolve; + } + }); + } + + return this._workerReadyPromise; + } + private _getFakeStream() { if (!this._fakeStream) { this._fakeStream = new PassThrough(); diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 53cc06b0a35b..01efa1cfd280 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -177,6 +177,8 @@ describe.each([ setTimeout(resolve, SIGKILL_DELAY + 100); }); + await worker.waitForWorkerReady(); + expect(worker.isWorkerRunning()).toBeTruthy(); }, 10000); @@ -222,6 +224,8 @@ describe.each([ ); await worker.waitForExit(); + + expect(worker.isWorkerRunning()).toBeFalsy(); }, 15000); test('should handle regular fatal crashes', async () => { @@ -282,7 +286,12 @@ describe.each([ // Expect the pids to be retries + 1 because it is restarted // one last time at the end ready for the next request. expect(pidChanges).toEqual(5); + expect(worker.isWorkerRunning()).toBeTruthy(); worker.forceExit(); + + await expect( + async () => await worker.waitForWorkerReady(), + ).rejects.toThrowError(); }); }); From 9435d0b863a2c9f1f417a979bc81c975f52b95bd Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 16:39:30 +0100 Subject: [PATCH 06/52] chore: add some diagnostic logging --- packages/jest-worker/src/types.ts | 1 + .../src/workers/ChildProcessWorker.ts | 4 ++ .../src/workers/NodeThreadsWorker.ts | 4 ++ .../workers/__tests__/WorkerEdgeCases.test.js | 42 ++++++++++++++++--- 4 files changed, 46 insertions(+), 5 deletions(-) diff --git a/packages/jest-worker/src/types.ts b/packages/jest-worker/src/types.ts index e4a0a357b503..34ec77709ff6 100644 --- a/packages/jest-worker/src/types.ts +++ b/packages/jest-worker/src/types.ts @@ -83,6 +83,7 @@ export interface WorkerInterface { */ getWorkerSystemId(): number; getMemoryUsage(): Promise; + getWorkerState(): WorkerStates; /** * Checks to see if the child worker is actually running. */ diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index ffae02c27109..51326d620478 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -514,4 +514,8 @@ export default class ChildProcessWorker implements WorkerInterface { } return this._fakeStream; } + + getWorkerState(): WorkerStates { + return this._state; + } } diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index ee585b4bf62c..7c05683abc5d 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -448,4 +448,8 @@ export default class ExperimentalWorker implements WorkerInterface { isWorkerRunning(): boolean { return this._worker.threadId >= 0; } + + getWorkerState(): WorkerStates { + return this._state; + } } diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 01efa1cfd280..57fe27690ac0 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -13,6 +13,7 @@ import { CHILD_MESSAGE_MEM_USAGE, WorkerInterface, WorkerOptions, + WorkerStates, } from '../../types'; import ChildProcessWorker, {SIGKILL_DELAY} from '../ChildProcessWorker'; import ThreadsWorker from '../NodeThreadsWorker'; @@ -59,11 +60,11 @@ describe.each([ workerClass: ChildProcessWorker, workerPath: processChildWorkerPath, }, - { - name: 'ThreadWorker', - workerClass: ThreadsWorker, - workerPath: threadChildWorkerPath, - }, + // { + // name: 'ThreadWorker', + // workerClass: ThreadsWorker, + // workerPath: threadChildWorkerPath, + // }, ])('$name', ({workerClass, workerPath}) => { /** @type WorkerInterface */ let worker; @@ -152,9 +153,13 @@ describe.each([ workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), }); + console.log('A'); + const startPid = worker.getWorkerSystemId(); expect(startPid).toBeGreaterThanOrEqual(0); + console.log('B'); + const onStart = jest.fn(); const onEnd = jest.fn(); const onCustom = jest.fn(); @@ -166,19 +171,29 @@ describe.each([ onCustom, ); + console.log('C'); + await waitForChange(() => worker.getWorkerSystemId()); + console.log('D'); + const endPid = worker.getWorkerSystemId(); expect(endPid).toBeGreaterThanOrEqual(0); expect(endPid).not.toEqual(startPid); expect(worker.isWorkerRunning()).toBeTruthy(); + console.log('E'); + await new Promise(resolve => { setTimeout(resolve, SIGKILL_DELAY + 100); }); + console.log('F'); + await worker.waitForWorkerReady(); + console.log('G'); + expect(worker.isWorkerRunning()).toBeTruthy(); }, 10000); @@ -193,6 +208,8 @@ describe.each([ workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), }; + console.time('1'); + if (workerClass === ThreadsWorker) { options.resourceLimits = { codeRangeSizeMb: workerHeapLimit * 2, @@ -216,6 +233,9 @@ describe.each([ const onEnd = jest.fn(); const onCustom = jest.fn(); + console.timeEnd('1'); + console.time('2'); + worker.send( [CHILD_MESSAGE_CALL, true, 'leakMemory', []], onStart, @@ -223,9 +243,20 @@ describe.each([ onCustom, ); + console.timeEnd('2'); + console.time('3'); + await worker.waitForExit(); + console.timeEnd('3'); + console.time('4'); + + await expect( + async () => await worker.waitForWorkerReady(), + ).rejects.toThrowError(); expect(worker.isWorkerRunning()).toBeFalsy(); + + console.timeEnd('4'); }, 15000); test('should handle regular fatal crashes', async () => { @@ -287,6 +318,7 @@ describe.each([ // one last time at the end ready for the next request. expect(pidChanges).toEqual(5); expect(worker.isWorkerRunning()).toBeTruthy(); + expect(worker.getWorkerState()).toEqual(WorkerStates.OK); worker.forceExit(); From 29e1193fe9219803b6fb151b4ad425dfb1299453 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 17:04:46 +0100 Subject: [PATCH 07/52] chore: break apart tests to see where the problem is occuring --- .../src/workers/NodeThreadsWorker.ts | 1 + .../workers/__tests__/WorkerEdgeCases.test.js | 408 ++++++++++-------- 2 files changed, 223 insertions(+), 186 deletions(-) diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 7c05683abc5d..f6fa3ea3ef8a 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -145,6 +145,7 @@ export default class ExperimentalWorker implements WorkerInterface { ]); this._retries++; + this._state = WorkerStates.OK; // If we exceeded the amount of retries, we will emulate an error reply // coming from the child. This avoids code duplication related with cleaning diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 57fe27690ac0..7946ebfa1d50 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -60,22 +60,16 @@ describe.each([ workerClass: ChildProcessWorker, workerPath: processChildWorkerPath, }, - // { - // name: 'ThreadWorker', - // workerClass: ThreadsWorker, - // workerPath: threadChildWorkerPath, - // }, + { + name: 'ThreadWorker', + workerClass: ThreadsWorker, + workerPath: threadChildWorkerPath, + }, ])('$name', ({workerClass, workerPath}) => { /** @type WorkerInterface */ - let worker; let int; afterEach(async () => { - if (worker) { - worker.forceExit(); - await worker.waitForExit(); - } - clearInterval(int); }); @@ -103,227 +97,269 @@ describe.each([ } test('should get memory usage', async () => { - worker = new workerClass({ - childWorkerPath: workerPath, - maxRetries: 0, - workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), - }); + let worker; - const memoryUsagePromise = worker.getMemoryUsage(); - expect(memoryUsagePromise).toBeInstanceOf(Promise); + try { + worker = new workerClass({ + childWorkerPath: workerPath, + maxRetries: 0, + workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), + }); + + const memoryUsagePromise = worker.getMemoryUsage(); + expect(memoryUsagePromise).toBeInstanceOf(Promise); - expect(await memoryUsagePromise).toBeGreaterThan(0); + expect(await memoryUsagePromise).toBeGreaterThan(0); + } finally { + if (worker) { + worker.forceExit(); + await worker.waitForExit(); + } + } }); test('should recycle on idle limit breach', async () => { - worker = new workerClass({ - childWorkerPath: workerPath, - // There is no way this is fitting into 1000 bytes, so it should restart - // after requesting a memory usage update - idleMemoryLimit: 1000, - maxRetries: 0, - workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), - }); + let worker; + + try { + worker = new workerClass({ + childWorkerPath: workerPath, + // There is no way this is fitting into 1000 bytes, so it should restart + // after requesting a memory usage update + idleMemoryLimit: 1000, + maxRetries: 0, + workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), + }); - const startSystemId = worker.getWorkerSystemId(); - expect(startSystemId).toBeGreaterThanOrEqual(0); + const startSystemId = worker.getWorkerSystemId(); + expect(startSystemId).toBeGreaterThanOrEqual(0); - worker.checkMemoryUsage(); + worker.checkMemoryUsage(); - await waitForChange(() => worker.getWorkerSystemId()); + await waitForChange(() => worker.getWorkerSystemId()); - const systemId = worker.getWorkerSystemId(); - expect(systemId).toBeGreaterThanOrEqual(0); - expect(systemId).not.toEqual(startSystemId); + const systemId = worker.getWorkerSystemId(); + expect(systemId).toBeGreaterThanOrEqual(0); + expect(systemId).not.toEqual(startSystemId); - await new Promise(resolve => { - setTimeout(resolve, SIGKILL_DELAY + 100); - }); + await new Promise(resolve => { + setTimeout(resolve, SIGKILL_DELAY + 100); + }); - expect(worker.isWorkerRunning()).toBeTruthy(); + expect(worker.isWorkerRunning()).toBeTruthy(); + } finally { + if (worker) { + worker.forceExit(); + await worker.waitForExit(); + } + } }); - test('should automatically recycle on idle limit breach', async () => { - worker = new workerClass({ - childWorkerPath: workerPath, - // There is no way this is fitting into 1000 bytes, so it should restart - // after requesting a memory usage update - idleMemoryLimit: 1000, - maxRetries: 0, - workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), + describe('should automatically recycle on idle limit breach', () => { + let startPid; + let worker; + + beforeAll(() => { + worker = new workerClass({ + childWorkerPath: workerPath, + // There is no way this is fitting into 1000 bytes, so it should restart + // after requesting a memory usage update + idleMemoryLimit: 1000, + maxRetries: 0, + workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), + }); }); - console.log('A'); - - const startPid = worker.getWorkerSystemId(); - expect(startPid).toBeGreaterThanOrEqual(0); - - console.log('B'); - - const onStart = jest.fn(); - const onEnd = jest.fn(); - const onCustom = jest.fn(); - - worker.send( - [CHILD_MESSAGE_CALL, true, 'safeFunction', []], - onStart, - onEnd, - onCustom, - ); - - console.log('C'); - - await waitForChange(() => worker.getWorkerSystemId()); + afterAll(async () => { + if (worker) { + worker.forceExit(); + await worker.waitForExit(); + } + }); - console.log('D'); + test('initial state', async () => { + startPid = worker.getWorkerSystemId(); + expect(startPid).toBeGreaterThanOrEqual(0); - const endPid = worker.getWorkerSystemId(); - expect(endPid).toBeGreaterThanOrEqual(0); - expect(endPid).not.toEqual(startPid); - expect(worker.isWorkerRunning()).toBeTruthy(); + const onStart = jest.fn(); + const onEnd = jest.fn(); + const onCustom = jest.fn(); - console.log('E'); + worker.send( + [CHILD_MESSAGE_CALL, true, 'safeFunction', []], + onStart, + onEnd, + onCustom, + ); - await new Promise(resolve => { - setTimeout(resolve, SIGKILL_DELAY + 100); + await waitForChange(() => worker.getWorkerSystemId()); }); - console.log('F'); - - await worker.waitForWorkerReady(); + test('new worker starts', async () => { + const endPid = worker.getWorkerSystemId(); + expect(endPid).toBeGreaterThanOrEqual(0); + expect(endPid).not.toEqual(startPid); + expect(worker.isWorkerRunning()).toBeTruthy(); + }); - console.log('G'); + test('worker continues to run after kill delay', async () => { + await new Promise(resolve => { + setTimeout(resolve, SIGKILL_DELAY + 100); + }); - expect(worker.isWorkerRunning()).toBeTruthy(); - }, 10000); + await worker.waitForWorkerReady(); - test('should cleanly exit on crash', async () => { - const workerHeapLimit = 10; + expect(worker.isWorkerRunning()).toBeTruthy(); + }); + }); - /** @type WorkerOptions */ - const options = { - childWorkerPath: workerPath, - maxRetries: 0, - silent: true, - workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), - }; + describe('should cleanly exit on crash', () => { + let worker; - console.time('1'); + beforeAll(() => { + const workerHeapLimit = 10; - if (workerClass === ThreadsWorker) { - options.resourceLimits = { - codeRangeSizeMb: workerHeapLimit * 2, - maxOldGenerationSizeMb: workerHeapLimit, - maxYoungGenerationSizeMb: workerHeapLimit * 2, - stackSizeMb: workerHeapLimit * 2, + /** @type WorkerOptions */ + const options = { + childWorkerPath: workerPath, + maxRetries: 0, + silent: true, + workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), }; - } else if (workerClass === ChildProcessWorker) { - options.forkOptions = { - // Forcibly set the heap limit so we can crash the process easily. - execArgv: [`--max-old-space-size=${workerHeapLimit}`], - }; - } - worker = new workerClass(options); + if (workerClass === ThreadsWorker) { + options.resourceLimits = { + codeRangeSizeMb: workerHeapLimit * 2, + maxOldGenerationSizeMb: workerHeapLimit, + maxYoungGenerationSizeMb: workerHeapLimit * 2, + stackSizeMb: workerHeapLimit * 2, + }; + } else if (workerClass === ChildProcessWorker) { + options.forkOptions = { + // Forcibly set the heap limit so we can crash the process easily. + execArgv: [`--max-old-space-size=${workerHeapLimit}`], + }; + } - const pid = worker.getWorkerSystemId(); - expect(pid).toBeGreaterThanOrEqual(0); + worker = new workerClass(options); + }); - const onStart = jest.fn(); - const onEnd = jest.fn(); - const onCustom = jest.fn(); + afterAll(async () => { + if (worker) { + worker.forceExit(); + await worker.waitForExit(); + } + }); - console.timeEnd('1'); - console.time('2'); + test('starting state', async () => { + const startPid = worker.getWorkerSystemId(); + expect(startPid).toBeGreaterThanOrEqual(0); + }); - worker.send( - [CHILD_MESSAGE_CALL, true, 'leakMemory', []], - onStart, - onEnd, - onCustom, - ); + test('worker crashes and exits', async () => { + const onStart = jest.fn(); + const onEnd = jest.fn(); + const onCustom = jest.fn(); - console.timeEnd('2'); - console.time('3'); + worker.send( + [CHILD_MESSAGE_CALL, true, 'leakMemory', []], + onStart, + onEnd, + onCustom, + ); - await worker.waitForExit(); + await worker.waitForExit(); - console.timeEnd('3'); - console.time('4'); + await expect( + async () => await worker.waitForWorkerReady(), + ).rejects.toThrowError(); + expect(worker.isWorkerRunning()).toBeFalsy(); + }); + }, 15000); - await expect( - async () => await worker.waitForWorkerReady(), - ).rejects.toThrowError(); - expect(worker.isWorkerRunning()).toBeFalsy(); + describe('should handle regular fatal crashes', () => { + let worker; - console.timeEnd('4'); - }, 15000); + beforeAll(() => { + worker = new workerClass({ + childWorkerPath: workerPath, + maxRetries: 4, + workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), + }); + }); - test('should handle regular fatal crashes', async () => { - worker = new workerClass({ - childWorkerPath: workerPath, - maxRetries: 4, - workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), + afterAll(async () => { + if (worker) { + worker.forceExit(); + await worker.waitForExit(); + } }); - const startPid = worker.getWorkerSystemId(); - expect(startPid).toBeGreaterThanOrEqual(0); - - const onStart = jest.fn(); - const onEnd = jest.fn(); - const onCustom = jest.fn(); - - worker.send( - [CHILD_MESSAGE_CALL, true, 'fatalExitCode', []], - onStart, - onEnd, - onCustom, - ); - - let pidChanges = 0; - - while (true) { - // Ideally this would use Promise.any but it's not supported in Node 14 - // so doing this instead. Essentially what we're doing is looping and - // capturing the pid every time it changes. When it stops changing the - // timeout will be hit and we should be left with a collection of all - // the pids used by the worker. - const newPid = await new Promise(resolve => { - const resolved = false; - - const to = setTimeout(() => { - if (!resolved) { - this.resolved = true; - resolve(undefined); - } - }, 500); - - waitForChange(() => worker.getWorkerSystemId()).then(() => { - clearTimeout(to); - - if (!resolved) { - resolve(worker.getWorkerSystemId()); - } + test('starting state', async () => { + const startPid = worker.getWorkerSystemId(); + expect(startPid).toBeGreaterThanOrEqual(0); + }); + + test('processes restart', async () => { + const onStart = jest.fn(); + const onEnd = jest.fn(); + const onCustom = jest.fn(); + + worker.send( + [CHILD_MESSAGE_CALL, true, 'fatalExitCode', []], + onStart, + onEnd, + onCustom, + ); + + let pidChanges = 0; + + while (true) { + // Ideally this would use Promise.any but it's not supported in Node 14 + // so doing this instead. Essentially what we're doing is looping and + // capturing the pid every time it changes. When it stops changing the + // timeout will be hit and we should be left with a collection of all + // the pids used by the worker. + const newPid = await new Promise(resolve => { + const resolved = false; + + const to = setTimeout(() => { + if (!resolved) { + this.resolved = true; + resolve(undefined); + } + }, 500); + + waitForChange(() => worker.getWorkerSystemId()).then(() => { + clearTimeout(to); + + if (!resolved) { + resolve(worker.getWorkerSystemId()); + } + }); }); - }); - if (typeof newPid === 'number') { - pidChanges++; - } else { - break; + if (typeof newPid === 'number') { + pidChanges++; + } else { + break; + } } - } - // Expect the pids to be retries + 1 because it is restarted - // one last time at the end ready for the next request. - expect(pidChanges).toEqual(5); - expect(worker.isWorkerRunning()).toBeTruthy(); - expect(worker.getWorkerState()).toEqual(WorkerStates.OK); + // Expect the pids to be retries + 1 because it is restarted + // one last time at the end ready for the next request. + expect(pidChanges).toEqual(5); + + expect(worker.isWorkerRunning()).toBeTruthy(); + expect(worker.getWorkerState()).toEqual(WorkerStates.OK); + }); - worker.forceExit(); + test('processes exits', async () => { + worker.forceExit(); - await expect( - async () => await worker.waitForWorkerReady(), - ).rejects.toThrowError(); + await expect( + async () => await worker.waitForWorkerReady(), + ).rejects.toThrowError(); + }); }); }); From c23f581f200c168125b5176ca980ad1e6c39ef6f Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 17:21:56 +0100 Subject: [PATCH 08/52] fix: handling of work ready status --- .../src/workers/ChildProcessWorker.ts | 33 ++++++++++--------- .../src/workers/NodeThreadsWorker.ts | 33 ++++++++++--------- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 51326d620478..81c0a0defaa5 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -169,7 +169,6 @@ export default class ChildProcessWorker implements WorkerInterface { ]); this._child = child; - this._state = WorkerStates.OK; this._retries++; @@ -193,6 +192,7 @@ export default class ChildProcessWorker implements WorkerInterface { this._request = null; } + this._state = WorkerStates.OK; if (this._resolveWorkerReady) { this._resolveWorkerReady(); } @@ -487,20 +487,23 @@ export default class ChildProcessWorker implements WorkerInterface { waitForWorkerReady(): Promise { if (!this._workerReadyPromise) { this._workerReadyPromise = new Promise((resolve, reject) => { - if ( - this._state === WorkerStates.OUT_OF_MEMORY || - this._state === WorkerStates.SHUTTING_DOWN || - this._state === WorkerStates.SHUT_DOWN - ) { - reject( - new Error( - `Worker state means it will never be ready: ${this._state}`, - ), - ); - } else if (this.isWorkerRunning()) { - resolve(); - } else { - this._resolveWorkerReady = resolve; + switch (this._state) { + case WorkerStates.OUT_OF_MEMORY: + case WorkerStates.SHUTTING_DOWN: + case WorkerStates.SHUT_DOWN: + reject( + new Error( + `Worker state means it will never be ready: ${this._state}`, + ), + ); + break; + case WorkerStates.STARTING: + case WorkerStates.RESTARTING: + this._resolveWorkerReady = resolve; + break; + case WorkerStates.OK: + resolve(); + break; } }); } diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index f6fa3ea3ef8a..69b387dc09c6 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -145,7 +145,6 @@ export default class ExperimentalWorker implements WorkerInterface { ]); this._retries++; - this._state = WorkerStates.OK; // If we exceeded the amount of retries, we will emulate an error reply // coming from the child. This avoids code duplication related with cleaning @@ -162,6 +161,7 @@ export default class ExperimentalWorker implements WorkerInterface { ]); } + this._state = WorkerStates.OK; if (this._resolveWorkerReady) { this._resolveWorkerReady(); } @@ -418,20 +418,23 @@ export default class ExperimentalWorker implements WorkerInterface { waitForWorkerReady(): Promise { if (!this._workerReadyPromise) { this._workerReadyPromise = new Promise((resolve, reject) => { - if ( - this._state === WorkerStates.OUT_OF_MEMORY || - this._state === WorkerStates.SHUTTING_DOWN || - this._state === WorkerStates.SHUT_DOWN - ) { - reject( - new Error( - `Worker state means it will never be ready: ${this._state}`, - ), - ); - } else if (this.isWorkerRunning()) { - resolve(); - } else { - this._resolveWorkerReady = resolve; + switch (this._state) { + case WorkerStates.OUT_OF_MEMORY: + case WorkerStates.SHUTTING_DOWN: + case WorkerStates.SHUT_DOWN: + reject( + new Error( + `Worker state means it will never be ready: ${this._state}`, + ), + ); + break; + case WorkerStates.STARTING: + case WorkerStates.RESTARTING: + this._resolveWorkerReady = resolve; + break; + case WorkerStates.OK: + resolve(); + break; } }); } From e8a8058237fdd4b3befadc6b9a936ccf8971d9f3 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 17:48:01 +0100 Subject: [PATCH 09/52] chore: add fallback timeout --- .../src/workers/ChildProcessWorker.ts | 42 +++++++++++++------ .../workers/__tests__/WorkerEdgeCases.test.js | 25 ++++++----- 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 81c0a0defaa5..980e484310f9 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -135,41 +135,39 @@ export default class ChildProcessWorker implements WorkerInterface { ...this._options.forkOptions, }; - const child = fork(this._childWorkerPath, [], options); + this._child = fork(this._childWorkerPath, [], options); - if (child.stdout) { + if (this._child.stdout) { if (!this._stdout) { // We need to add a permanent stream to the merged stream to prevent it // from ending when the subprocess stream ends this._stdout = mergeStream(this._getFakeStream()); } - this._stdout.add(child.stdout); + this._stdout.add(this._child.stdout); } - if (child.stderr) { + if (this._child.stderr) { if (!this._stderr) { // We need to add a permanent stream to the merged stream to prevent it // from ending when the subprocess stream ends this._stderr = mergeStream(this._getFakeStream()); } - this._stderr.add(child.stderr); + this._stderr.add(this._child.stderr); } - this._detectOutOfMemoryCrash(child); - child.on('message', this._onMessage.bind(this)); - child.on('exit', this._onExit.bind(this)); + this._detectOutOfMemoryCrash(this._child); + this._child.on('message', this._onMessage.bind(this)); + this._child.on('exit', this._onExit.bind(this)); - child.send([ + this._child.send([ CHILD_MESSAGE_INITIALIZE, false, this._options.workerPath, this._options.setupArgs, ]); - this._child = child; - this._retries++; // If we exceeded the amount of retries, we will emulate an error reply @@ -487,10 +485,14 @@ export default class ChildProcessWorker implements WorkerInterface { waitForWorkerReady(): Promise { if (!this._workerReadyPromise) { this._workerReadyPromise = new Promise((resolve, reject) => { + let settled = false; + let to: NodeJS.Timeout | undefined; + switch (this._state) { case WorkerStates.OUT_OF_MEMORY: case WorkerStates.SHUTTING_DOWN: case WorkerStates.SHUT_DOWN: + settled = true; reject( new Error( `Worker state means it will never be ready: ${this._state}`, @@ -499,12 +501,28 @@ export default class ChildProcessWorker implements WorkerInterface { break; case WorkerStates.STARTING: case WorkerStates.RESTARTING: - this._resolveWorkerReady = resolve; + this._resolveWorkerReady = () => { + settled = true; + resolve(); + + if (to) { + clearTimeout(to); + } + }; break; case WorkerStates.OK: + settled = true; resolve(); break; } + + if (!settled) { + to = setTimeout(() => { + if (!settled) { + reject(new Error('Timeout starting worker')); + } + }, 500); + } }); } diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 7946ebfa1d50..5d09b8974443 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -180,7 +180,10 @@ describe.each([ test('initial state', async () => { startPid = worker.getWorkerSystemId(); expect(startPid).toBeGreaterThanOrEqual(0); + expect(worker.getWorkerState()).toEqual(WorkerStates.OK); + }); + test('new worker starts', async () => { const onStart = jest.fn(); const onEnd = jest.fn(); const onCustom = jest.fn(); @@ -193,24 +196,26 @@ describe.each([ ); await waitForChange(() => worker.getWorkerSystemId()); - }); - test('new worker starts', async () => { const endPid = worker.getWorkerSystemId(); expect(endPid).toBeGreaterThanOrEqual(0); expect(endPid).not.toEqual(startPid); expect(worker.isWorkerRunning()).toBeTruthy(); + expect(worker.getWorkerState()).toEqual(WorkerStates.OK); }); - test('worker continues to run after kill delay', async () => { - await new Promise(resolve => { - setTimeout(resolve, SIGKILL_DELAY + 100); - }); - - await worker.waitForWorkerReady(); + test( + 'worker continues to run after kill delay', + async () => { + await new Promise(resolve => { + setTimeout(resolve, SIGKILL_DELAY + 100); + }); - expect(worker.isWorkerRunning()).toBeTruthy(); - }); + expect(worker.getWorkerState()).toEqual(WorkerStates.OK); + expect(worker.isWorkerRunning()).toBeTruthy(); + }, + SIGKILL_DELAY * 3, + ); }); describe('should cleanly exit on crash', () => { From 4e0b3992776378171fe41872f6e04d84b1155cd4 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 18:08:27 +0100 Subject: [PATCH 10/52] refactor: add abstract class with common functions and vars --- .../src/workers/ChildProcessWorker.ts | 59 ++-------------- .../src/workers/NodeThreadsWorker.ts | 40 ++--------- .../jest-worker/src/workers/WorkerAbstract.ts | 69 +++++++++++++++++++ 3 files changed, 83 insertions(+), 85 deletions(-) create mode 100644 packages/jest-worker/src/workers/WorkerAbstract.ts diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 980e484310f9..7dd05c833e43 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -27,6 +27,7 @@ import { WorkerOptions, WorkerStates, } from '../types'; +import WorkerAbstract from './WorkerAbstract'; const SIGNAL_BASE_EXIT_CODE = 128; const SIGKILL_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 9; @@ -53,7 +54,10 @@ export const SIGKILL_DELAY = 500; * field is changed to "true", so that other workers which might encounter the * same call skip it. */ -export default class ChildProcessWorker implements WorkerInterface { +export default class ChildProcessWorker + extends WorkerAbstract + implements WorkerInterface +{ private _child!: ChildProcess; private _options: WorkerOptions; @@ -72,17 +76,15 @@ export default class ChildProcessWorker implements WorkerInterface { private _memoryUsagePromise: Promise | undefined; private _resolveMemoryUsage: ((arg0: number) => void) | undefined; - private _workerReadyPromise: Promise | undefined; - private _resolveWorkerReady: (() => void) | undefined; - private _childIdleMemoryUsage: number | null; private _childIdleMemoryUsageLimit: number | null; private _memoryUsageCheck = false; - private _state: WorkerStates; private _childWorkerPath: string; constructor(options: WorkerOptions) { + super(); + this._options = options; this._request = null; @@ -482,53 +484,6 @@ export default class ChildProcessWorker implements WorkerInterface { return this._child.connected && !this._child.killed; } - waitForWorkerReady(): Promise { - if (!this._workerReadyPromise) { - this._workerReadyPromise = new Promise((resolve, reject) => { - let settled = false; - let to: NodeJS.Timeout | undefined; - - switch (this._state) { - case WorkerStates.OUT_OF_MEMORY: - case WorkerStates.SHUTTING_DOWN: - case WorkerStates.SHUT_DOWN: - settled = true; - reject( - new Error( - `Worker state means it will never be ready: ${this._state}`, - ), - ); - break; - case WorkerStates.STARTING: - case WorkerStates.RESTARTING: - this._resolveWorkerReady = () => { - settled = true; - resolve(); - - if (to) { - clearTimeout(to); - } - }; - break; - case WorkerStates.OK: - settled = true; - resolve(); - break; - } - - if (!settled) { - to = setTimeout(() => { - if (!settled) { - reject(new Error('Timeout starting worker')); - } - }, 500); - } - }); - } - - return this._workerReadyPromise; - } - private _getFakeStream() { if (!this._fakeStream) { this._fakeStream = new PassThrough(); diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 69b387dc09c6..5d0dda92a6c4 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -26,8 +26,12 @@ import { WorkerOptions, WorkerStates, } from '../types'; +import WorkerAbstract from './WorkerAbstract'; -export default class ExperimentalWorker implements WorkerInterface { +export default class ExperimentalWorker + extends WorkerAbstract + implements WorkerInterface +{ private _worker!: Worker; private _options: WorkerOptions; @@ -46,17 +50,15 @@ export default class ExperimentalWorker implements WorkerInterface { private _memoryUsagePromise: Promise | undefined; private _resolveMemoryUsage: ((arg0: number) => void) | undefined; - private _workerReadyPromise: Promise | undefined; - private _resolveWorkerReady: (() => void) | undefined; - private _childWorkerPath: string; private _childIdleMemoryUsage: number | null; private _childIdleMemoryUsageLimit: number | null; private _memoryUsageCheck = false; - private _state: WorkerStates; constructor(options: WorkerOptions) { + super(); + this._options = options; this._request = null; @@ -75,7 +77,6 @@ export default class ExperimentalWorker implements WorkerInterface { this._resolveExitPromise = resolve; }); - this._state = WorkerStates.STARTING; this.initialize(); } @@ -415,33 +416,6 @@ export default class ExperimentalWorker implements WorkerInterface { return this._worker.threadId; } - waitForWorkerReady(): Promise { - if (!this._workerReadyPromise) { - this._workerReadyPromise = new Promise((resolve, reject) => { - switch (this._state) { - case WorkerStates.OUT_OF_MEMORY: - case WorkerStates.SHUTTING_DOWN: - case WorkerStates.SHUT_DOWN: - reject( - new Error( - `Worker state means it will never be ready: ${this._state}`, - ), - ); - break; - case WorkerStates.STARTING: - case WorkerStates.RESTARTING: - this._resolveWorkerReady = resolve; - break; - case WorkerStates.OK: - resolve(); - break; - } - }); - } - - return this._workerReadyPromise; - } - private _getFakeStream() { if (!this._fakeStream) { this._fakeStream = new PassThrough(); diff --git a/packages/jest-worker/src/workers/WorkerAbstract.ts b/packages/jest-worker/src/workers/WorkerAbstract.ts new file mode 100644 index 000000000000..3f65c92612e4 --- /dev/null +++ b/packages/jest-worker/src/workers/WorkerAbstract.ts @@ -0,0 +1,69 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {WorkerInterface, WorkerStates} from '../types'; + +export default abstract class WorkerAbstract + implements Pick +{ + protected _state = WorkerStates.STARTING; + + protected _workerReadyPromise: Promise | undefined; + protected _resolveWorkerReady: (() => void) | undefined; + + /** + * Wait for the worker child process to be ready to handle requests. + * + * @returns Promise which resolves when ready. + */ + public waitForWorkerReady(): Promise { + if (!this._workerReadyPromise) { + this._workerReadyPromise = new Promise((resolve, reject) => { + let settled = false; + let to: NodeJS.Timeout | undefined; + + switch (this._state) { + case WorkerStates.OUT_OF_MEMORY: + case WorkerStates.SHUTTING_DOWN: + case WorkerStates.SHUT_DOWN: + settled = true; + reject( + new Error( + `Worker state means it will never be ready: ${this._state}`, + ), + ); + break; + case WorkerStates.STARTING: + case WorkerStates.RESTARTING: + this._resolveWorkerReady = () => { + settled = true; + resolve(); + + if (to) { + clearTimeout(to); + } + }; + break; + case WorkerStates.OK: + settled = true; + resolve(); + break; + } + + if (!settled) { + to = setTimeout(() => { + if (!settled) { + reject(new Error('Timeout starting worker')); + } + }, 500); + } + }); + } + + return this._workerReadyPromise; + } +} From 790f772876dee00499c93f80b413f6256fb39382 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 18:39:41 +0100 Subject: [PATCH 11/52] fix: restarting on init --- .../src/workers/ChildProcessWorker.ts | 8 +++++++- .../src/workers/NodeThreadsWorker.ts | 4 +++- .../workers/__tests__/WorkerEdgeCases.test.js | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 7dd05c833e43..f3805180f0a4 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -363,11 +363,17 @@ export default class ChildProcessWorker onProcessStart(this); this._onProcessEnd = (...args) => { + const hasRequest = !!this._request; + // Clean the request to avoid sending past requests to workers that fail // while waiting for a new request (timers, unhandled rejections...) this._request = null; - if (this._childIdleMemoryUsageLimit && this._child.connected) { + if ( + this._childIdleMemoryUsageLimit && + this._child.connected && + hasRequest + ) { this.checkMemoryUsage(); } diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 5d0dda92a6c4..6e8dec9e5ba5 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -290,11 +290,13 @@ export default class ExperimentalWorker ): void { onProcessStart(this); this._onProcessEnd = (...args) => { + const hasRequest = !!this._request; + // Clean the request to avoid sending past requests to workers that fail // while waiting for a new request (timers, unhandled rejections...) this._request = null; - if (this._childIdleMemoryUsageLimit) { + if (this._childIdleMemoryUsageLimit && hasRequest) { this.checkMemoryUsage(); } diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 5d09b8974443..9b7620caa9e7 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -158,6 +158,8 @@ describe.each([ describe('should automatically recycle on idle limit breach', () => { let startPid; let worker; + const orderOfEvents = []; + let orderInt; beforeAll(() => { worker = new workerClass({ @@ -168,6 +170,16 @@ describe.each([ maxRetries: 0, workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), }); + + // We're doing this so we can track the state changes that have occurred within + // the worker to make sure it hasn't done anything weird. + orderInt = setInterval(() => { + const currentState = worker.getWorkerState(); + + if (orderOfEvents[orderOfEvents.length - 1] !== currentState) { + orderOfEvents.push(currentState); + } + }, 1); }); afterAll(async () => { @@ -175,6 +187,8 @@ describe.each([ worker.forceExit(); await worker.waitForExit(); } + + clearInterval(orderInt); }); test('initial state', async () => { @@ -216,6 +230,10 @@ describe.each([ }, SIGKILL_DELAY * 3, ); + + test('expected state order', () => { + expect(orderOfEvents).toMatchObject(['ok', 'restarting', 'ok']); + }); }); describe('should cleanly exit on crash', () => { From b4151ea6920ac607664224d78a0b3b30c19cba80 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 19:04:50 +0100 Subject: [PATCH 12/52] refactor: event emitting is a much cleaner way of tracking what's going on --- packages/jest-worker/src/types.ts | 16 ++++++- .../src/workers/ChildProcessWorker.ts | 32 +++++++------- .../src/workers/NodeThreadsWorker.ts | 28 ++++++------ .../jest-worker/src/workers/WorkerAbstract.ts | 44 ++++++++++++++++--- .../workers/__tests__/WorkerEdgeCases.test.js | 36 +++++++-------- 5 files changed, 102 insertions(+), 54 deletions(-) diff --git a/packages/jest-worker/src/types.ts b/packages/jest-worker/src/types.ts index 34ec77709ff6..ee7173bdf163 100644 --- a/packages/jest-worker/src/types.ts +++ b/packages/jest-worker/src/types.ts @@ -66,12 +66,15 @@ export interface WorkerPoolInterface { } export interface WorkerInterface { + get state(): WorkerStates; + send( request: ChildMessage, onProcessStart: OnStart, onProcessEnd: OnEnd, onCustomMessage: OnCustomMessage, ): void; + waitForExit(): Promise; forceExit(): void; @@ -83,7 +86,6 @@ export interface WorkerInterface { */ getWorkerSystemId(): number; getMemoryUsage(): Promise; - getWorkerState(): WorkerStates; /** * Checks to see if the child worker is actually running. */ @@ -183,6 +185,14 @@ export type WorkerOptions = { * the raw output of the worker. */ silent?: boolean; + /** + * Used to immediately bind event handlers. + */ + on?: { + [key in WorkerEvents]?: + | Array<(...args: Array) => void> + | ((...args: Array) => void); + }; }; // Messages passed from the parent to the children. @@ -278,3 +288,7 @@ export enum WorkerStates { SHUTTING_DOWN = 'shutting-down', SHUT_DOWN = 'shut-down', } + +export enum WorkerEvents { + STATE_CHANGE = 'state-change', +} diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index f3805180f0a4..4bae0468933b 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -83,7 +83,7 @@ export default class ChildProcessWorker private _childWorkerPath: string; constructor(options: WorkerOptions) { - super(); + super(options); this._options = options; @@ -102,15 +102,15 @@ export default class ChildProcessWorker this._childWorkerPath = options.childWorkerPath || require.resolve('./processChild'); - this._state = WorkerStates.STARTING; + this.state = WorkerStates.STARTING; this.initialize(); } initialize(): void { if ( - this._state === WorkerStates.OUT_OF_MEMORY || - this._state === WorkerStates.SHUTTING_DOWN || - this._state === WorkerStates.SHUT_DOWN + this.state === WorkerStates.OUT_OF_MEMORY || + this.state === WorkerStates.SHUTTING_DOWN || + this.state === WorkerStates.SHUT_DOWN ) { return; } @@ -119,7 +119,7 @@ export default class ChildProcessWorker this._child.kill('SIGKILL'); } - this._state = WorkerStates.STARTING; + this.state = WorkerStates.STARTING; const forceColor = stdoutSupportsColor ? {FORCE_COLOR: '1'} : {}; const options: ForkOptions = { @@ -192,7 +192,7 @@ export default class ChildProcessWorker this._request = null; } - this._state = WorkerStates.OK; + this.state = WorkerStates.OK; if (this._resolveWorkerReady) { this._resolveWorkerReady(); } @@ -202,7 +202,7 @@ export default class ChildProcessWorker let stderrStr = ''; const handler = (chunk: any) => { - if (this._state !== WorkerStates.OUT_OF_MEMORY) { + if (this.state !== WorkerStates.OUT_OF_MEMORY) { let str: string | undefined = undefined; if (chunk instanceof Buffer) { @@ -216,7 +216,7 @@ export default class ChildProcessWorker } if (stderrStr.includes('heap out of memory')) { - this._state = WorkerStates.OUT_OF_MEMORY; + this.state = WorkerStates.OUT_OF_MEMORY; } } }; @@ -225,7 +225,7 @@ export default class ChildProcessWorker } private _shutdown() { - this._state = WorkerStates.SHUTTING_DOWN; + this.state = WorkerStates.SHUTTING_DOWN; // End the temporary streams so the merged streams end too if (this._fakeStream) { @@ -318,7 +318,7 @@ export default class ChildProcessWorker this._childIdleMemoryUsage && this._childIdleMemoryUsage > limit ) { - this._state = WorkerStates.RESTARTING; + this.state = WorkerStates.RESTARTING; this.killChild(); } @@ -329,7 +329,7 @@ export default class ChildProcessWorker this._workerReadyPromise = undefined; this._resolveWorkerReady = undefined; - if (exitCode !== 0 && this._state === WorkerStates.OUT_OF_MEMORY) { + if (exitCode !== 0 && this.state === WorkerStates.OUT_OF_MEMORY) { this._onProcessEnd( new Error('Jest worker ran out of memory and crashed'), null, @@ -341,8 +341,8 @@ export default class ChildProcessWorker exitCode !== null && exitCode !== SIGTERM_EXIT_CODE && exitCode !== SIGKILL_EXIT_CODE && - this._state !== WorkerStates.SHUTTING_DOWN) || - this._state === WorkerStates.RESTARTING + this.state !== WorkerStates.SHUTTING_DOWN) || + this.state === WorkerStates.RESTARTING ) { this.initialize(); @@ -402,7 +402,7 @@ export default class ChildProcessWorker } forceExit(): void { - this._state = WorkerStates.SHUTTING_DOWN; + this.state = WorkerStates.SHUTTING_DOWN; const sigkillTimeout = this.killChild(); this._exitPromise.then(() => clearTimeout(sigkillTimeout)); @@ -498,6 +498,6 @@ export default class ChildProcessWorker } getWorkerState(): WorkerStates { - return this._state; + return this.state; } } diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 6e8dec9e5ba5..a740ec7d80fa 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -57,7 +57,7 @@ export default class ExperimentalWorker private _memoryUsageCheck = false; constructor(options: WorkerOptions) { - super(); + super(options); this._options = options; @@ -82,9 +82,9 @@ export default class ExperimentalWorker initialize(): void { if ( - this._state === WorkerStates.OUT_OF_MEMORY || - this._state === WorkerStates.SHUTTING_DOWN || - this._state === WorkerStates.SHUT_DOWN + this.state === WorkerStates.OUT_OF_MEMORY || + this.state === WorkerStates.SHUTTING_DOWN || + this.state === WorkerStates.SHUT_DOWN ) { return; } @@ -93,7 +93,7 @@ export default class ExperimentalWorker this._worker.terminate(); } - this._state = WorkerStates.STARTING; + this.state = WorkerStates.STARTING; this._worker = new Worker(this._childWorkerPath, { eval: false, @@ -162,7 +162,7 @@ export default class ExperimentalWorker ]); } - this._state = WorkerStates.OK; + this.state = WorkerStates.OK; if (this._resolveWorkerReady) { this._resolveWorkerReady(); } @@ -180,7 +180,7 @@ export default class ExperimentalWorker private _onError(error: Error) { if (error.message.includes('heap out of memory')) { - this._state = WorkerStates.OUT_OF_MEMORY; + this.state = WorkerStates.OUT_OF_MEMORY; } } @@ -250,7 +250,7 @@ export default class ExperimentalWorker this._workerReadyPromise = undefined; this._resolveWorkerReady = undefined; - if (exitCode !== 0 && this._state === WorkerStates.OUT_OF_MEMORY) { + if (exitCode !== 0 && this.state === WorkerStates.OUT_OF_MEMORY) { this._onProcessEnd( new Error('Jest worker ran out of memory and crashed'), null, @@ -259,9 +259,9 @@ export default class ExperimentalWorker this._shutdown(); } else if ( (exitCode !== 0 && - this._state !== WorkerStates.SHUTTING_DOWN && - this._state !== WorkerStates.SHUT_DOWN) || - this._state === WorkerStates.RESTARTING + this.state !== WorkerStates.SHUTTING_DOWN && + this.state !== WorkerStates.SHUT_DOWN) || + this.state === WorkerStates.RESTARTING ) { this.initialize(); @@ -278,7 +278,7 @@ export default class ExperimentalWorker } forceExit(): void { - this._state = WorkerStates.SHUTTING_DOWN; + this.state = WorkerStates.SHUTTING_DOWN; this._worker.terminate(); } @@ -349,7 +349,7 @@ export default class ExperimentalWorker this._childIdleMemoryUsage && this._childIdleMemoryUsage > limit ) { - this._state = WorkerStates.RESTARTING; + this.state = WorkerStates.RESTARTING; this._worker.terminate(); } @@ -430,6 +430,6 @@ export default class ExperimentalWorker } getWorkerState(): WorkerStates { - return this._state; + return this.state; } } diff --git a/packages/jest-worker/src/workers/WorkerAbstract.ts b/packages/jest-worker/src/workers/WorkerAbstract.ts index 3f65c92612e4..52b629108b7a 100644 --- a/packages/jest-worker/src/workers/WorkerAbstract.ts +++ b/packages/jest-worker/src/workers/WorkerAbstract.ts @@ -5,16 +5,50 @@ * LICENSE file in the root directory of this source tree. */ -import {WorkerInterface, WorkerStates} from '../types'; +import {EventEmitter} from 'stream'; +import { + WorkerEvents, + WorkerInterface, + WorkerOptions, + WorkerStates, +} from '../types'; export default abstract class WorkerAbstract - implements Pick + extends EventEmitter + implements Pick { - protected _state = WorkerStates.STARTING; + private __state = WorkerStates.STARTING; protected _workerReadyPromise: Promise | undefined; protected _resolveWorkerReady: (() => void) | undefined; + public get state(): WorkerStates { + return this.__state; + } + protected set state(value: WorkerStates) { + if (this.__state !== value) { + this.__state = value; + + this.emit(WorkerEvents.STATE_CHANGE, value); + } + } + + constructor(options: WorkerOptions) { + super(); + + if (typeof options.on === 'object') { + for (const [event, handlers] of Object.entries(options.on)) { + if (Array.isArray(handlers)) { + for (const handler of handlers) { + super.on(event, handler); + } + } else { + super.on(event, handlers); + } + } + } + } + /** * Wait for the worker child process to be ready to handle requests. * @@ -26,14 +60,14 @@ export default abstract class WorkerAbstract let settled = false; let to: NodeJS.Timeout | undefined; - switch (this._state) { + switch (this.state) { case WorkerStates.OUT_OF_MEMORY: case WorkerStates.SHUTTING_DOWN: case WorkerStates.SHUT_DOWN: settled = true; reject( new Error( - `Worker state means it will never be ready: ${this._state}`, + `Worker state means it will never be ready: ${this.state}`, ), ); break; diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 9b7620caa9e7..81ddc1a08559 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -11,6 +11,7 @@ import {transformFileAsync} from '@babel/core'; import { CHILD_MESSAGE_CALL, CHILD_MESSAGE_MEM_USAGE, + WorkerEvents, WorkerInterface, WorkerOptions, WorkerStates, @@ -159,7 +160,6 @@ describe.each([ let startPid; let worker; const orderOfEvents = []; - let orderInt; beforeAll(() => { worker = new workerClass({ @@ -168,18 +168,13 @@ describe.each([ // after requesting a memory usage update idleMemoryLimit: 1000, maxRetries: 0, + on: { + [WorkerEvents.STATE_CHANGE]: state => { + orderOfEvents.push(state); + }, + }, workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), }); - - // We're doing this so we can track the state changes that have occurred within - // the worker to make sure it hasn't done anything weird. - orderInt = setInterval(() => { - const currentState = worker.getWorkerState(); - - if (orderOfEvents[orderOfEvents.length - 1] !== currentState) { - orderOfEvents.push(currentState); - } - }, 1); }); afterAll(async () => { @@ -187,14 +182,14 @@ describe.each([ worker.forceExit(); await worker.waitForExit(); } - - clearInterval(orderInt); }); test('initial state', async () => { startPid = worker.getWorkerSystemId(); expect(startPid).toBeGreaterThanOrEqual(0); - expect(worker.getWorkerState()).toEqual(WorkerStates.OK); + expect(worker.state).toEqual(WorkerStates.OK); + + expect(orderOfEvents).toMatchObject(['ok']); }); test('new worker starts', async () => { @@ -215,7 +210,7 @@ describe.each([ expect(endPid).toBeGreaterThanOrEqual(0); expect(endPid).not.toEqual(startPid); expect(worker.isWorkerRunning()).toBeTruthy(); - expect(worker.getWorkerState()).toEqual(WorkerStates.OK); + expect(worker.state).toEqual(WorkerStates.OK); }); test( @@ -225,14 +220,19 @@ describe.each([ setTimeout(resolve, SIGKILL_DELAY + 100); }); - expect(worker.getWorkerState()).toEqual(WorkerStates.OK); + expect(worker.state).toEqual(WorkerStates.OK); expect(worker.isWorkerRunning()).toBeTruthy(); }, SIGKILL_DELAY * 3, ); test('expected state order', () => { - expect(orderOfEvents).toMatchObject(['ok', 'restarting', 'ok']); + expect(orderOfEvents).toMatchObject([ + 'ok', + 'restarting', + 'starting', + 'ok', + ]); }); }); @@ -374,7 +374,7 @@ describe.each([ expect(pidChanges).toEqual(5); expect(worker.isWorkerRunning()).toBeTruthy(); - expect(worker.getWorkerState()).toEqual(WorkerStates.OK); + expect(worker.state).toEqual(WorkerStates.OK); }); test('processes exits', async () => { From 2753069a7b52c4faa23cc6634cbdbb03f568624a Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 20:16:16 +0100 Subject: [PATCH 13/52] chore: better handle shutdown events --- packages/jest-worker/src/types.ts | 11 ++++++++--- .../src/workers/ChildProcessWorker.ts | 3 +++ .../src/workers/NodeThreadsWorker.ts | 5 +++++ .../jest-worker/src/workers/WorkerAbstract.ts | 3 ++- .../workers/__tests__/WorkerEdgeCases.test.js | 19 +++++++++++++++++++ 5 files changed, 37 insertions(+), 4 deletions(-) diff --git a/packages/jest-worker/src/types.ts b/packages/jest-worker/src/types.ts index ee7173bdf163..31acac62d0e5 100644 --- a/packages/jest-worker/src/types.ts +++ b/packages/jest-worker/src/types.ts @@ -189,12 +189,17 @@ export type WorkerOptions = { * Used to immediately bind event handlers. */ on?: { - [key in WorkerEvents]?: - | Array<(...args: Array) => void> - | ((...args: Array) => void); + [WorkerEvents.STATE_CHANGE]: + | onStateChangeHandler + | Array; }; }; +export type onStateChangeHandler = ( + state: WorkerStates, + oldState: WorkerStates, +) => void; + // Messages passed from the parent to the children. export type MessagePort = typeof EventEmitter & { diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 4bae0468933b..013a1e30b1c3 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -98,6 +98,9 @@ export default class ChildProcessWorker this._exitPromise = new Promise(resolve => { this._resolveExitPromise = resolve; }); + this._exitPromise.then(() => { + this.state = WorkerStates.SHUT_DOWN; + }); this._childWorkerPath = options.childWorkerPath || require.resolve('./processChild'); diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index a740ec7d80fa..8d6ac6bd2a1c 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -76,6 +76,9 @@ export default class ExperimentalWorker this._exitPromise = new Promise(resolve => { this._resolveExitPromise = resolve; }); + this._exitPromise.then(() => { + this.state = WorkerStates.SHUT_DOWN; + }); this.initialize(); } @@ -181,6 +184,8 @@ export default class ExperimentalWorker private _onError(error: Error) { if (error.message.includes('heap out of memory')) { this.state = WorkerStates.OUT_OF_MEMORY; + + this.forceExit(); } } diff --git a/packages/jest-worker/src/workers/WorkerAbstract.ts b/packages/jest-worker/src/workers/WorkerAbstract.ts index 52b629108b7a..b476d75b4d62 100644 --- a/packages/jest-worker/src/workers/WorkerAbstract.ts +++ b/packages/jest-worker/src/workers/WorkerAbstract.ts @@ -27,9 +27,10 @@ export default abstract class WorkerAbstract } protected set state(value: WorkerStates) { if (this.__state !== value) { + const oldState = this.__state; this.__state = value; - this.emit(WorkerEvents.STATE_CHANGE, value); + this.emit(WorkerEvents.STATE_CHANGE, value, oldState); } } diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 81ddc1a08559..effe46298199 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -238,6 +238,7 @@ describe.each([ describe('should cleanly exit on crash', () => { let worker; + const orderOfEvents = []; beforeAll(() => { const workerHeapLimit = 10; @@ -246,6 +247,11 @@ describe.each([ const options = { childWorkerPath: workerPath, maxRetries: 0, + on: { + [WorkerEvents.STATE_CHANGE]: state => { + orderOfEvents.push(state); + }, + }, silent: true, workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), }; @@ -293,11 +299,24 @@ describe.each([ await worker.waitForExit(); + expect(worker.state).not.toEqual(WorkerStates.OK); + }); + + test('worker stays dead', async () => { await expect( async () => await worker.waitForWorkerReady(), ).rejects.toThrowError(); expect(worker.isWorkerRunning()).toBeFalsy(); }); + + test('expected state order', () => { + expect(orderOfEvents).toMatchObject([ + WorkerStates.OK, + WorkerStates.OUT_OF_MEMORY, + WorkerStates.SHUTTING_DOWN, + WorkerStates.SHUT_DOWN, + ]); + }); }, 15000); describe('should handle regular fatal crashes', () => { From 617e1023c32c5728ff5696b2bcf162332753100c Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 20:48:15 +0100 Subject: [PATCH 14/52] fix: only set out of memory when necessary --- packages/jest-worker/src/workers/ChildProcessWorker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 013a1e30b1c3..0422621f6319 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -205,7 +205,7 @@ export default class ChildProcessWorker let stderrStr = ''; const handler = (chunk: any) => { - if (this.state !== WorkerStates.OUT_OF_MEMORY) { + if (this.state === WorkerStates.OK) { let str: string | undefined = undefined; if (chunk instanceof Buffer) { From 28230daef8827fa85bb48ca3f2b88203b404d492 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 21:50:12 +0100 Subject: [PATCH 15/52] chore: windows debugging --- packages/jest-worker/src/workers/ChildProcessWorker.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 0422621f6319..52b2c83548e8 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -222,6 +222,8 @@ export default class ChildProcessWorker this.state = WorkerStates.OUT_OF_MEMORY; } } + + console.log({stderrStr}); }; child.stderr?.on('data', handler); From 60b5b1b68ae61129fa59ebb02fb4288b5fe5ad18 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 22:14:45 +0100 Subject: [PATCH 16/52] chore: try and debug single test --- .github/workflows/test.yml | 4 ++-- packages/jest-worker/src/workers/ChildProcessWorker.ts | 8 ++++++-- .../src/workers/__tests__/WorkerEdgeCases.test.js | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d7060b18db3a..1c399d7e98da 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,7 @@ jobs: id: cpu-cores uses: SimenB/github-actions-cpu-cores@v1 - name: run tests - run: yarn test-ci-partial:parallel --max-workers ${{ steps.cpu-cores.outputs.count }} --shard=${{ matrix.shard }} + run: yarn test-ci-partial:parallel WorkerEdgeCases test-jasmine: strategy: @@ -57,4 +57,4 @@ jobs: id: cpu-cores uses: SimenB/github-actions-cpu-cores@v1 - name: run tests using jest-jasmine - run: yarn jest-jasmine-ci --max-workers ${{ steps.cpu-cores.outputs.count }} --shard=${{ matrix.shard }} + run: yarn jest-jasmine-ci WorkerEdgeCases diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 52b2c83548e8..c9f8e3f1bc58 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -223,7 +223,7 @@ export default class ChildProcessWorker } } - console.log({stderrStr}); + console.log('ON DATA', {stderrStr}); }; child.stderr?.on('data', handler); @@ -245,6 +245,8 @@ export default class ChildProcessWorker // TODO: Add appropriate type check let error: any; + console.log('ON MESSAGE', {response}); + switch (response[0]) { case PARENT_MESSAGE_OK: this._onProcessEnd(null, response[1]); @@ -330,7 +332,9 @@ export default class ChildProcessWorker } } - private _onExit(exitCode: number | null) { + private _onExit(exitCode: number | null, signal: any) { + console.log('ON EXIT', {exitCode, signal}); + this._workerReadyPromise = undefined; this._resolveWorkerReady = undefined; diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index effe46298199..d8f23e63dd37 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -236,7 +236,7 @@ describe.each([ }); }); - describe('should cleanly exit on crash', () => { + describe.only('should cleanly exit on out of memory crash', () => { let worker; const orderOfEvents = []; From a9a3779b3f190bf971cee84c4d08ad1d30ff32b6 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 22:39:21 +0100 Subject: [PATCH 17/52] chore: remove debugging --- .github/workflows/test.yml | 4 ++-- .../jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1c399d7e98da..d7060b18db3a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,7 @@ jobs: id: cpu-cores uses: SimenB/github-actions-cpu-cores@v1 - name: run tests - run: yarn test-ci-partial:parallel WorkerEdgeCases + run: yarn test-ci-partial:parallel --max-workers ${{ steps.cpu-cores.outputs.count }} --shard=${{ matrix.shard }} test-jasmine: strategy: @@ -57,4 +57,4 @@ jobs: id: cpu-cores uses: SimenB/github-actions-cpu-cores@v1 - name: run tests using jest-jasmine - run: yarn jest-jasmine-ci WorkerEdgeCases + run: yarn jest-jasmine-ci --max-workers ${{ steps.cpu-cores.outputs.count }} --shard=${{ matrix.shard }} diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index d8f23e63dd37..c2c8f1e19821 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -236,7 +236,7 @@ describe.each([ }); }); - describe.only('should cleanly exit on out of memory crash', () => { + describe('should cleanly exit on out of memory crash', () => { let worker; const orderOfEvents = []; From cdf9627d3325c97cf2ba499fa44f43918d4b5633 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 23:07:38 +0100 Subject: [PATCH 18/52] chore: remove logging --- packages/jest-worker/src/workers/ChildProcessWorker.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index c9f8e3f1bc58..0422621f6319 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -222,8 +222,6 @@ export default class ChildProcessWorker this.state = WorkerStates.OUT_OF_MEMORY; } } - - console.log('ON DATA', {stderrStr}); }; child.stderr?.on('data', handler); @@ -245,8 +243,6 @@ export default class ChildProcessWorker // TODO: Add appropriate type check let error: any; - console.log('ON MESSAGE', {response}); - switch (response[0]) { case PARENT_MESSAGE_OK: this._onProcessEnd(null, response[1]); @@ -332,9 +328,7 @@ export default class ChildProcessWorker } } - private _onExit(exitCode: number | null, signal: any) { - console.log('ON EXIT', {exitCode, signal}); - + private _onExit(exitCode: number | null) { this._workerReadyPromise = undefined; this._resolveWorkerReady = undefined; From 7a79c925a1c37a83f7a960d12b35d2ec21a28f22 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 9 Aug 2022 23:40:59 +0100 Subject: [PATCH 19/52] chore: simplify tests and move more to abstract worker --- .../src/workers/ChildProcessWorker.ts | 36 ------------- .../src/workers/NodeThreadsWorker.ts | 39 ++------------- .../jest-worker/src/workers/WorkerAbstract.ts | 35 ++++++++++++- .../workers/__tests__/WorkerEdgeCases.test.js | 50 +++++-------------- 4 files changed, 51 insertions(+), 109 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 0422621f6319..67ddca5abdb4 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -7,7 +7,6 @@ import {ChildProcess, ForkOptions, fork} from 'child_process'; import {totalmem} from 'os'; -import {PassThrough} from 'stream'; import mergeStream = require('merge-stream'); import {stdout as stdoutSupportsColor} from 'supports-color'; import { @@ -66,13 +65,9 @@ export default class ChildProcessWorker private _onProcessEnd!: OnEnd; private _onCustomMessage!: OnCustomMessage; - private _fakeStream: PassThrough | null; private _stdout: ReturnType | null; private _stderr: ReturnType | null; - private _exitPromise: Promise; - private _resolveExitPromise!: () => void; - private _memoryUsagePromise: Promise | undefined; private _resolveMemoryUsage: ((arg0: number) => void) | undefined; @@ -89,19 +84,11 @@ export default class ChildProcessWorker this._request = null; - this._fakeStream = null; this._stdout = null; this._stderr = null; this._childIdleMemoryUsage = null; this._childIdleMemoryUsageLimit = options.idleMemoryLimit || null; - this._exitPromise = new Promise(resolve => { - this._resolveExitPromise = resolve; - }); - this._exitPromise.then(() => { - this.state = WorkerStates.SHUT_DOWN; - }); - this._childWorkerPath = options.childWorkerPath || require.resolve('./processChild'); @@ -227,18 +214,6 @@ export default class ChildProcessWorker child.stderr?.on('data', handler); } - private _shutdown() { - this.state = WorkerStates.SHUTTING_DOWN; - - // End the temporary streams so the merged streams end too - if (this._fakeStream) { - this._fakeStream.end(); - this._fakeStream = null; - } - - this._resolveExitPromise(); - } - private _onMessage(response: ParentMessage) { // TODO: Add appropriate type check let error: any; @@ -492,15 +467,4 @@ export default class ChildProcessWorker isWorkerRunning(): boolean { return this._child.connected && !this._child.killed; } - - private _getFakeStream() { - if (!this._fakeStream) { - this._fakeStream = new PassThrough(); - } - return this._fakeStream; - } - - getWorkerState(): WorkerStates { - return this.state; - } } diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 8d6ac6bd2a1c..8735404ed243 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -6,7 +6,6 @@ */ import {totalmem} from 'os'; -import {PassThrough} from 'stream'; import {Worker} from 'worker_threads'; import mergeStream = require('merge-stream'); import { @@ -40,13 +39,9 @@ export default class ExperimentalWorker private _onProcessEnd!: OnEnd; private _onCustomMessage!: OnCustomMessage; - private _fakeStream: PassThrough | null; private _stdout: ReturnType | null; private _stderr: ReturnType | null; - private _exitPromise: Promise; - private _resolveExitPromise!: () => void; - private _memoryUsagePromise: Promise | undefined; private _resolveMemoryUsage: ((arg0: number) => void) | undefined; @@ -63,7 +58,6 @@ export default class ExperimentalWorker this._request = null; - this._fakeStream = null; this._stdout = null; this._stderr = null; @@ -73,13 +67,6 @@ export default class ExperimentalWorker this._childIdleMemoryUsage = null; this._childIdleMemoryUsageLimit = options.idleMemoryLimit || null; - this._exitPromise = new Promise(resolve => { - this._resolveExitPromise = resolve; - }); - this._exitPromise.then(() => { - this.state = WorkerStates.SHUT_DOWN; - }); - this.initialize(); } @@ -171,21 +158,14 @@ export default class ExperimentalWorker } } - private _shutdown() { - // End the permanent stream so the merged stream end too - if (this._fakeStream) { - this._fakeStream.end(); - this._fakeStream = null; - } - - this._resolveExitPromise(); - } - private _onError(error: Error) { if (error.message.includes('heap out of memory')) { this.state = WorkerStates.OUT_OF_MEMORY; - this.forceExit(); + // Threads don't behave like processes, they don't crash when they run out of + // memory. But for consistency we want them to behave like processes so we call + // terminate to simulate a crash happening that was not planned + this._worker.terminate(); } } @@ -423,18 +403,7 @@ export default class ExperimentalWorker return this._worker.threadId; } - private _getFakeStream() { - if (!this._fakeStream) { - this._fakeStream = new PassThrough(); - } - return this._fakeStream; - } - isWorkerRunning(): boolean { return this._worker.threadId >= 0; } - - getWorkerState(): WorkerStates { - return this.state; - } } diff --git a/packages/jest-worker/src/workers/WorkerAbstract.ts b/packages/jest-worker/src/workers/WorkerAbstract.ts index b476d75b4d62..0cf97a3ee66f 100644 --- a/packages/jest-worker/src/workers/WorkerAbstract.ts +++ b/packages/jest-worker/src/workers/WorkerAbstract.ts @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import {EventEmitter} from 'stream'; +import {EventEmitter, PassThrough} from 'stream'; import { WorkerEvents, WorkerInterface, @@ -19,6 +19,11 @@ export default abstract class WorkerAbstract { private __state = WorkerStates.STARTING; + protected _fakeStream: PassThrough | null = null; + + protected _exitPromise: Promise; + protected _resolveExitPromise!: () => void; + protected _workerReadyPromise: Promise | undefined; protected _resolveWorkerReady: (() => void) | undefined; @@ -48,6 +53,13 @@ export default abstract class WorkerAbstract } } } + + this._exitPromise = new Promise(resolve => { + this._resolveExitPromise = resolve; + }); + this._exitPromise.then(() => { + this.state = WorkerStates.SHUT_DOWN; + }); } /** @@ -101,4 +113,25 @@ export default abstract class WorkerAbstract return this._workerReadyPromise; } + + /** + * Used to shut down the current working instance once the children have been + * killed off. + */ + protected _shutdown(): void { + // End the permanent stream so the merged stream end too + if (this._fakeStream) { + this._fakeStream.end(); + this._fakeStream = null; + } + + this._resolveExitPromise(); + } + + protected _getFakeStream(): PassThrough { + if (!this._fakeStream) { + this._fakeStream = new PassThrough(); + } + return this._fakeStream; + } } diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index c2c8f1e19821..0cb6f9a1b778 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -19,6 +19,8 @@ import { import ChildProcessWorker, {SIGKILL_DELAY} from '../ChildProcessWorker'; import ThreadsWorker from '../NodeThreadsWorker'; +jest.retryTimes(0); + const root = join('../../'); const filesToBuild = ['workers/processChild', 'workers/threadChild', 'types']; const writeDestination = join(__dirname, '__temp__'); @@ -313,7 +315,6 @@ describe.each([ expect(orderOfEvents).toMatchObject([ WorkerStates.OK, WorkerStates.OUT_OF_MEMORY, - WorkerStates.SHUTTING_DOWN, WorkerStates.SHUT_DOWN, ]); }); @@ -321,11 +322,19 @@ describe.each([ describe('should handle regular fatal crashes', () => { let worker; + let startedWorkers = 0; beforeAll(() => { worker = new workerClass({ childWorkerPath: workerPath, maxRetries: 4, + on: { + [WorkerEvents.STATE_CHANGE]: state => { + if (state === WorkerStates.OK) { + startedWorkers++; + } + }, + }, workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), }); }); @@ -354,43 +363,10 @@ describe.each([ onCustom, ); - let pidChanges = 0; - - while (true) { - // Ideally this would use Promise.any but it's not supported in Node 14 - // so doing this instead. Essentially what we're doing is looping and - // capturing the pid every time it changes. When it stops changing the - // timeout will be hit and we should be left with a collection of all - // the pids used by the worker. - const newPid = await new Promise(resolve => { - const resolved = false; - - const to = setTimeout(() => { - if (!resolved) { - this.resolved = true; - resolve(undefined); - } - }, 500); - - waitForChange(() => worker.getWorkerSystemId()).then(() => { - clearTimeout(to); - - if (!resolved) { - resolve(worker.getWorkerSystemId()); - } - }); - }); - - if (typeof newPid === 'number') { - pidChanges++; - } else { - break; - } - } + // Give it some time to restart some workers + await new Promise(resolve => setTimeout(resolve, 4000)); - // Expect the pids to be retries + 1 because it is restarted - // one last time at the end ready for the next request. - expect(pidChanges).toEqual(5); + expect(startedWorkers).toEqual(6); expect(worker.isWorkerRunning()).toBeTruthy(); expect(worker.state).toEqual(WorkerStates.OK); From 11fd6f8fffe201fc5bb60eebc7ace2344af99f10 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 07:57:39 +0100 Subject: [PATCH 20/52] chore: only run flaky test --- .circleci/config.yml | 4 ++-- .github/workflows/test.yml | 4 ++-- e2e/__tests__/worker-restarting.test.ts | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 e2e/__tests__/worker-restarting.test.ts diff --git a/.circleci/config.yml b/.circleci/config.yml index 61be28b5a409..e2d4bcc073fb 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -28,7 +28,7 @@ jobs: node-version: << parameters.node-version >> - node/install-packages: *install - run: - command: yarn test-ci-partial --shard=$(expr $CIRCLE_NODE_INDEX + 1)/$CIRCLE_NODE_TOTAL + command: yarn test-ci-partial WorkerEdgeCases - store_test_results: path: reports/junit @@ -43,7 +43,7 @@ jobs: - node/install-packages: *install - run: name: Test - command: JEST_JASMINE=1 yarn test-ci-partial --shard=$(expr $CIRCLE_NODE_INDEX + 1)/$CIRCLE_NODE_TOTAL + command: JEST_JASMINE=1 yarn test-ci-partial WorkerEdgeCases - run: name: Leak test command: JEST_JASMINE=1 yarn test-leak diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d7060b18db3a..1c399d7e98da 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,7 @@ jobs: id: cpu-cores uses: SimenB/github-actions-cpu-cores@v1 - name: run tests - run: yarn test-ci-partial:parallel --max-workers ${{ steps.cpu-cores.outputs.count }} --shard=${{ matrix.shard }} + run: yarn test-ci-partial:parallel WorkerEdgeCases test-jasmine: strategy: @@ -57,4 +57,4 @@ jobs: id: cpu-cores uses: SimenB/github-actions-cpu-cores@v1 - name: run tests using jest-jasmine - run: yarn jest-jasmine-ci --max-workers ${{ steps.cpu-cores.outputs.count }} --shard=${{ matrix.shard }} + run: yarn jest-jasmine-ci WorkerEdgeCases diff --git a/e2e/__tests__/worker-restarting.test.ts b/e2e/__tests__/worker-restarting.test.ts new file mode 100644 index 000000000000..e6446adc6def --- /dev/null +++ b/e2e/__tests__/worker-restarting.test.ts @@ -0,0 +1,16 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {extractSummary} from '../Utils'; +import runJest from '../runJest'; + +it('all 3 test files should complete', () => { + const result = runJest('worker-restating'); + expect(result.exitCode).toBe(0); + const {summary} = extractSummary(result.stderr); + expect(summary).toMatchSnapshot(); +}); From 55f4097a4168b131d68b03588d0a659c7b3bf33a Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 11:44:00 +0100 Subject: [PATCH 21/52] chore: enable tests again --- .circleci/config.yml | 4 ++-- .github/workflows/test.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index e2d4bcc073fb..61be28b5a409 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -28,7 +28,7 @@ jobs: node-version: << parameters.node-version >> - node/install-packages: *install - run: - command: yarn test-ci-partial WorkerEdgeCases + command: yarn test-ci-partial --shard=$(expr $CIRCLE_NODE_INDEX + 1)/$CIRCLE_NODE_TOTAL - store_test_results: path: reports/junit @@ -43,7 +43,7 @@ jobs: - node/install-packages: *install - run: name: Test - command: JEST_JASMINE=1 yarn test-ci-partial WorkerEdgeCases + command: JEST_JASMINE=1 yarn test-ci-partial --shard=$(expr $CIRCLE_NODE_INDEX + 1)/$CIRCLE_NODE_TOTAL - run: name: Leak test command: JEST_JASMINE=1 yarn test-leak diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1c399d7e98da..d7060b18db3a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,7 @@ jobs: id: cpu-cores uses: SimenB/github-actions-cpu-cores@v1 - name: run tests - run: yarn test-ci-partial:parallel WorkerEdgeCases + run: yarn test-ci-partial:parallel --max-workers ${{ steps.cpu-cores.outputs.count }} --shard=${{ matrix.shard }} test-jasmine: strategy: @@ -57,4 +57,4 @@ jobs: id: cpu-cores uses: SimenB/github-actions-cpu-cores@v1 - name: run tests using jest-jasmine - run: yarn jest-jasmine-ci WorkerEdgeCases + run: yarn jest-jasmine-ci --max-workers ${{ steps.cpu-cores.outputs.count }} --shard=${{ matrix.shard }} From 723951b225692a4a3a60ac599916d69b1fd87c12 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 13:41:44 +0100 Subject: [PATCH 22/52] fix: missing snapshot --- .../__snapshots__/workerRestarting.test.ts.snap | 9 +++++++++ ...orker-restarting.test.ts => workerRestarting.test.ts} | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 e2e/__tests__/__snapshots__/workerRestarting.test.ts.snap rename e2e/__tests__/{worker-restarting.test.ts => workerRestarting.test.ts} (90%) diff --git a/e2e/__tests__/__snapshots__/workerRestarting.test.ts.snap b/e2e/__tests__/__snapshots__/workerRestarting.test.ts.snap new file mode 100644 index 000000000000..edf50a0f2d69 --- /dev/null +++ b/e2e/__tests__/__snapshots__/workerRestarting.test.ts.snap @@ -0,0 +1,9 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`all 3 test files should complete 1`] = ` +"Test Suites: 3 passed, 3 total +Tests: 3 passed, 3 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; diff --git a/e2e/__tests__/worker-restarting.test.ts b/e2e/__tests__/workerRestarting.test.ts similarity index 90% rename from e2e/__tests__/worker-restarting.test.ts rename to e2e/__tests__/workerRestarting.test.ts index e6446adc6def..aa8cb425f77b 100644 --- a/e2e/__tests__/worker-restarting.test.ts +++ b/e2e/__tests__/workerRestarting.test.ts @@ -9,7 +9,7 @@ import {extractSummary} from '../Utils'; import runJest from '../runJest'; it('all 3 test files should complete', () => { - const result = runJest('worker-restating'); + const result = runJest('worker-restarting'); expect(result.exitCode).toBe(0); const {summary} = extractSummary(result.stderr); expect(summary).toMatchSnapshot(); From a062850977b6866486eff4cfd91554890a3e2ae9 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 13:49:05 +0100 Subject: [PATCH 23/52] chore: try to resolve the flake --- packages/jest-worker/src/workers/ChildProcessWorker.ts | 2 +- .../src/workers/__tests__/WorkerEdgeCases.test.js | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 67ddca5abdb4..a08999c8df1d 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -128,6 +128,7 @@ export default class ChildProcessWorker }; this._child = fork(this._childWorkerPath, [], options); + this._detectOutOfMemoryCrash(this._child); if (this._child.stdout) { if (!this._stdout) { @@ -149,7 +150,6 @@ export default class ChildProcessWorker this._stderr.add(this._child.stderr); } - this._detectOutOfMemoryCrash(this._child); this._child.on('message', this._onMessage.bind(this)); this._child.on('exit', this._onExit.bind(this)); diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 0cb6f9a1b778..08d0cefe55bb 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -19,7 +19,7 @@ import { import ChildProcessWorker, {SIGKILL_DELAY} from '../ChildProcessWorker'; import ThreadsWorker from '../NodeThreadsWorker'; -jest.retryTimes(0); +jest.retryTimes(5); const root = join('../../'); const filesToBuild = ['workers/processChild', 'workers/threadChild', 'types']; @@ -243,7 +243,7 @@ describe.each([ const orderOfEvents = []; beforeAll(() => { - const workerHeapLimit = 10; + const workerHeapLimit = 50; /** @type WorkerOptions */ const options = { @@ -287,6 +287,11 @@ describe.each([ expect(startPid).toBeGreaterThanOrEqual(0); }); + test('worker ready', async () => { + await worker.waitForWorkerReady(); + expect(worker.state).toEqual(WorkerStates.OK); + }); + test('worker crashes and exits', async () => { const onStart = jest.fn(); const onEnd = jest.fn(); From 66c83bd9f3afc597b9115f7567b34f924ecfbab0 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 14:13:07 +0100 Subject: [PATCH 24/52] chore: logging --- jest.config.ci.mjs | 9 +++++---- .../src/workers/__tests__/WorkerEdgeCases.test.js | 6 ++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/jest.config.ci.mjs b/jest.config.ci.mjs index 2b2f36983f5e..fbf7ab6f1590 100644 --- a/jest.config.ci.mjs +++ b/jest.config.ci.mjs @@ -12,15 +12,16 @@ export default { ...baseConfig, coverageReporters: ['json'], reporters: [ + 'default', 'github-actions', [ 'jest-junit', {outputDirectory: 'reports/junit', outputName: 'js-test-results.xml'}, ], - [ - 'jest-silent-reporter', - {showPaths: true, showWarnings: true, useDots: true}, - ], + // [ + // 'jest-silent-reporter', + // {showPaths: true, showWarnings: true, useDots: true}, + // ], 'summary', ], }; diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 08d0cefe55bb..6b06d9c2a881 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -239,11 +239,13 @@ describe.each([ }); describe('should cleanly exit on out of memory crash', () => { + const workerHeapLimit = 50; + let worker; - const orderOfEvents = []; + let orderOfEvents = []; beforeAll(() => { - const workerHeapLimit = 50; + orderOfEvents = []; /** @type WorkerOptions */ const options = { From c47404de3b6b928292b2b6efb05445c4754b562d Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 14:36:56 +0100 Subject: [PATCH 25/52] chore: see if this changes debugging output --- packages/jest-worker/src/workers/ChildProcessWorker.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index a08999c8df1d..de5df13ff5fa 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -322,6 +322,8 @@ export default class ChildProcessWorker this.state !== WorkerStates.SHUTTING_DOWN) || this.state === WorkerStates.RESTARTING ) { + this.state = WorkerStates.RESTARTING; + this.initialize(); if (this._request) { From 9331bd228a7a455c88629c395b5138ce69354ce6 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 14:52:03 +0100 Subject: [PATCH 26/52] chore: make noisy --- .../jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 6b06d9c2a881..d02d3379a188 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -175,6 +175,7 @@ describe.each([ orderOfEvents.push(state); }, }, + silent: false, workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), }); }); From 5058e144c6b084d0d2616b007ca3889e3cb91762 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 15:09:32 +0100 Subject: [PATCH 27/52] chore: debugging --- packages/jest-worker/src/workers/ChildProcessWorker.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index de5df13ff5fa..49fea2d1647d 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -303,7 +303,7 @@ export default class ChildProcessWorker } } - private _onExit(exitCode: number | null) { + private _onExit(exitCode: number | null, signal: unknown) { this._workerReadyPromise = undefined; this._resolveWorkerReady = undefined; @@ -322,6 +322,11 @@ export default class ChildProcessWorker this.state !== WorkerStates.SHUTTING_DOWN) || this.state === WorkerStates.RESTARTING ) { + console.log({ + exitCode, + signal, + }); + this.state = WorkerStates.RESTARTING; this.initialize(); From ddbfb5e5c73572042fcd6e5ebc8a10e828aef56f Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 15:30:42 +0100 Subject: [PATCH 28/52] chore: more --- .../jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index d02d3379a188..c5dac73d6366 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -257,7 +257,7 @@ describe.each([ orderOfEvents.push(state); }, }, - silent: true, + silent: false, workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), }; From 811e2288d3d37c718ca3862b99e03fccdd6165d1 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 16:18:31 +0100 Subject: [PATCH 29/52] chore: even more robust logging --- .../src/workers/ChildProcessWorker.ts | 56 +++++++++++++------ .../workers/__tests__/WorkerEdgeCases.test.js | 4 +- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 49fea2d1647d..511de2c804da 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -112,6 +112,12 @@ export default class ChildProcessWorker this.state = WorkerStates.STARTING; const forceColor = stdoutSupportsColor ? {FORCE_COLOR: '1'} : {}; + const silent = this._options.silent ?? true; + + if (!silent) { + console.warn('Unable to detect out of memory event if silent === false'); + } + const options: ForkOptions = { cwd: process.cwd(), env: { @@ -123,7 +129,7 @@ export default class ChildProcessWorker execArgv: process.execArgv.filter(v => !/^--(debug|inspect)/.test(v)), // default to advanced serialization in order to match worker threads serialization: 'advanced', - silent: this._options.silent ?? true, + silent, ...this._options.forkOptions, }; @@ -189,29 +195,43 @@ export default class ChildProcessWorker } private _detectOutOfMemoryCrash(child: ChildProcess): void { - let stderrStr = ''; - - const handler = (chunk: any) => { - if (this.state === WorkerStates.OK) { - let str: string | undefined = undefined; + const createHandler = (stream: string) => { + let bufferStr = ''; + + return (chunk: unknown) => { + if ( + this.state === WorkerStates.OK || + this.state === WorkerStates.STARTING + ) { + let str: string | undefined = undefined; + + if (chunk instanceof Buffer) { + str = chunk.toString('utf8'); + } else if (typeof chunk === 'string') { + str = chunk; + } - if (chunk instanceof Buffer) { - str = chunk.toString('utf8'); - } else if (typeof chunk === 'string') { - str = chunk; - } + if (str) { + bufferStr += str; + } - if (str) { - stderrStr += str; + if (bufferStr.includes('heap out of memory')) { + this.state = WorkerStates.OUT_OF_MEMORY; + } } - if (stderrStr.includes('heap out of memory')) { - this.state = WorkerStates.OUT_OF_MEMORY; - } - } + // eslint-disable-next-line sort-keys + console.log({stream, state: this.state, bufferStr}); + }; }; - child.stderr?.on('data', handler); + const stderrHandler = createHandler('stderr'); + child.stderr?.on('data', stderrHandler); + child.stderr?.on('end', stderrHandler); + + const stdoutHandler = createHandler('stdout'); + child.stdout?.on('data', stdoutHandler); + child.stdout?.on('end', stdoutHandler); } private _onMessage(response: ParentMessage) { diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index c5dac73d6366..6cf3f4afc24d 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -175,7 +175,7 @@ describe.each([ orderOfEvents.push(state); }, }, - silent: false, + silent: true, workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), }); }); @@ -257,7 +257,7 @@ describe.each([ orderOfEvents.push(state); }, }, - silent: false, + silent: true, workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), }; From 197c0daf07ddf7c686d1f2ba54bae8bcdf47792d Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 16:41:08 +0100 Subject: [PATCH 30/52] chore: refactor error streaming monitoring --- .../src/workers/ChildProcessWorker.ts | 57 ++++++++----------- .../workers/__tests__/WorkerEdgeCases.test.js | 2 +- 2 files changed, 24 insertions(+), 35 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 511de2c804da..5905eed5fcf3 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -195,43 +195,37 @@ export default class ChildProcessWorker } private _detectOutOfMemoryCrash(child: ChildProcess): void { - const createHandler = (stream: string) => { - let bufferStr = ''; - - return (chunk: unknown) => { - if ( - this.state === WorkerStates.OK || - this.state === WorkerStates.STARTING - ) { - let str: string | undefined = undefined; - - if (chunk instanceof Buffer) { - str = chunk.toString('utf8'); - } else if (typeof chunk === 'string') { - str = chunk; - } + const setupHandler = ( + stream: keyof Pick, + ) => { + const readableStream = child[stream]; + + if (readableStream) { + const buffer: Array = []; - if (str) { - bufferStr += str; + readableStream.on('data', chunk => { + if (chunk) { + buffer.push(Buffer.from(chunk)); } + }); + readableStream.on('end', () => { + const bufferStr = Buffer.concat(buffer).toString('utf8'); if (bufferStr.includes('heap out of memory')) { this.state = WorkerStates.OUT_OF_MEMORY; } - } - // eslint-disable-next-line sort-keys - console.log({stream, state: this.state, bufferStr}); - }; + console.log({ + state: this.state, + stream, + bufferStr, + }); + }); + } }; - const stderrHandler = createHandler('stderr'); - child.stderr?.on('data', stderrHandler); - child.stderr?.on('end', stderrHandler); - - const stdoutHandler = createHandler('stdout'); - child.stdout?.on('data', stdoutHandler); - child.stdout?.on('end', stdoutHandler); + setupHandler('stderr'); + setupHandler('stdout'); } private _onMessage(response: ParentMessage) { @@ -323,7 +317,7 @@ export default class ChildProcessWorker } } - private _onExit(exitCode: number | null, signal: unknown) { + private _onExit(exitCode: number | null) { this._workerReadyPromise = undefined; this._resolveWorkerReady = undefined; @@ -342,11 +336,6 @@ export default class ChildProcessWorker this.state !== WorkerStates.SHUTTING_DOWN) || this.state === WorkerStates.RESTARTING ) { - console.log({ - exitCode, - signal, - }); - this.state = WorkerStates.RESTARTING; this.initialize(); diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 6cf3f4afc24d..ac941ef23c7d 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -239,7 +239,7 @@ describe.each([ }); }); - describe('should cleanly exit on out of memory crash', () => { + describe.only('should cleanly exit on out of memory crash', () => { const workerHeapLimit = 50; let worker; From 0d0d03e0aca0630f5279fe57ad84c42605eab9c0 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 16:43:26 +0100 Subject: [PATCH 31/52] chore: error handling --- packages/jest-worker/src/workers/ChildProcessWorker.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 5905eed5fcf3..3e2a00333a13 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -134,7 +134,6 @@ export default class ChildProcessWorker }; this._child = fork(this._childWorkerPath, [], options); - this._detectOutOfMemoryCrash(this._child); if (this._child.stdout) { if (!this._stdout) { @@ -156,6 +155,8 @@ export default class ChildProcessWorker this._stderr.add(this._child.stderr); } + this._detectOutOfMemoryCrash(this._child); + this._child.on('error', this._onError.bind(this)); this._child.on('message', this._onMessage.bind(this)); this._child.on('exit', this._onExit.bind(this)); @@ -228,6 +229,12 @@ export default class ChildProcessWorker setupHandler('stdout'); } + private _onError(error: Error) { + if (error.message.includes('heap out of memory')) { + this.state = WorkerStates.OUT_OF_MEMORY; + } + } + private _onMessage(response: ParentMessage) { // TODO: Add appropriate type check let error: any; From 11e8b25f955e693cc48591654e63ac20288646c1 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 17:12:12 +0100 Subject: [PATCH 32/52] chore: log this --- .../src/workers/ChildProcessWorker.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 3e2a00333a13..ec0a6312c78b 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -213,14 +213,19 @@ export default class ChildProcessWorker const bufferStr = Buffer.concat(buffer).toString('utf8'); if (bufferStr.includes('heap out of memory')) { - this.state = WorkerStates.OUT_OF_MEMORY; + if ( + this.state === WorkerStates.OK || + this.state === WorkerStates.STARTING + ) { + this.state = WorkerStates.OUT_OF_MEMORY; + } else { + console.log({ + state: this.state, + stream, + bufferStr, + }); + } } - - console.log({ - state: this.state, - stream, - bufferStr, - }); }); } }; From 8f427a42ecbe8804f2cc8ee67c2724900ca31c68 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 19:18:19 +0100 Subject: [PATCH 33/52] fix: tests and try to detect process crashes differently on windows --- .../src/workers/ChildProcessWorker.ts | 79 +++++++++---------- .../__tests__/ChildProcessWorker.test.js | 1 + .../workers/__tests__/WorkerEdgeCases.test.js | 3 +- 3 files changed, 41 insertions(+), 42 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index ec0a6312c78b..6e239c7f2177 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -68,6 +68,9 @@ export default class ChildProcessWorker private _stdout: ReturnType | null; private _stderr: ReturnType | null; + private _stdoutBuffer: Array = []; + private _stderrBuffer: Array = []; + private _memoryUsagePromise: Promise | undefined; private _resolveMemoryUsage: ((arg0: number) => void) | undefined; @@ -118,6 +121,9 @@ export default class ChildProcessWorker console.warn('Unable to detect out of memory event if silent === false'); } + this._stdoutBuffer = []; + this._stderrBuffer = []; + const options: ForkOptions = { cwd: process.cwd(), env: { @@ -143,6 +149,12 @@ export default class ChildProcessWorker } this._stdout.add(this._child.stdout); + + this._child.stdout.on('data', chunk => { + if (chunk) { + this._stdoutBuffer.push(Buffer.from(chunk)); + } + }); } if (this._child.stderr) { @@ -153,10 +165,14 @@ export default class ChildProcessWorker } this._stderr.add(this._child.stderr); + + this._child.stderr.on('data', chunk => { + if (chunk) { + this._stderrBuffer.push(Buffer.from(chunk)); + } + }); } - this._detectOutOfMemoryCrash(this._child); - this._child.on('error', this._onError.bind(this)); this._child.on('message', this._onMessage.bind(this)); this._child.on('exit', this._onExit.bind(this)); @@ -195,48 +211,24 @@ export default class ChildProcessWorker } } - private _detectOutOfMemoryCrash(child: ChildProcess): void { - const setupHandler = ( - stream: keyof Pick, - ) => { - const readableStream = child[stream]; + private _detectOutOfMemoryCrash(): void { + for (const buffer of [this._stderrBuffer, this._stdoutBuffer]) { + const bufferStr = Buffer.concat(buffer).toString('utf8'); - if (readableStream) { - const buffer: Array = []; + console.log({ + bufferStr, + state: this.state, + oom: bufferStr.includes('heap out of memory'), + }); - readableStream.on('data', chunk => { - if (chunk) { - buffer.push(Buffer.from(chunk)); - } - }); - readableStream.on('end', () => { - const bufferStr = Buffer.concat(buffer).toString('utf8'); - - if (bufferStr.includes('heap out of memory')) { - if ( - this.state === WorkerStates.OK || - this.state === WorkerStates.STARTING - ) { - this.state = WorkerStates.OUT_OF_MEMORY; - } else { - console.log({ - state: this.state, - stream, - bufferStr, - }); - } - } - }); + if (bufferStr.includes('heap out of memory')) { + if ( + this.state === WorkerStates.OK || + this.state === WorkerStates.STARTING + ) { + this.state = WorkerStates.OUT_OF_MEMORY; + } } - }; - - setupHandler('stderr'); - setupHandler('stdout'); - } - - private _onError(error: Error) { - if (error.message.includes('heap out of memory')) { - this.state = WorkerStates.OUT_OF_MEMORY; } } @@ -333,6 +325,8 @@ export default class ChildProcessWorker this._workerReadyPromise = undefined; this._resolveWorkerReady = undefined; + this._detectOutOfMemoryCrash(); + if (exitCode !== 0 && this.state === WorkerStates.OUT_OF_MEMORY) { this._onProcessEnd( new Error('Jest worker ran out of memory and crashed'), @@ -366,6 +360,9 @@ export default class ChildProcessWorker onProcessEnd: OnEnd, onCustomMessage: OnCustomMessage, ): void { + this._stdoutBuffer = []; + this._stderrBuffer = []; + onProcessStart(this); this._onProcessEnd = (...args) => { diff --git a/packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js b/packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js index 0948ea82be57..bce2d97a3bea 100644 --- a/packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js +++ b/packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js @@ -430,6 +430,7 @@ it('when out of memory occurs the worker is killed and exits', async () => { 1: 0x10da153a5 node::Abort() (.cold.1) [/Users/paul/.nvm/versions/node/v16.10.0/bin/node] 2: 0x10c6f09b9 node::Abort() [/Users/paul/.nvm/versions/node/v16.10.0/bin/node]`, ); + forkInterface.stderr.emit('end'); forkInterface.emit('exit', null, 'SIGABRT'); diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index ac941ef23c7d..a3604f0f333b 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -19,7 +19,8 @@ import { import ChildProcessWorker, {SIGKILL_DELAY} from '../ChildProcessWorker'; import ThreadsWorker from '../NodeThreadsWorker'; -jest.retryTimes(5); +jest.retryTimes(0); +jest.setTimeout(1000); const root = join('../../'); const filesToBuild = ['workers/processChild', 'workers/threadChild', 'types']; From 57d3b2066120a2642ec25fd56e5903965c549f69 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 19:32:02 +0100 Subject: [PATCH 34/52] chore: remove debug logging --- .../src/workers/ChildProcessWorker.ts | 32 +++++++++++-------- .../workers/__tests__/WorkerEdgeCases.test.js | 2 +- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 6e239c7f2177..d6e7163cb077 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -213,21 +213,25 @@ export default class ChildProcessWorker private _detectOutOfMemoryCrash(): void { for (const buffer of [this._stderrBuffer, this._stdoutBuffer]) { - const bufferStr = Buffer.concat(buffer).toString('utf8'); - - console.log({ - bufferStr, - state: this.state, - oom: bufferStr.includes('heap out of memory'), - }); - - if (bufferStr.includes('heap out of memory')) { - if ( - this.state === WorkerStates.OK || - this.state === WorkerStates.STARTING - ) { - this.state = WorkerStates.OUT_OF_MEMORY; + try { + const bufferStr = Buffer.concat(buffer).toString('utf8'); + + // console.log({ + // bufferStr, + // state: this.state, + // oom: bufferStr.includes('heap out of memory'), + // }); + + if (bufferStr.includes('heap out of memory')) { + if ( + this.state === WorkerStates.OK || + this.state === WorkerStates.STARTING + ) { + this.state = WorkerStates.OUT_OF_MEMORY; + } } + } catch (err) { + console.error('Error looking for out of memory crash', err); } } } diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index a3604f0f333b..8c0b7e9822ea 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -20,7 +20,7 @@ import ChildProcessWorker, {SIGKILL_DELAY} from '../ChildProcessWorker'; import ThreadsWorker from '../NodeThreadsWorker'; jest.retryTimes(0); -jest.setTimeout(1000); +jest.setTimeout(10000); const root = join('../../'); const filesToBuild = ['workers/processChild', 'workers/threadChild', 'types']; From d3abe1413751c63cec7696e1b1663b7e29e70369 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 19:34:13 +0100 Subject: [PATCH 35/52] chore: remove only --- .../jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 8c0b7e9822ea..2a63effa18b7 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -240,7 +240,7 @@ describe.each([ }); }); - describe.only('should cleanly exit on out of memory crash', () => { + describe('should cleanly exit on out of memory crash', () => { const workerHeapLimit = 50; let worker; From f704620f901cb16828e9f619aafb95037ad8c442 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 20:32:56 +0100 Subject: [PATCH 36/52] chore: add win32 specific logic to try and work around the issue --- .../src/workers/ChildProcessWorker.ts | 31 +++++++++++++------ .../__tests__/__fixtures__/EdgeCasesWorker.js | 2 +- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index d6e7163cb077..68666ba5551c 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -29,6 +29,7 @@ import { import WorkerAbstract from './WorkerAbstract'; const SIGNAL_BASE_EXIT_CODE = 128; +const SIGABRT_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 6; const SIGKILL_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 9; const SIGTERM_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 15; @@ -211,17 +212,14 @@ export default class ChildProcessWorker } } - private _detectOutOfMemoryCrash(): void { + private _detectOutOfMemoryCrash( + exitCode?: number | null, + signal?: NodeJS.Signals | null, + ): void { for (const buffer of [this._stderrBuffer, this._stdoutBuffer]) { try { const bufferStr = Buffer.concat(buffer).toString('utf8'); - // console.log({ - // bufferStr, - // state: this.state, - // oom: bufferStr.includes('heap out of memory'), - // }); - if (bufferStr.includes('heap out of memory')) { if ( this.state === WorkerStates.OK || @@ -234,6 +232,21 @@ export default class ChildProcessWorker console.error('Error looking for out of memory crash', err); } } + + // This is to try and work around a weird edge case when for some reason Windows doesn't feel + // like sending stderr, so we end up with no data to look at. + if (process.platform === 'win32') { + if (!this._stderrBuffer.length && !this._stdoutBuffer.length) { + if (exitCode === SIGABRT_EXIT_CODE && signal === null) { + if ( + this.state === WorkerStates.OK || + this.state === WorkerStates.STARTING + ) { + this.state = WorkerStates.OUT_OF_MEMORY; + } + } + } + } } private _onMessage(response: ParentMessage) { @@ -325,11 +338,11 @@ export default class ChildProcessWorker } } - private _onExit(exitCode: number | null) { + private _onExit(exitCode: number | null, signal: NodeJS.Signals | null) { this._workerReadyPromise = undefined; this._resolveWorkerReady = undefined; - this._detectOutOfMemoryCrash(); + this._detectOutOfMemoryCrash(exitCode, signal); if (exitCode !== 0 && this.state === WorkerStates.OUT_OF_MEMORY) { this._onProcessEnd( diff --git a/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js b/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js index 482fd7c33508..8791d1213ba6 100644 --- a/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js +++ b/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js @@ -28,7 +28,7 @@ async function leakMemory() { } function fatalExitCode() { - process.exit(134); + process.exit(139); } function safeFunction() { From 5a840bbe8a6e7c29e79215b2ffbd1917cdff79ac Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 21:00:37 +0100 Subject: [PATCH 37/52] chore: back to adding debug logging --- .github/workflows/test.yml | 2 +- .../src/workers/ChildProcessWorker.ts | 19 +++++++++++++++++++ .../workers/__tests__/WorkerEdgeCases.test.js | 2 +- .../__tests__/__fixtures__/EdgeCasesWorker.js | 4 ++++ 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d7060b18db3a..1bfb6a20ecee 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,7 @@ jobs: id: cpu-cores uses: SimenB/github-actions-cpu-cores@v1 - name: run tests - run: yarn test-ci-partial:parallel --max-workers ${{ steps.cpu-cores.outputs.count }} --shard=${{ matrix.shard }} + run: yarn test-ci-partial:parallel --max-workers ${{ steps.cpu-cores.outputs.count }} WorkerEdgeCases test-jasmine: strategy: diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 68666ba5551c..67a1fc7590c2 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -342,8 +342,27 @@ export default class ChildProcessWorker this._workerReadyPromise = undefined; this._resolveWorkerReady = undefined; + console.log('PRE', { + exitCode, + signal, + stderr: Buffer.concat(this._stderrBuffer).toString('utf8'), + stdout: Buffer.concat(this._stdoutBuffer).toString('utf8'), + childExitCode: this._child.exitCode, + childConnected: this._child.connected, + childKilled: this._child.killed, + state: this.state, + platform: process.platform, + }); + this._detectOutOfMemoryCrash(exitCode, signal); + console.log('POST', { + exitCode, + signal, + state: this.state, + platform: process.platform, + }); + if (exitCode !== 0 && this.state === WorkerStates.OUT_OF_MEMORY) { this._onProcessEnd( new Error('Jest worker ran out of memory and crashed'), diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 2a63effa18b7..8c0b7e9822ea 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -240,7 +240,7 @@ describe.each([ }); }); - describe('should cleanly exit on out of memory crash', () => { + describe.only('should cleanly exit on out of memory crash', () => { const workerHeapLimit = 50; let worker; diff --git a/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js b/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js index 8791d1213ba6..50ab3bdc3c91 100644 --- a/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js +++ b/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js @@ -31,6 +31,10 @@ function fatalExitCode() { process.exit(139); } +function weirdWindowsOutOfMemoryExit() { + process.exit(134); +} + function safeFunction() { // Doesn't do anything. } From d6b2db01db595c15d5982003f269d58ad55b681d Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 21:18:21 +0100 Subject: [PATCH 38/52] chore: try adding more possible detections --- .../src/workers/ChildProcessWorker.ts | 69 ++++++++++++------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 67a1fc7590c2..7b9877dc2b81 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -69,7 +69,6 @@ export default class ChildProcessWorker private _stdout: ReturnType | null; private _stderr: ReturnType | null; - private _stdoutBuffer: Array = []; private _stderrBuffer: Array = []; private _memoryUsagePromise: Promise | undefined; @@ -122,7 +121,6 @@ export default class ChildProcessWorker console.warn('Unable to detect out of memory event if silent === false'); } - this._stdoutBuffer = []; this._stderrBuffer = []; const options: ForkOptions = { @@ -150,12 +148,6 @@ export default class ChildProcessWorker } this._stdout.add(this._child.stdout); - - this._child.stdout.on('data', chunk => { - if (chunk) { - this._stdoutBuffer.push(Buffer.from(chunk)); - } - }); } if (this._child.stderr) { @@ -216,28 +208,58 @@ export default class ChildProcessWorker exitCode?: number | null, signal?: NodeJS.Signals | null, ): void { - for (const buffer of [this._stderrBuffer, this._stdoutBuffer]) { - try { - const bufferStr = Buffer.concat(buffer).toString('utf8'); + try { + const bufferStr = Buffer.concat(this._stderrBuffer).toString('utf8'); - if (bufferStr.includes('heap out of memory')) { - if ( - this.state === WorkerStates.OK || - this.state === WorkerStates.STARTING - ) { - this.state = WorkerStates.OUT_OF_MEMORY; - } + if ( + bufferStr.includes('heap out of memory') || + bufferStr.includes('allocation failure;') || + bufferStr.includes('Last few GCs') + ) { + if ( + this.state === WorkerStates.OK || + this.state === WorkerStates.STARTING + ) { + this.state = WorkerStates.OUT_OF_MEMORY; } - } catch (err) { - console.error('Error looking for out of memory crash', err); } + } catch (err) { + console.error('Error looking for out of memory crash', err); } // This is to try and work around a weird edge case when for some reason Windows doesn't feel - // like sending stderr, so we end up with no data to look at. + // like sending all of stderr, or even any of it in some cases, so we end up with not much + // to go on. + // Just to add to the confusion out of memory on windows can occur with either + // -> exitCode 134 | signal null + // OR (for some unknown reason) + // -> exitCode null | signal 'SIGTERM' + // Here are some samples + + // exitCode: 134, + // signal: null, + // stderr: '\r\n' + + // '<--- Last few GCs --->\r\n' + + // '\r\n' + + // '[6996:000001FCB84D40B0] 263 ms: Mark-sweep 60.1 (9', + // platform: 'win32' + + // exitCode: null, + // signal: 'SIGTERM', + // stderr: '2.7) -> 37.7 (70.3) MB, 33.7 / 0.0 ms (average mu = 0.539, current mu = 0.562) allocation failure; scavenge might not succeed\r\n' + + // '[6996:000001FCB84D40B0] 357 ms: Mark-sweep 88.1 (120.7) -> 54.4 (87.1) MB, 32.3 / 0.0 ms (average mu = 0.608, current mu = 0.656) allocation failure; scavenge might not succeed\r\n' + + // '\r\n' + + // '\r\n' + + // '<--- JS stacktrace --->\r\n' + + // '\r\n', + // platform: 'win32' + if (process.platform === 'win32') { - if (!this._stderrBuffer.length && !this._stdoutBuffer.length) { - if (exitCode === SIGABRT_EXIT_CODE && signal === null) { + if (!this._stderrBuffer.length) { + if ( + (exitCode === SIGABRT_EXIT_CODE && signal === null) || + (exitCode === null && signal === 'SIGTERM') + ) { if ( this.state === WorkerStates.OK || this.state === WorkerStates.STARTING @@ -346,7 +368,6 @@ export default class ChildProcessWorker exitCode, signal, stderr: Buffer.concat(this._stderrBuffer).toString('utf8'), - stdout: Buffer.concat(this._stdoutBuffer).toString('utf8'), childExitCode: this._child.exitCode, childConnected: this._child.connected, childKilled: this._child.killed, From 202323b6497330bd365b9603a849c6a807d4f08f Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 21:34:11 +0100 Subject: [PATCH 39/52] chore: try a different event --- .../jest-worker/src/workers/ChildProcessWorker.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 7b9877dc2b81..37e2ad59d852 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -168,6 +168,15 @@ export default class ChildProcessWorker this._child.on('message', this._onMessage.bind(this)); this._child.on('exit', this._onExit.bind(this)); + this._child.on('disconnect', () => { + console.log('DISCONNECT'); + }); + this._child.on('error', err => { + console.log('ERROR', err); + }); + this._child.on('close', (code, signal) => { + console.log('CLOSE', code, signal); + }); this._child.send([ CHILD_MESSAGE_INITIALIZE, @@ -275,6 +284,8 @@ export default class ChildProcessWorker // TODO: Add appropriate type check let error: any; + console.log('MSG'); + switch (response[0]) { case PARENT_MESSAGE_OK: this._onProcessEnd(null, response[1]); From 9e7d0819c0b43d8f252581020f1b326602578b71 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 21:56:47 +0100 Subject: [PATCH 40/52] chore: disconnect handling --- .github/workflows/test.yml | 2 +- .../src/workers/ChildProcessWorker.ts | 37 +++++++++++++------ .../jest-worker/src/workers/WorkerAbstract.ts | 2 + 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1bfb6a20ecee..3e120465c155 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -57,4 +57,4 @@ jobs: id: cpu-cores uses: SimenB/github-actions-cpu-cores@v1 - name: run tests using jest-jasmine - run: yarn jest-jasmine-ci --max-workers ${{ steps.cpu-cores.outputs.count }} --shard=${{ matrix.shard }} + run: yarn jest-jasmine-ci --max-workers ${{ steps.cpu-cores.outputs.count }} WorkerEdgeCases diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 37e2ad59d852..ca92cf99d7fe 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -168,15 +168,7 @@ export default class ChildProcessWorker this._child.on('message', this._onMessage.bind(this)); this._child.on('exit', this._onExit.bind(this)); - this._child.on('disconnect', () => { - console.log('DISCONNECT'); - }); - this._child.on('error', err => { - console.log('ERROR', err); - }); - this._child.on('close', (code, signal) => { - console.log('CLOSE', code, signal); - }); + this._child.on('disconnect', this._onDisconnect.bind(this)); this._child.send([ CHILD_MESSAGE_INITIALIZE, @@ -280,6 +272,29 @@ export default class ChildProcessWorker } } + private _onDisconnect() { + console.log('_onDisconnect PRE', { + stderr: Buffer.concat(this._stderrBuffer).toString('utf8'), + childExitCode: this._child.exitCode, + childConnected: this._child.connected, + childKilled: this._child.killed, + state: this.state, + platform: process.platform, + }); + + this._detectOutOfMemoryCrash(); + + if (this.state === WorkerStates.OUT_OF_MEMORY) { + this.killChild(); + this._shutdown(); + } + + console.log('_onDisconnect POST', { + state: this.state, + platform: process.platform, + }); + } + private _onMessage(response: ParentMessage) { // TODO: Add appropriate type check let error: any; @@ -375,7 +390,7 @@ export default class ChildProcessWorker this._workerReadyPromise = undefined; this._resolveWorkerReady = undefined; - console.log('PRE', { + console.log('_onExit PRE', { exitCode, signal, stderr: Buffer.concat(this._stderrBuffer).toString('utf8'), @@ -388,7 +403,7 @@ export default class ChildProcessWorker this._detectOutOfMemoryCrash(exitCode, signal); - console.log('POST', { + console.log('_onExit POST', { exitCode, signal, state: this.state, diff --git a/packages/jest-worker/src/workers/WorkerAbstract.ts b/packages/jest-worker/src/workers/WorkerAbstract.ts index 0cf97a3ee66f..202e62509844 100644 --- a/packages/jest-worker/src/workers/WorkerAbstract.ts +++ b/packages/jest-worker/src/workers/WorkerAbstract.ts @@ -119,6 +119,8 @@ export default abstract class WorkerAbstract * killed off. */ protected _shutdown(): void { + this.state === WorkerStates.SHUT_DOWN; + // End the permanent stream so the merged stream end too if (this._fakeStream) { this._fakeStream.end(); From 75037d278c1f89863e93854c3df6428d4616bb80 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 22:02:16 +0100 Subject: [PATCH 41/52] fix: test bug --- packages/jest-worker/src/workers/ChildProcessWorker.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index ca92cf99d7fe..b4b038a3b5a1 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -273,6 +273,9 @@ export default class ChildProcessWorker } private _onDisconnect() { + this._workerReadyPromise = undefined; + this._resolveWorkerReady = undefined; + console.log('_onDisconnect PRE', { stderr: Buffer.concat(this._stderrBuffer).toString('utf8'), childExitCode: this._child.exitCode, @@ -443,7 +446,6 @@ export default class ChildProcessWorker onProcessEnd: OnEnd, onCustomMessage: OnCustomMessage, ): void { - this._stdoutBuffer = []; this._stderrBuffer = []; onProcessStart(this); From 37c5a4394dfd32dc3e175cfb25ba22470cdfda84 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 22:17:58 +0100 Subject: [PATCH 42/52] chore: log child and buffer --- packages/jest-worker/src/workers/ChildProcessWorker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index b4b038a3b5a1..9d39bb4ecf5d 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -58,7 +58,7 @@ export default class ChildProcessWorker extends WorkerAbstract implements WorkerInterface { - private _child!: ChildProcess; + public _child!: ChildProcess; private _options: WorkerOptions; private _request: ChildMessage | null; From 3e8b3ae831c3028d6ddb7f269e7f8fafd6d34c0a Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 22:18:48 +0100 Subject: [PATCH 43/52] chore: forgot to make public --- packages/jest-worker/src/workers/ChildProcessWorker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 9d39bb4ecf5d..c644f108b52f 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -69,7 +69,7 @@ export default class ChildProcessWorker private _stdout: ReturnType | null; private _stderr: ReturnType | null; - private _stderrBuffer: Array = []; + public _stderrBuffer: Array = []; private _memoryUsagePromise: Promise | undefined; private _resolveMemoryUsage: ((arg0: number) => void) | undefined; From fab410ae0220d9ae7ad01f77e51505ecbc44e51a Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 22:20:45 +0100 Subject: [PATCH 44/52] chore: log --- .../src/workers/__tests__/WorkerEdgeCases.test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 8c0b7e9822ea..a652a7c5d037 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -314,6 +314,13 @@ describe.each([ }); test('worker stays dead', async () => { + if (worker instanceof ChildProcessWorker) { + console.log({ + child: worker._child, + buff: Buffer.concat(worker._stderrBuffer).toString('utf8'), + }); + } + await expect( async () => await worker.waitForWorkerReady(), ).rejects.toThrowError(); From 8e4a31ea3e58d3388dae0d26ae43c2ad6e86c41d Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 22:29:22 +0100 Subject: [PATCH 45/52] chore: use up memory quicker and more logging --- packages/jest-worker/src/workers/ChildProcessWorker.ts | 6 ++++++ .../src/workers/__tests__/__fixtures__/EdgeCasesWorker.js | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index c644f108b52f..d94a6c6be162 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -169,6 +169,12 @@ export default class ChildProcessWorker this._child.on('message', this._onMessage.bind(this)); this._child.on('exit', this._onExit.bind(this)); this._child.on('disconnect', this._onDisconnect.bind(this)); + this._child.on('close', (code, signal) => { + console.log('CLOSE', code, signal); + }); + this._child.on('error', err => { + console.log('ERROR', err); + }); this._child.send([ CHILD_MESSAGE_INITIALIZE, diff --git a/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js b/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js index 50ab3bdc3c91..83b02f58d2d1 100644 --- a/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js +++ b/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js @@ -19,7 +19,7 @@ async function leakMemory() { ).toFixed(2)}MB at start`, ); - let i = 0; + let i = Number.MAX_SAFE_INTEGER / 2; while (true) { i++; From b974eeaac3ed5d139abaec1861353340cb78939a Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 22:43:53 +0100 Subject: [PATCH 46/52] chore: try removing force exit to see if that helps with the confusing noise --- .../src/workers/__tests__/WorkerEdgeCases.test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index a652a7c5d037..2757841f2e3d 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -279,12 +279,12 @@ describe.each([ worker = new workerClass(options); }); - afterAll(async () => { - if (worker) { - worker.forceExit(); - await worker.waitForExit(); - } - }); + // afterAll(async () => { + // if (worker) { + // worker.forceExit(); + // await worker.waitForExit(); + // } + // }); test('starting state', async () => { const startPid = worker.getWorkerSystemId(); From 1e1023d45fdb650f2809b3d66dd3aa5d6c98e902 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 23:15:37 +0100 Subject: [PATCH 47/52] chore: different strat --- .../src/workers/ChildProcessWorker.ts | 72 +++++-------------- .../workers/__tests__/WorkerEdgeCases.test.js | 19 +++-- 2 files changed, 32 insertions(+), 59 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index d94a6c6be162..2b236ee4142c 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -159,11 +159,7 @@ export default class ChildProcessWorker this._stderr.add(this._child.stderr); - this._child.stderr.on('data', chunk => { - if (chunk) { - this._stderrBuffer.push(Buffer.from(chunk)); - } - }); + this._child.stderr.on('data', this.stderrDataHandler.bind(this)); } this._child.on('message', this._onMessage.bind(this)); @@ -211,10 +207,23 @@ export default class ChildProcessWorker } } - private _detectOutOfMemoryCrash( - exitCode?: number | null, - signal?: NodeJS.Signals | null, - ): void { + private stderrDataHandler(chunk: any): void { + if (chunk) { + this._stderrBuffer.push(Buffer.from(chunk)); + } + + this._detectOutOfMemoryCrash(); + + if (this.state === WorkerStates.OUT_OF_MEMORY) { + this._workerReadyPromise = undefined; + this._resolveWorkerReady = undefined; + + this.killChild(); + this._shutdown(); + } + } + + private _detectOutOfMemoryCrash(): void { try { const bufferStr = Buffer.concat(this._stderrBuffer).toString('utf8'); @@ -233,49 +242,6 @@ export default class ChildProcessWorker } catch (err) { console.error('Error looking for out of memory crash', err); } - - // This is to try and work around a weird edge case when for some reason Windows doesn't feel - // like sending all of stderr, or even any of it in some cases, so we end up with not much - // to go on. - // Just to add to the confusion out of memory on windows can occur with either - // -> exitCode 134 | signal null - // OR (for some unknown reason) - // -> exitCode null | signal 'SIGTERM' - // Here are some samples - - // exitCode: 134, - // signal: null, - // stderr: '\r\n' + - // '<--- Last few GCs --->\r\n' + - // '\r\n' + - // '[6996:000001FCB84D40B0] 263 ms: Mark-sweep 60.1 (9', - // platform: 'win32' - - // exitCode: null, - // signal: 'SIGTERM', - // stderr: '2.7) -> 37.7 (70.3) MB, 33.7 / 0.0 ms (average mu = 0.539, current mu = 0.562) allocation failure; scavenge might not succeed\r\n' + - // '[6996:000001FCB84D40B0] 357 ms: Mark-sweep 88.1 (120.7) -> 54.4 (87.1) MB, 32.3 / 0.0 ms (average mu = 0.608, current mu = 0.656) allocation failure; scavenge might not succeed\r\n' + - // '\r\n' + - // '\r\n' + - // '<--- JS stacktrace --->\r\n' + - // '\r\n', - // platform: 'win32' - - if (process.platform === 'win32') { - if (!this._stderrBuffer.length) { - if ( - (exitCode === SIGABRT_EXIT_CODE && signal === null) || - (exitCode === null && signal === 'SIGTERM') - ) { - if ( - this.state === WorkerStates.OK || - this.state === WorkerStates.STARTING - ) { - this.state = WorkerStates.OUT_OF_MEMORY; - } - } - } - } } private _onDisconnect() { @@ -410,7 +376,7 @@ export default class ChildProcessWorker platform: process.platform, }); - this._detectOutOfMemoryCrash(exitCode, signal); + this._detectOutOfMemoryCrash(); console.log('_onExit POST', { exitCode, diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 2757841f2e3d..1dc44b5e2275 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -279,12 +279,18 @@ describe.each([ worker = new workerClass(options); }); - // afterAll(async () => { - // if (worker) { - // worker.forceExit(); - // await worker.waitForExit(); - // } - // }); + afterAll(async () => { + await new Promise(resolve => { + setTimeout(async () => { + if (worker) { + worker.forceExit(); + await worker.waitForExit(); + } + + resolve(); + }, 500); + }); + }); test('starting state', async () => { const startPid = worker.getWorkerSystemId(); @@ -318,6 +324,7 @@ describe.each([ console.log({ child: worker._child, buff: Buffer.concat(worker._stderrBuffer).toString('utf8'), + state: worker.state, }); } From ab2931d2b75f1d71de86dc6c719d9b9ba5a87b01 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Wed, 10 Aug 2022 23:26:04 +0100 Subject: [PATCH 48/52] chore: remove debugging --- .github/workflows/test.yml | 4 +- .../src/workers/ChildProcessWorker.ts | 47 ++----------------- .../workers/__tests__/WorkerEdgeCases.test.js | 10 +--- 3 files changed, 6 insertions(+), 55 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3e120465c155..d7060b18db3a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,7 @@ jobs: id: cpu-cores uses: SimenB/github-actions-cpu-cores@v1 - name: run tests - run: yarn test-ci-partial:parallel --max-workers ${{ steps.cpu-cores.outputs.count }} WorkerEdgeCases + run: yarn test-ci-partial:parallel --max-workers ${{ steps.cpu-cores.outputs.count }} --shard=${{ matrix.shard }} test-jasmine: strategy: @@ -57,4 +57,4 @@ jobs: id: cpu-cores uses: SimenB/github-actions-cpu-cores@v1 - name: run tests using jest-jasmine - run: yarn jest-jasmine-ci --max-workers ${{ steps.cpu-cores.outputs.count }} WorkerEdgeCases + run: yarn jest-jasmine-ci --max-workers ${{ steps.cpu-cores.outputs.count }} --shard=${{ matrix.shard }} diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 2b236ee4142c..8fb9676d4879 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -29,7 +29,6 @@ import { import WorkerAbstract from './WorkerAbstract'; const SIGNAL_BASE_EXIT_CODE = 128; -const SIGABRT_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 6; const SIGKILL_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 9; const SIGTERM_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 15; @@ -58,7 +57,7 @@ export default class ChildProcessWorker extends WorkerAbstract implements WorkerInterface { - public _child!: ChildProcess; + private _child!: ChildProcess; private _options: WorkerOptions; private _request: ChildMessage | null; @@ -69,7 +68,7 @@ export default class ChildProcessWorker private _stdout: ReturnType | null; private _stderr: ReturnType | null; - public _stderrBuffer: Array = []; + private _stderrBuffer: Array = []; private _memoryUsagePromise: Promise | undefined; private _resolveMemoryUsage: ((arg0: number) => void) | undefined; @@ -165,12 +164,6 @@ export default class ChildProcessWorker this._child.on('message', this._onMessage.bind(this)); this._child.on('exit', this._onExit.bind(this)); this._child.on('disconnect', this._onDisconnect.bind(this)); - this._child.on('close', (code, signal) => { - console.log('CLOSE', code, signal); - }); - this._child.on('error', err => { - console.log('ERROR', err); - }); this._child.send([ CHILD_MESSAGE_INITIALIZE, @@ -248,34 +241,18 @@ export default class ChildProcessWorker this._workerReadyPromise = undefined; this._resolveWorkerReady = undefined; - console.log('_onDisconnect PRE', { - stderr: Buffer.concat(this._stderrBuffer).toString('utf8'), - childExitCode: this._child.exitCode, - childConnected: this._child.connected, - childKilled: this._child.killed, - state: this.state, - platform: process.platform, - }); - this._detectOutOfMemoryCrash(); if (this.state === WorkerStates.OUT_OF_MEMORY) { this.killChild(); this._shutdown(); } - - console.log('_onDisconnect POST', { - state: this.state, - platform: process.platform, - }); } private _onMessage(response: ParentMessage) { // TODO: Add appropriate type check let error: any; - console.log('MSG'); - switch (response[0]) { case PARENT_MESSAGE_OK: this._onProcessEnd(null, response[1]); @@ -361,30 +338,12 @@ export default class ChildProcessWorker } } - private _onExit(exitCode: number | null, signal: NodeJS.Signals | null) { + private _onExit(exitCode: number | null) { this._workerReadyPromise = undefined; this._resolveWorkerReady = undefined; - console.log('_onExit PRE', { - exitCode, - signal, - stderr: Buffer.concat(this._stderrBuffer).toString('utf8'), - childExitCode: this._child.exitCode, - childConnected: this._child.connected, - childKilled: this._child.killed, - state: this.state, - platform: process.platform, - }); - this._detectOutOfMemoryCrash(); - console.log('_onExit POST', { - exitCode, - signal, - state: this.state, - platform: process.platform, - }); - if (exitCode !== 0 && this.state === WorkerStates.OUT_OF_MEMORY) { this._onProcessEnd( new Error('Jest worker ran out of memory and crashed'), diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 1dc44b5e2275..68211a60a481 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -240,7 +240,7 @@ describe.each([ }); }); - describe.only('should cleanly exit on out of memory crash', () => { + describe('should cleanly exit on out of memory crash', () => { const workerHeapLimit = 50; let worker; @@ -320,14 +320,6 @@ describe.each([ }); test('worker stays dead', async () => { - if (worker instanceof ChildProcessWorker) { - console.log({ - child: worker._child, - buff: Buffer.concat(worker._stderrBuffer).toString('utf8'), - state: worker.state, - }); - } - await expect( async () => await worker.waitForWorkerReady(), ).rejects.toThrowError(); From c610bdb356c307fbf3cc5ecabbd51a5023b1f37f Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Thu, 11 Aug 2022 08:07:07 +0100 Subject: [PATCH 49/52] chore: revert to silent worker --- jest.config.ci.mjs | 9 ++++----- .../workers/__tests__/__fixtures__/EdgeCasesWorker.js | 4 ---- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/jest.config.ci.mjs b/jest.config.ci.mjs index fbf7ab6f1590..2b2f36983f5e 100644 --- a/jest.config.ci.mjs +++ b/jest.config.ci.mjs @@ -12,16 +12,15 @@ export default { ...baseConfig, coverageReporters: ['json'], reporters: [ - 'default', 'github-actions', [ 'jest-junit', {outputDirectory: 'reports/junit', outputName: 'js-test-results.xml'}, ], - // [ - // 'jest-silent-reporter', - // {showPaths: true, showWarnings: true, useDots: true}, - // ], + [ + 'jest-silent-reporter', + {showPaths: true, showWarnings: true, useDots: true}, + ], 'summary', ], }; diff --git a/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js b/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js index 83b02f58d2d1..02ca3754ae26 100644 --- a/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js +++ b/packages/jest-worker/src/workers/__tests__/__fixtures__/EdgeCasesWorker.js @@ -28,10 +28,6 @@ async function leakMemory() { } function fatalExitCode() { - process.exit(139); -} - -function weirdWindowsOutOfMemoryExit() { process.exit(134); } From ee20013c0ce2aaf888b3f444ad293fbb13969fe2 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Thu, 11 Aug 2022 10:36:18 +0200 Subject: [PATCH 50/52] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc3a69b6e7cb..f5ea35996abb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ - `[jest-config]` [**BREAKING**] Make `snapshotFormat` default to `escapeString: false` and `printBasicPrototype: false` ([#13036](https://github.com/facebook/jest/pull/13036)) - `[jest-environment-jsdom]` [**BREAKING**] Upgrade to `jsdom@20` ([#13037](https://github.com/facebook/jest/pull/13037), [#13058](https://github.com/facebook/jest/pull/13058)) -- `[jest-worker]` Adds `workerIdleMemoryLimit` option which is used as a check for worker memory leaks >= Node 16.11.0 and recycles child workers as required. ([#13056](https://github.com/facebook/jest/pull/13056), [#13105](https://github.com/facebook/jest/pull/13105), [#13106](https://github.com/facebook/jest/pull/13106)) +- `[jest-worker]` Adds `workerIdleMemoryLimit` option which is used as a check for worker memory leaks >= Node 16.11.0 and recycles child workers as required. ([#13056](https://github.com/facebook/jest/pull/13056), [#13105](https://github.com/facebook/jest/pull/13105), [#13106](https://github.com/facebook/jest/pull/13106), [#13107](https://github.com/facebook/jest/pull/13107)) - `[pretty-format]` [**BREAKING**] Remove `ConvertAnsi` plugin in favour of `jest-serializer-ansi-escapes` ([#13040](https://github.com/facebook/jest/pull/13040)) ### Fixes From bd786b1ffef09f85a4226be444b3d5140d0db480 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Thu, 11 Aug 2022 10:40:41 +0100 Subject: [PATCH 51/52] chore: pr feedback --- packages/jest-worker/src/types.ts | 6 +- .../src/workers/ChildProcessWorker.ts | 5 ++ .../jest-worker/src/workers/WorkerAbstract.ts | 22 ++++-- .../workers/__tests__/WorkerEdgeCases.test.js | 74 +++++++++---------- 4 files changed, 57 insertions(+), 50 deletions(-) diff --git a/packages/jest-worker/src/types.ts b/packages/jest-worker/src/types.ts index 31acac62d0e5..a301c6b994a0 100644 --- a/packages/jest-worker/src/types.ts +++ b/packages/jest-worker/src/types.ts @@ -190,12 +190,12 @@ export type WorkerOptions = { */ on?: { [WorkerEvents.STATE_CHANGE]: - | onStateChangeHandler - | Array; + | OnStateChangeHandler + | ReadonlyArray; }; }; -export type onStateChangeHandler = ( +export type OnStateChangeHandler = ( state: WorkerStates, oldState: WorkerStates, ) => void; diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 8fb9676d4879..9706a0235f56 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -117,6 +117,11 @@ export default class ChildProcessWorker const silent = this._options.silent ?? true; if (!silent) { + // NOTE: Detecting an out of memory crash is independent of idle memory usage monitoring. We want to + // monitor for a crash occurring so that it can be handled as required and so we can tell the difference + // between an OOM crash and another kind of crash. We need to do this because if a worker crashes due to + // an OOM event sometimes it isn't seen by the worker pool and it just sits there waiting for the worker + // to respond and it never will. console.warn('Unable to detect out of memory event if silent === false'); } diff --git a/packages/jest-worker/src/workers/WorkerAbstract.ts b/packages/jest-worker/src/workers/WorkerAbstract.ts index 202e62509844..957660990b4f 100644 --- a/packages/jest-worker/src/workers/WorkerAbstract.ts +++ b/packages/jest-worker/src/workers/WorkerAbstract.ts @@ -17,7 +17,11 @@ export default abstract class WorkerAbstract extends EventEmitter implements Pick { - private __state = WorkerStates.STARTING; + /** + * DO NOT WRITE TO THIS DIRECTLY. + * Use this.state getter/setters so events are emitted correctly. + */ + #state = WorkerStates.STARTING; protected _fakeStream: PassThrough | null = null; @@ -28,12 +32,12 @@ export default abstract class WorkerAbstract protected _resolveWorkerReady: (() => void) | undefined; public get state(): WorkerStates { - return this.__state; + return this.#state; } protected set state(value: WorkerStates) { - if (this.__state !== value) { - const oldState = this.__state; - this.__state = value; + if (this.#state !== value) { + const oldState = this.#state; + this.#state = value; this.emit(WorkerEvents.STATE_CHANGE, value, oldState); } @@ -44,12 +48,14 @@ export default abstract class WorkerAbstract if (typeof options.on === 'object') { for (const [event, handlers] of Object.entries(options.on)) { - if (Array.isArray(handlers)) { + // Can't do Array.isArray on a ReadonlyArray. + // https://github.com/microsoft/TypeScript/issues/17002 + if (typeof handlers === 'function') { + super.on(event, handlers); + } else { for (const handler of handlers) { super.on(event, handler); } - } else { - super.on(event, handlers); } } } diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 68211a60a481..31b5dc3fa35b 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -19,7 +19,6 @@ import { import ChildProcessWorker, {SIGKILL_DELAY} from '../ChildProcessWorker'; import ThreadsWorker from '../NodeThreadsWorker'; -jest.retryTimes(0); jest.setTimeout(10000); const root = join('../../'); @@ -58,6 +57,15 @@ test.each(filesToBuild)('%s.js should exist', async file => { await expect(async () => await access(path)).not.toThrowError(); }); +async function closeWorkerAfter(worker, testBody) { + try { + await testBody(worker); + } finally { + worker.forceExit(); + await worker.waitForExit(); + } +} + describe.each([ { name: 'ProcessWorker', @@ -101,62 +109,50 @@ describe.each([ } test('should get memory usage', async () => { - let worker; - - try { - worker = new workerClass({ + await closeWorkerAfter( + new workerClass({ childWorkerPath: workerPath, maxRetries: 0, workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), - }); + }), + async worker => { + const memoryUsagePromise = worker.getMemoryUsage(); + expect(memoryUsagePromise).toBeInstanceOf(Promise); - const memoryUsagePromise = worker.getMemoryUsage(); - expect(memoryUsagePromise).toBeInstanceOf(Promise); - - expect(await memoryUsagePromise).toBeGreaterThan(0); - } finally { - if (worker) { - worker.forceExit(); - await worker.waitForExit(); - } - } + expect(await memoryUsagePromise).toBeGreaterThan(0); + }, + ); }); test('should recycle on idle limit breach', async () => { - let worker; - - try { - worker = new workerClass({ + await closeWorkerAfter( + new workerClass({ childWorkerPath: workerPath, // There is no way this is fitting into 1000 bytes, so it should restart // after requesting a memory usage update idleMemoryLimit: 1000, maxRetries: 0, workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'), - }); - - const startSystemId = worker.getWorkerSystemId(); - expect(startSystemId).toBeGreaterThanOrEqual(0); + }), + async worker => { + const startSystemId = worker.getWorkerSystemId(); + expect(startSystemId).toBeGreaterThanOrEqual(0); - worker.checkMemoryUsage(); + worker.checkMemoryUsage(); - await waitForChange(() => worker.getWorkerSystemId()); + await waitForChange(() => worker.getWorkerSystemId()); - const systemId = worker.getWorkerSystemId(); - expect(systemId).toBeGreaterThanOrEqual(0); - expect(systemId).not.toEqual(startSystemId); + const systemId = worker.getWorkerSystemId(); + expect(systemId).toBeGreaterThanOrEqual(0); + expect(systemId).not.toEqual(startSystemId); - await new Promise(resolve => { - setTimeout(resolve, SIGKILL_DELAY + 100); - }); + await new Promise(resolve => { + setTimeout(resolve, SIGKILL_DELAY + 100); + }); - expect(worker.isWorkerRunning()).toBeTruthy(); - } finally { - if (worker) { - worker.forceExit(); - await worker.waitForExit(); - } - } + expect(worker.isWorkerRunning()).toBeTruthy(); + }, + ); }); describe('should automatically recycle on idle limit breach', () => { From 613fcf2d2118ee6171defc76c2e9696b5323345d Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Thu, 11 Aug 2022 13:15:13 +0100 Subject: [PATCH 52/52] chore: pr changes --- .../src/workers/__tests__/WorkerEdgeCases.test.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js index 31b5dc3fa35b..0b68ea8755a6 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js @@ -78,7 +78,6 @@ describe.each([ workerPath: threadChildWorkerPath, }, ])('$name', ({workerClass, workerPath}) => { - /** @type WorkerInterface */ let int; afterEach(async () => { @@ -386,9 +385,7 @@ describe.each([ test('processes exits', async () => { worker.forceExit(); - await expect( - async () => await worker.waitForWorkerReady(), - ).rejects.toThrowError(); + await expect(() => worker.waitForWorkerReady()).rejects.toThrowError(); }); }); });