Skip to content

Commit

Permalink
test: fix flaky test-inspector-connect-main-thread
Browse files Browse the repository at this point in the history
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: #31226

PR-URL: #31637
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
addaleax authored and codebytere committed Feb 17, 2020
1 parent 348c787 commit 6139d4e
Showing 1 changed file with 15 additions and 25 deletions.
40 changes: 15 additions & 25 deletions test/parallel/test-inspector-connect-main-thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
}
Expand Down Expand Up @@ -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',
Expand All @@ -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');
Expand Down

0 comments on commit 6139d4e

Please sign in to comment.