Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(jest-worker): Remove circular references from messages sent to workers #10881

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- `[jest-transform]` Show enhanced `SyntaxError` message for all `SyntaxError`s ([#10749](https://github.com/facebook/jest/pull/10749))
- `[jest-transform]` [**BREAKING**] Refactor API to pass an options bag around rather than multiple boolean options ([#10753](https://github.com/facebook/jest/pull/10753))
- `[jest-transform]` [**BREAKING**] Refactor API of transformers to pass an options bag rather than separate `config` and other options
- `[jest-worker]` Remove circular references from messages sent to workers to prevent error and jest hanging ([#10881](https://github.com/facebook/jest/pull/10881))
- `[pretty-format]` [**BREAKING**] Convert to ES Modules ([#10515](https://github.com/facebook/jest/pull/10515))

### Chore & Maintenance
Expand Down
1 change: 1 addition & 0 deletions packages/jest-worker/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
},
"dependencies": {
"@types/node": "*",
"fclone": "^1.0.11",
"merge-stream": "^2.0.0",
"supports-color": "^8.0.0"
},
Expand Down
80 changes: 80 additions & 0 deletions packages/jest-worker/src/workers/__tests__/messageParent.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/**
* 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.
*/

'use strict';

import {PARENT_MESSAGE_CUSTOM} from '../../types';

const processSend = process.send;

let messageParent;
let mockWorkerThreads;

beforeEach(() => {
mockWorkerThreads = {};
process.send = jest.fn();
jest.mock('worker_threads', () => mockWorkerThreads);
messageParent = require('../messageParent').default;
});

afterEach(() => {
jest.resetModules();
// console.log(require('worker_threads'));
process.send = processSend;
});

describe('with worker threads', () => {
beforeEach(() => {
mockWorkerThreads.parentPort = {
postMessage: jest.fn(),
};
});

it('cand send a message', () => {
messageParent('some-message');
expect(mockWorkerThreads.parentPort.postMessage).toHaveBeenCalledWith([
PARENT_MESSAGE_CUSTOM,
'some-message',
]);
});

it('removes circular references from the message being sent', () => {
const circular = {ref: null, some: 'thing'};
circular.ref = circular;
messageParent(circular);
expect(mockWorkerThreads.parentPort.postMessage).toHaveBeenCalledWith([
PARENT_MESSAGE_CUSTOM,
{
ref: '[Circular]',
some: 'thing',
},
]);
});
});

describe('without worker threads', () => {
it('cand send a message', () => {
messageParent('some-message');
expect(process.send).toHaveBeenCalledWith([
PARENT_MESSAGE_CUSTOM,
'some-message',
]);
});

it('removes circular references from the message being sent', () => {
const circular = {ref: null, some: 'thing'};
circular.ref = circular;
messageParent(circular);
expect(process.send).toHaveBeenCalledWith([
PARENT_MESSAGE_CUSTOM,
{
ref: '[Circular]',
some: 'thing',
},
]);
});
});
30 changes: 30 additions & 0 deletions packages/jest-worker/src/workers/__tests__/processChild.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ beforeEach(() => {
});
},

fooReturnsCircular() {
const circular = {ref: null, some: 'thing'};
circular.ref = circular;
return circular;
},

fooThrows() {
throw mockError;
},
Expand Down Expand Up @@ -376,3 +382,27 @@ it('throws if child is not forked', () => {
]);
}).toThrow();
});

it('removes circular references from the message being sent', () => {
process.emit('message', [
CHILD_MESSAGE_INITIALIZE,
true, // Not really used here, but for flow type purity.
'./my-fancy-worker',
]);

process.emit('message', [
CHILD_MESSAGE_CALL,
true, // Not really used here, but for flow type purity.
'fooReturnsCircular',
[],
]);

expect(process.send).toHaveBeenCalled();
expect(process.send.mock.calls[0][0]).toEqual([
PARENT_MESSAGE_OK,
{
ref: '[Circular]',
some: 'thing',
},
]);
});
7 changes: 5 additions & 2 deletions packages/jest-worker/src/workers/messageParent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import fclone from 'fclone';
import {PARENT_MESSAGE_CUSTOM} from '../types';

const isWorkerThread = () => {
Expand All @@ -22,12 +23,14 @@ const messageParent = (
parentProcess: NodeJS.Process = process,
): void => {
try {
const nonCircularMessage = fclone(message);

if (isWorkerThread()) {
// `Require` here to support Node v10
const {parentPort} = require('worker_threads');
parentPort.postMessage([PARENT_MESSAGE_CUSTOM, message]);
parentPort.postMessage([PARENT_MESSAGE_CUSTOM, nonCircularMessage]);
} else if (typeof parentProcess.send === 'function') {
parentProcess.send([PARENT_MESSAGE_CUSTOM, message]);
parentProcess.send([PARENT_MESSAGE_CUSTOM, nonCircularMessage]);
}
} catch {
throw new Error('"messageParent" can only be used inside a worker');
Expand Down
3 changes: 2 additions & 1 deletion packages/jest-worker/src/workers/processChild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import fclone from 'fclone';
import {
CHILD_MESSAGE_CALL,
CHILD_MESSAGE_END,
Expand Down Expand Up @@ -64,7 +65,7 @@ function reportSuccess(result: unknown) {
throw new Error('Child can only be used on a forked process');
}

process.send([PARENT_MESSAGE_OK, result]);
process.send([PARENT_MESSAGE_OK, fclone(result)]);
}

function reportClientError(error: Error) {
Expand Down
8 changes: 8 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9069,6 +9069,13 @@ fbjs-scripts@^1.1.0:
languageName: node
linkType: hard

"fclone@npm:^1.0.11":
version: 1.0.11
resolution: "fclone@npm:1.0.11"
checksum: f4a2c63c141dd2796745acbea3858704a86aec1985a226259c2a574e4b44596197b4fe4bea8b45950d0fb577a63ceef21b5e075016ee52f6401c06f6a0396a2d
languageName: node
linkType: hard

"fd-slicer@npm:~1.1.0":
version: 1.1.0
resolution: "fd-slicer@npm:1.1.0"
Expand Down Expand Up @@ -12422,6 +12429,7 @@ fsevents@~2.1.2:
"@types/merge-stream": ^1.1.2
"@types/node": "*"
"@types/supports-color": ^7.2.0
fclone: ^1.0.11
get-stream: ^6.0.0
merge-stream: ^2.0.0
supports-color: ^8.0.0
Expand Down