From 75e522fb4068a3c8216de81bb28c212e61b29cb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franti=C5=A1ek=20=C5=BDia=C4=8Dik?= Date: Fri, 27 Nov 2020 21:46:50 +0100 Subject: [PATCH] fix(jest-worker): Remove circular references from messages sent to workers Fixes #10577 --- CHANGELOG.md | 1 + packages/jest-worker/package.json | 1 + .../workers/__tests__/messageParent.test.js | 68 +++++++++++++++++++ .../workers/__tests__/processChild.test.js | 27 ++++++++ .../jest-worker/src/workers/messageParent.ts | 7 +- .../jest-worker/src/workers/processChild.ts | 3 +- yarn.lock | 8 +++ 7 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 packages/jest-worker/src/workers/__tests__/messageParent.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cfecb820074..0fbd15037cd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/packages/jest-worker/package.json b/packages/jest-worker/package.json index 17450625a8e7..c4982e964423 100644 --- a/packages/jest-worker/package.json +++ b/packages/jest-worker/package.json @@ -15,6 +15,7 @@ }, "dependencies": { "@types/node": "*", + "fclone": "^1.0.11", "merge-stream": "^2.0.0", "supports-color": "^8.0.0" }, diff --git a/packages/jest-worker/src/workers/__tests__/messageParent.test.js b/packages/jest-worker/src/workers/__tests__/messageParent.test.js new file mode 100644 index 000000000000..c63acb50cba1 --- /dev/null +++ b/packages/jest-worker/src/workers/__tests__/messageParent.test.js @@ -0,0 +1,68 @@ +/** + * 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 = { some: 'thing', ref: null }; + circular.ref = circular; + messageParent(circular); + expect(mockWorkerThreads.parentPort.postMessage).toHaveBeenCalledWith([PARENT_MESSAGE_CUSTOM, { + some: 'thing', + ref: '[Circular]' + }]); + }) +}); + +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 = { some: 'thing', ref: null }; + circular.ref = circular; + messageParent(circular); + expect(process.send).toHaveBeenCalledWith([PARENT_MESSAGE_CUSTOM, { + some: 'thing', + ref: '[Circular]' + }]); + }) +}); diff --git a/packages/jest-worker/src/workers/__tests__/processChild.test.js b/packages/jest-worker/src/workers/__tests__/processChild.test.js index ba579e052efe..247099041f6f 100644 --- a/packages/jest-worker/src/workers/__tests__/processChild.test.js +++ b/packages/jest-worker/src/workers/__tests__/processChild.test.js @@ -73,6 +73,12 @@ beforeEach(() => { return 1989; }, + fooReturnsCircular() { + const circular = { some: 'thing', ref: null }; + circular.ref = circular; + return circular; + }, + setup(param) { initializeParm = param; }, @@ -376,3 +382,24 @@ 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, { + some: 'thing', + ref: '[Circular]' + }]); +}); diff --git a/packages/jest-worker/src/workers/messageParent.ts b/packages/jest-worker/src/workers/messageParent.ts index bf0f828cbb63..ae69cf35ad47 100644 --- a/packages/jest-worker/src/workers/messageParent.ts +++ b/packages/jest-worker/src/workers/messageParent.ts @@ -6,6 +6,7 @@ */ import {PARENT_MESSAGE_CUSTOM} from '../types'; +import fclone from 'fclone'; const isWorkerThread = () => { try { @@ -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'); diff --git a/packages/jest-worker/src/workers/processChild.ts b/packages/jest-worker/src/workers/processChild.ts index 64d29e19e132..bd2053014bb2 100644 --- a/packages/jest-worker/src/workers/processChild.ts +++ b/packages/jest-worker/src/workers/processChild.ts @@ -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, @@ -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) { diff --git a/yarn.lock b/yarn.lock index d55a40221f12..3f3dae57a6b9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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" @@ -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