From 74ed376a9149a3e9930eff38f841af2f76317858 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Thu, 8 Apr 2021 09:12:57 +0200 Subject: [PATCH 1/2] fix: run GC before collecting open handles --- CHANGELOG.md | 1 + e2e/__tests__/detectOpenHandles.ts | 11 ++++++++ e2e/detect-open-handles/__tests__/crypto.js | 13 +++++++++ packages/jest-core/src/collectHandles.ts | 29 +++++++++++++++++++-- packages/jest-core/src/runJest.ts | 6 ++--- 5 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 e2e/detect-open-handles/__tests__/crypto.js diff --git a/CHANGELOG.md b/CHANGELOG.md index d323b4d7ed35..8fca48ea0867 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ - `[jest-console]` `console.dir` now respects the second argument correctly ([#10638](https://github.com/facebook/jest/pull/10638)) - `[jest-core]` Don't report PerformanceObserver as open handle ([#11123](https://github.com/facebook/jest/pull/11123)) - `[jest-core]` Use `WeakRef` to hold timers when detecting open handles ([#11277](https://github.com/facebook/jest/pull/11277)) +- `[jest-core]` Run GC detecting open handles ([#11278](https://github.com/facebook/jest/pull/11278)) - `[jest-each]` [**BREAKING**] Ignore excess words in headings ([#8766](https://github.com/facebook/jest/pull/8766)) - `[jest-environment]` [**BREAKING**] Drop support for `runScript` for test environments ([#11155](https://github.com/facebook/jest/pull/11155)) - `[jest-environment-jsdom]` Use inner realm’s `ArrayBuffer` constructor ([#10885](https://github.com/facebook/jest/pull/10885)) diff --git a/e2e/__tests__/detectOpenHandles.ts b/e2e/__tests__/detectOpenHandles.ts index 977434965a3f..3a302f752c90 100644 --- a/e2e/__tests__/detectOpenHandles.ts +++ b/e2e/__tests__/detectOpenHandles.ts @@ -71,6 +71,17 @@ it('does not report promises', () => { expect(textAfterTest).toBe(''); }); +it('does not report crypto random data', () => { + // The test here is basically that it exits cleanly without reporting anything (does not need `until`) + const {stderr} = runJest('detect-open-handles', [ + 'crypto', + '--detectOpenHandles', + ]); + const textAfterTest = getTextAfterTest(stderr); + + expect(textAfterTest).toBe(''); +}); + onNodeVersions('>=11.10.0', () => { it('does not report ELD histograms', () => { const {stderr} = runJest('detect-open-handles', [ diff --git a/e2e/detect-open-handles/__tests__/crypto.js b/e2e/detect-open-handles/__tests__/crypto.js new file mode 100644 index 000000000000..1a4284994d8e --- /dev/null +++ b/e2e/detect-open-handles/__tests__/crypto.js @@ -0,0 +1,13 @@ +/** + * 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 {randomFillSync} = require('crypto'); + +test('randomFillSync()', () => { + const buf = Buffer.alloc(10); + randomFillSync(buf); +}); diff --git a/packages/jest-core/src/collectHandles.ts b/packages/jest-core/src/collectHandles.ts index 7bdf62b5ccb7..633ba3756ff4 100644 --- a/packages/jest-core/src/collectHandles.ts +++ b/packages/jest-core/src/collectHandles.ts @@ -8,12 +8,15 @@ /* eslint-disable local/ban-types-eventually */ import * as asyncHooks from 'async_hooks'; +import {promisify} from 'util'; +import {setFlagsFromString} from 'v8'; +import {runInNewContext} from 'vm'; import stripAnsi = require('strip-ansi'); import type {Config} from '@jest/types'; import {formatExecError} from 'jest-message-util'; import {ErrorWithStack} from 'jest-util'; -export type HandleCollectionResult = () => Array; +export type HandleCollectionResult = () => Promise>; function stackIsFromUser(stack: string) { // Either the test file, or something required by it @@ -41,6 +44,21 @@ const alwaysActive = () => true; const hasWeakRef = typeof WeakRef === 'function'; +const tick = promisify(setImmediate); + +function runGarbageCollector() { + const isGarbageCollectorHidden = !global.gc; + + // GC is usually hidden, so we have to expose it before running. + setFlagsFromString('--expose-gc'); + runInNewContext('gc')(); + + // The GC was not initially exposed, so let's hide it again. + if (isGarbageCollectorHidden) { + setFlagsFromString('--no-expose-gc'); + } +} + // Inspired by https://github.com/mafintosh/why-is-node-running/blob/master/index.js // Extracted as we want to format the result ourselves export default function collectHandles(): HandleCollectionResult { @@ -100,7 +118,14 @@ export default function collectHandles(): HandleCollectionResult { hook.enable(); - return () => { + return async () => { + runGarbageCollector(); + + // wait some ticks to allow GC to run properly, see https://github.com/nodejs/node/issues/34636#issuecomment-669366235 + for (let i = 0; i < 10; i++) { + await tick(); + } + hook.disable(); // Get errors for every async resource still referenced at this moment diff --git a/packages/jest-core/src/runJest.ts b/packages/jest-core/src/runJest.ts index 65f191802eac..9000e6da7b5a 100644 --- a/packages/jest-core/src/runJest.ts +++ b/packages/jest-core/src/runJest.ts @@ -75,7 +75,7 @@ type ProcessResultOptions = Pick< outputStream: NodeJS.WriteStream; }; -const processResults = ( +const processResults = async ( runResults: AggregatedResult, options: ProcessResultOptions, ) => { @@ -89,7 +89,7 @@ const processResults = ( } = options; if (collectHandles) { - runResults.openHandles = collectHandles(); + runResults.openHandles = await collectHandles(); } else { runResults.openHandles = []; } @@ -278,7 +278,7 @@ export default async function runJest({ await runGlobalHook({allTests, globalConfig, moduleName: 'globalTeardown'}); } - processResults(results, { + await processResults(results, { collectHandles, json: globalConfig.json, onComplete, From cb6ae169cb41c312efa6a66f39b2f3bba49d6932 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Thu, 8 Apr 2021 09:44:49 +0200 Subject: [PATCH 2/2] do not collect RANDOMBYTESREQUEST --- CHANGELOG.md | 2 +- packages/jest-core/src/collectHandles.ts | 32 +++--------------------- packages/jest-core/src/runJest.ts | 6 ++--- 3 files changed, 8 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fca48ea0867..2a7d61e3b338 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,7 +50,7 @@ - `[jest-console]` `console.dir` now respects the second argument correctly ([#10638](https://github.com/facebook/jest/pull/10638)) - `[jest-core]` Don't report PerformanceObserver as open handle ([#11123](https://github.com/facebook/jest/pull/11123)) - `[jest-core]` Use `WeakRef` to hold timers when detecting open handles ([#11277](https://github.com/facebook/jest/pull/11277)) -- `[jest-core]` Run GC detecting open handles ([#11278](https://github.com/facebook/jest/pull/11278)) +- `[jest-core]` Do not collect `RANDOMBYTESREQUEST` as open handles ([#11278](https://github.com/facebook/jest/pull/11278)) - `[jest-each]` [**BREAKING**] Ignore excess words in headings ([#8766](https://github.com/facebook/jest/pull/8766)) - `[jest-environment]` [**BREAKING**] Drop support for `runScript` for test environments ([#11155](https://github.com/facebook/jest/pull/11155)) - `[jest-environment-jsdom]` Use inner realm’s `ArrayBuffer` constructor ([#10885](https://github.com/facebook/jest/pull/10885)) diff --git a/packages/jest-core/src/collectHandles.ts b/packages/jest-core/src/collectHandles.ts index 633ba3756ff4..45fd328dab22 100644 --- a/packages/jest-core/src/collectHandles.ts +++ b/packages/jest-core/src/collectHandles.ts @@ -8,15 +8,12 @@ /* eslint-disable local/ban-types-eventually */ import * as asyncHooks from 'async_hooks'; -import {promisify} from 'util'; -import {setFlagsFromString} from 'v8'; -import {runInNewContext} from 'vm'; import stripAnsi = require('strip-ansi'); import type {Config} from '@jest/types'; import {formatExecError} from 'jest-message-util'; import {ErrorWithStack} from 'jest-util'; -export type HandleCollectionResult = () => Promise>; +export type HandleCollectionResult = () => Array; function stackIsFromUser(stack: string) { // Either the test file, or something required by it @@ -44,21 +41,6 @@ const alwaysActive = () => true; const hasWeakRef = typeof WeakRef === 'function'; -const tick = promisify(setImmediate); - -function runGarbageCollector() { - const isGarbageCollectorHidden = !global.gc; - - // GC is usually hidden, so we have to expose it before running. - setFlagsFromString('--expose-gc'); - runInNewContext('gc')(); - - // The GC was not initially exposed, so let's hide it again. - if (isGarbageCollectorHidden) { - setFlagsFromString('--no-expose-gc'); - } -} - // Inspired by https://github.com/mafintosh/why-is-node-running/blob/master/index.js // Extracted as we want to format the result ourselves export default function collectHandles(): HandleCollectionResult { @@ -80,7 +62,8 @@ export default function collectHandles(): HandleCollectionResult { type === 'PROMISE' || type === 'TIMERWRAP' || type === 'ELDHISTOGRAM' || - type === 'PerformanceObserver' + type === 'PerformanceObserver' || + type === 'RANDOMBYTESREQUEST' ) { return; } @@ -118,14 +101,7 @@ export default function collectHandles(): HandleCollectionResult { hook.enable(); - return async () => { - runGarbageCollector(); - - // wait some ticks to allow GC to run properly, see https://github.com/nodejs/node/issues/34636#issuecomment-669366235 - for (let i = 0; i < 10; i++) { - await tick(); - } - + return () => { hook.disable(); // Get errors for every async resource still referenced at this moment diff --git a/packages/jest-core/src/runJest.ts b/packages/jest-core/src/runJest.ts index 9000e6da7b5a..65f191802eac 100644 --- a/packages/jest-core/src/runJest.ts +++ b/packages/jest-core/src/runJest.ts @@ -75,7 +75,7 @@ type ProcessResultOptions = Pick< outputStream: NodeJS.WriteStream; }; -const processResults = async ( +const processResults = ( runResults: AggregatedResult, options: ProcessResultOptions, ) => { @@ -89,7 +89,7 @@ const processResults = async ( } = options; if (collectHandles) { - runResults.openHandles = await collectHandles(); + runResults.openHandles = collectHandles(); } else { runResults.openHandles = []; } @@ -278,7 +278,7 @@ export default async function runJest({ await runGlobalHook({allTests, globalConfig, moduleName: 'globalTeardown'}); } - await processResults(results, { + processResults(results, { collectHandles, json: globalConfig.json, onComplete,