From 896690d2aca68edbadbb9c554fcc9184f3cf2470 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 4 Feb 2020 23:27:28 +0100
Subject: [PATCH] test: fix flaky test-inspector-connect-main-thread
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, the test waited for a (any) message from the workers,
and then attached another event listener to a specific kind of
message. However, it was possible that the second listener was
attached after the Worker had already exited, thus never receiving
the message it was supposed to receive. (This is the race condition
here – usually, the Worker thread would exit *after* the second
listener was attached.)

Solve this by keeping a single `'message'` event listener attached
to the worker instance during its entire lifetime.

Fixes: https://github.com/nodejs/node/issues/31226

PR-URL: https://github.com/nodejs/node/pull/31637
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 .../test-inspector-connect-main-thread.js     | 40 +++++++------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/test/parallel/test-inspector-connect-main-thread.js b/test/parallel/test-inspector-connect-main-thread.js
index 7f354fb76f9393..bb21a54e90f1d1 100644
--- a/test/parallel/test-inspector-connect-main-thread.js
+++ b/test/parallel/test-inspector-connect-main-thread.js
@@ -50,28 +50,19 @@ function startWorker(skipChild, sharedBuffer) {
       console.error(e);
       throw e;
     });
-    worker.once('message', (m) => {
+    // Add 2 promises to the worker, one resolved when a message with a
+    // .doConsoleLog property is received and one resolved when a .messagesSent
+    // property is received.
+    let resolveConsoleRequest;
+    let resolveMessagesSent;
+    worker.onConsoleRequest =
+      new Promise((resolve) => resolveConsoleRequest = resolve);
+    worker.onMessagesSent =
+      new Promise((resolve) => resolveMessagesSent = resolve);
+    worker.on('message', (m) => {
       resolve(worker);
-    });
-  });
-}
-
-function waitForConsoleRequest(worker) {
-  return new Promise((resolve) => {
-    worker.on('message', ({ doConsoleLog }) => {
-      if (doConsoleLog) {
-        resolve();
-      }
-    });
-  });
-}
-
-function waitForMessagesSent(worker) {
-  return new Promise((resolve) => {
-    worker.on('message', ({ messagesSent }) => {
-      if (messagesSent) {
-        resolve(messagesSent);
-      }
+      if (m.doConsoleLog) resolveConsoleRequest();
+      if (m.messagesSent) resolveMessagesSent(m.messagesSent);
     });
   });
 }
@@ -107,10 +98,9 @@ async function main() {
   const arrayBuffer = new Uint8Array(sharedBuffer);
   arrayBuffer[0] = 1;
   const worker = await startWorker(false, sharedBuffer);
-  waitForConsoleRequest(worker).then(doConsoleLog.bind(null, arrayBuffer));
-  const workerDonePromise = waitForMessagesSent(worker);
+  worker.onConsoleRequest.then(doConsoleLog.bind(null, arrayBuffer));
   assert.strictEqual(toDebug(), 400);
-  assert.deepStrictEqual(await workerDonePromise, [
+  assert.deepStrictEqual(await worker.onMessagesSent, [
     'Debugger.enable',
     'Runtime.enable',
     'Debugger.setBreakpointByUrl',
@@ -122,7 +112,7 @@ async function main() {
 async function childMain() {
   // Ensures the worker does not terminate too soon
   parentPort.on('message', () => { });
-  await waitForMessagesSent(await startWorker(true));
+  await (await startWorker(true)).onMessagesSent;
   const session = new Session();
   session.connectToMainThread();
   await post(session, 'Debugger.enable');