Don't detect async handles already queued to close

Some types of async resources in Node.js are not destroyed until *after* their `close` or `end` or similar callbacks and events run, leading to a situation where the `--detectOpenHandles` option can falsely flag resources that have been cleaned up in user code and are already scheduled for destruction. For example, if a test ends from the callback to `TCPServer.close()` and no other tests or lifecycle functions impose additional delays, `--detectOpenHandles` will flag the server even though it has been closed. This is the main cause of issues people encounter with Supertest (see jestjs#8554).

This addresses the issue by adding a short delay before collecting open handles.

Depends on jestjs#11382.

Author: Mr0grog

e2e/__tests__/__snapshots__/detectOpenHandles.ts.snap

@@ -1,5 +1,7 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`does not print info about open handlers for a server that is already closed 1`] = ``;
+
 exports[`prints message about flag on forceExit 1`] = `Force exiting Jest: Have you considered using \`--detectOpenHandles\` to detect async operations that kept running after all tests finished?`;
 
 exports[`prints message about flag on slow tests 1`] = `

e2e/__tests__/detectOpenHandles.ts

@@ -155,3 +155,15 @@ it('prints out info about open handlers from lifecycle functions with a `done` c
 
   expect(wrap(textAfterTest)).toMatchSnapshot();
 });
+
+it('does not print info about open handlers for a server that is already closed', async () => {
+  const run = runContinuous('detect-open-handles', [
+    'recently-closed',
+    '--detectOpenHandles',
+  ]);
+  await run.waitUntil(({stderr}) => stderr.includes('Ran all test suites'));
+  const {stderr} = await run.end();
+  const textAfterTest = getTextAfterTest(stderr);
+
+  expect(wrap(textAfterTest)).toMatchSnapshot();
+});

e2e/detect-open-handles/__tests__/recently-closed.js

@@ -0,0 +1,20 @@
+/**
+ * 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 {createServer} from 'http';
+
+test('a recently closed server should not be detected by --detectOpenHandles', done => {
+  const server = createServer((_, response) => response.end('ok'));
+  server.listen(0, () => {
+    expect(true).toBe(true);
+
+    // Close server and return immediately on callback. During the "close"
+    // callback, async hooks usually have not yet been called, but we want to
+    // make sure Jest can figure out that this server is closed.
+    server.close(done);
+  });
+});

packages/jest-core/src/__tests__/collectHandles.test.js

@@ -11,25 +11,25 @@ import {PerformanceObserver} from 'perf_hooks';
 import collectHandles from '../collectHandles';
 
 describe('collectHandles', () => {
-  it('should collect Timeout', () => {
+  it('should collect Timeout', async () => {
     const handleCollector = collectHandles();
 
     const interval = setInterval(() => {}, 100);
 
-    const openHandles = handleCollector();
+    const openHandles = await handleCollector();
 
     expect(openHandles).toHaveLength(1);
     expect(openHandles[0].message).toContain('Timeout');
 
     clearInterval(interval);
   });
 
-  it('should not collect the PerformanceObserver open handle', () => {
+  it('should not collect the PerformanceObserver open handle', async () => {
     const handleCollector = collectHandles();
     const obs = new PerformanceObserver((list, observer) => {});
     obs.observe({entryTypes: ['mark']});
 
-    const openHandles = handleCollector();
+    const openHandles = await handleCollector();
 
     expect(openHandles).toHaveLength(0);
     obs.disconnect();
@@ -40,14 +40,28 @@ describe('collectHandles', () => {
     const server = http.createServer((_, response) => response.end('ok'));
     server.listen(0, () => {
       // Collect results while server is still open.
-      const openHandles = handleCollector();
-
-      server.close(() => {
-        expect(openHandles).toContainEqual(
-          expect.objectContaining({message: 'TCPSERVERWRAP'}),
-        );
-        done();
-      });
+      handleCollector()
+        .then(openHandles => {
+          server.close(() => {
+            expect(openHandles).toContainEqual(
+              expect.objectContaining({message: 'TCPSERVERWRAP'}),
+            );
+            done();
+          });
+        })
+        .catch(done);
     });
   });
+
+  it('should not collect handles that have been queued to close', async () => {
+    const handleCollector = collectHandles();
+    const server = http.createServer((_, response) => response.end('ok'));
+
+    // Start and stop server.
+    await new Promise(r => server.listen(0, r));
+    await new Promise(r => server.close(r));
+
+    const openHandles = await handleCollector();
+    expect(openHandles).toHaveLength(0);
+  });
 });

packages/jest-core/src/collectHandles.ts

@@ -8,12 +8,13 @@
 /* eslint-disable local/ban-types-eventually */
 
 import * as asyncHooks from 'async_hooks';
+import {promisify} from 'util';
 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<Error>;
+export type HandleCollectionResult = () => Promise<Array<Error>>;
 
 function stackIsFromUser(stack: string) {
   // Either the test file, or something required by it
@@ -41,6 +42,8 @@ const alwaysActive = () => true;
 
 const hasWeakRef = typeof WeakRef === 'function';
 
+const nextTask = promisify(setImmediate);
+
 // 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 +103,14 @@ export default function collectHandles(): HandleCollectionResult {
 
   hook.enable();
 
-  return () => {
+  return async () => {
+    // Wait until the next JS task for any async resources that have been queued
+    // for destruction to actually be destroyed.
+    // For example, Node.js TCP Servers are not destroyed until *after* their
+    // `close` callback runs. If someone finishes a test from the `close`
+    // callback, we will not yet have seen the resource be destroyed here.
+    await nextTask();
+
     hook.disable();
 
     // Get errors for every async resource still referenced at this moment

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 = [];
   }
@@ -280,7 +280,7 @@ export default async function runJest({
     await runGlobalHook({allTests, globalConfig, moduleName: 'globalTeardown'});
   }
 
-  processResults(results, {
+  await processResults(results, {
     collectHandles,
     json: globalConfig.json,
     onComplete,