Skip to content

Commit

Permalink
fix(jest-worker): fix hanging when child process workers are killed
Browse files Browse the repository at this point in the history
  • Loading branch information
gluxon committed Nov 6, 2022
1 parent 7c48c4c commit a0703c9
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Fixes

- `[jest-mock]` Treat cjs modules as objects so they can be mocked ([#13513](https://github.com/facebook/jest/pull/13513))
- `[jest-worker]` Throw an error instead of hanging when jest workers are killed ([#13566](https://github.com/facebook/jest/pull/13566))

### Chore & Maintenance

Expand Down
42 changes: 41 additions & 1 deletion packages/jest-worker/src/workers/ChildProcessWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ export default class ChildProcessWorker
}
}

private _onExit(exitCode: number | null) {
private _onExit(exitCode: number | null, signal: NodeJS.Signals | null) {
this._workerReadyPromise = undefined;
this._resolveWorkerReady = undefined;

Expand Down Expand Up @@ -372,6 +372,46 @@ export default class ChildProcessWorker
this._child.send(this._request);
}
} else {
// At this point, it's not clear why the child process exited. There could
// be several reasons:
//
// 1. The child process exited successfully after finishing its work.
// This is the most likely case.
// 2. The child process crashed in a manner that wasn't caught through
// any of the heuristic-based checks above.
// 3. The child process was killed by another process or daemon unrelated
// to Jest. For example, oom-killer on Linux may have picked the child
// process to kill because overall system memory is constrained.
//
// If there's a pending request to the child process in any of those
// situations, the request still needs to be handled in some manner before
// entering the shutdown phase. Otherwise the caller expecting a response
// from the worker will never receive indication that something unexpected
// happened and hang forever.
//
// In normal operation, the request is handled and cleared before the
// child process exits. If it's still present, it's not clear what
// happened and probably best to throw an error. In practice, this usually
// happens when the child process is killed externally.
//
// There's a reasonable argument that the child process should be retried
// with request re-sent in this scenario. However, if the problem was due
// to situations such as oom-killer attempting to free up system
// resources, retrying would exacerbate the problem.
const isRequestStillPending = !!this._request;
if (isRequestStillPending) {
// If a signal is present, we can be reasonably confident the process
// was killed externally. Log this fact so it's more clear to users that
// something went wrong externally, rather than a bug in Jest itself.
const error = new Error(
signal != null
? `A jest worker process (pid=${this._child.pid}) was terminated by another process: signal=${signal}, exitCode=${exitCode}. Operating system logs may contain more information on why this occurred.`
: `A jest worker process (pid=${this._child.pid}) crashed for an unknown reason: exitCode=${exitCode}`,
);

this._onProcessEnd(error, null);
}

this._shutdown();
}
}
Expand Down
65 changes: 65 additions & 0 deletions packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,3 +386,68 @@ describe.each([
});
});
});

// This describe block only applies to the child process worker since it's
// generally not possible for external processes to abruptly kill a thread of
// another process.
describe('should not hang on external process kill', () => {
let worker: ChildProcessWorker;

beforeEach(() => {
const options = {
childWorkerPath: processChildWorkerPath,
maxRetries: 0,
silent: true,
workerPath: join(__dirname, '__fixtures__', 'SelfKillWorker'),
} as unknown as WorkerOptions;

worker = new ChildProcessWorker(options);
});

afterEach(async () => {
await new Promise<void>(resolve => {
setTimeout(async () => {
if (worker) {
worker.forceExit();
await worker.waitForExit();
}

resolve();
}, 500);
});
});

// Regression test for https://github.com/facebook/jest/issues/13183
test('onEnd callback is called', async () => {
let onEndPromiseResolve: () => void;
let onEndPromiseReject: (err: Error) => void;
const onEndPromise = new Promise<void>((resolve, reject) => {
onEndPromiseResolve = resolve;
onEndPromiseReject = reject;
});

const onStart = jest.fn();
const onEnd = jest.fn((err: Error | null) => {
if (err) {
return onEndPromiseReject(err);
}
onEndPromiseResolve();
});
const onCustom = jest.fn();

await worker.waitForWorkerReady();

// The SelfKillWorker simulates an external process calling SIGTERM on it,
// but just SIGTERMs itself underneath the hood to make this test easier.
worker.send(
[CHILD_MESSAGE_CALL, true, 'selfKill', []],
onStart,
onEnd,
onCustom,
);

// The onEnd callback should be called when the child process exits.
await expect(onEndPromise).rejects.toBeInstanceOf(Error);
expect(onEnd).toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* 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.
*/
const {isMainThread} = require('worker_threads');

async function selfKill() {
// This test is intended for the child process worker. If the Node.js worker
// thread mode is accidentally tested instead, let's prevent a confusing
// situation where process.kill stops the Jest test harness itself.
if (!isMainThread) {
// process.exit is documented to only stop the current thread rather than
// the process in a worker_threads environment.
process.exit();
}

process.kill(process.pid);
}

module.exports = {
selfKill,
};

0 comments on commit a0703c9

Please sign in to comment.