From a47e76f5bf686f95dd3c1bef242178ee316ae8b8 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 9 Jul 2019 16:41:55 +0200 Subject: [PATCH 1/3] test: fix race condition in test-worker-process-cwd.js This simplifies the test logic and fixes the race condition that could happen right now. Refs: https://github.com/nodejs/node/issues/28193 Closes: https://github.com/nodejs/node/pull/28477 Fixes: https://github.com/nodejs/node/issues/27669 --- test/parallel/test-worker-process-cwd.js | 62 ++++++++++++++++-------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/test/parallel/test-worker-process-cwd.js b/test/parallel/test-worker-process-cwd.js index e281610a49d1d0..58556d961e5874 100644 --- a/test/parallel/test-worker-process-cwd.js +++ b/test/parallel/test-worker-process-cwd.js @@ -3,6 +3,9 @@ const common = require('../common'); const assert = require('assert'); const { Worker, isMainThread, parentPort } = require('worker_threads'); +// Verify that cwd changes from the main thread are handled correctly in +// workers. + // Do not use isMainThread directly, otherwise the test would time out in case // it's started inside of another worker thread. if (!process.env.HAS_STARTED_WORKER) { @@ -10,36 +13,53 @@ if (!process.env.HAS_STARTED_WORKER) { if (!isMainThread) { common.skip('This test can only run as main thread'); } + // Normalize the current working dir to also work with the root folder. process.chdir(__dirname); + + // 1. Start the first worker. const w = new Worker(__filename); - process.chdir('..'); - w.on('message', common.mustCall((message) => { + w.once('message', common.mustCall((message) => { + // 5. step: change the cwd and send that to the spawned worker assert.strictEqual(message, process.cwd()); process.chdir('..'); w.postMessage(process.cwd()); })); } else if (!process.env.SECOND_WORKER) { process.env.SECOND_WORKER = '1'; - const firstCwd = process.cwd(); + + // 2. Save the current cwd and verify that `process.cwd` includes the + // Atomics.load call and spawn a new worker. + const cwd = process.cwd(); + assert(process.cwd.toString().includes('Atomics.load')); + const w = new Worker(__filename); - w.on('message', common.mustCall((message) => { - assert.strictEqual(message, process.cwd()); - parentPort.postMessage(firstCwd); - parentPort.onmessage = common.mustCall((obj) => { - const secondCwd = process.cwd(); - assert.strictEqual(secondCwd, obj.data); - assert.notStrictEqual(secondCwd, firstCwd); - w.postMessage(secondCwd); - parentPort.unref(); - }); + w.once('message', common.mustCall((message) => { + // 4. Verify at the current cwd is identical to the received and the + // original one. + assert.strictEqual(process.cwd(), message); + assert.strictEqual(message, cwd); + parentPort.postMessage(cwd); + })); + + parentPort.once('message', common.mustCall((message) => { + // 6. Verify that the current cwd is identical to the received one but not + // with the original one. + assert.strictEqual(process.cwd(), message); + assert.notStrictEqual(message, cwd); + w.postMessage(message); })); } else { - const firstCwd = process.cwd(); - parentPort.postMessage(firstCwd); - parentPort.onmessage = common.mustCall((obj) => { - const secondCwd = process.cwd(); - assert.strictEqual(secondCwd, obj.data); - assert.notStrictEqual(secondCwd, firstCwd); - process.exit(); - }); + // 3. Save the current cwd, post back to the "main" thread and verify that + // `process.cwd` includes the Atomics.load call. + const cwd = process.cwd(); + // Send the current cwd to the parent. + parentPort.postMessage(cwd); + assert(process.cwd.toString().includes('Atomics.load')); + + parentPort.once('message', common.mustCall((message) => { + // 6. Verify that the current cwd is identical to the received one but + // not with the original one. + assert.strictEqual(process.cwd(), message); + assert.notStrictEqual(message, cwd); + })); } From f82fbc39d7b7953ed67b6961a73ebf819eda9c51 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 9 Jul 2019 17:53:13 +0200 Subject: [PATCH 2/3] fixup: remove obsolete "step" in comment --- test/parallel/test-worker-process-cwd.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-worker-process-cwd.js b/test/parallel/test-worker-process-cwd.js index 58556d961e5874..268afc29fdc08c 100644 --- a/test/parallel/test-worker-process-cwd.js +++ b/test/parallel/test-worker-process-cwd.js @@ -19,7 +19,7 @@ if (!process.env.HAS_STARTED_WORKER) { // 1. Start the first worker. const w = new Worker(__filename); w.once('message', common.mustCall((message) => { - // 5. step: change the cwd and send that to the spawned worker + // 5. Change the cwd and send that to the spawned worker. assert.strictEqual(message, process.cwd()); process.chdir('..'); w.postMessage(process.cwd()); From afd8c9510f355db600f770b65501775208a1f7bb Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 10 Jul 2019 12:47:48 +0200 Subject: [PATCH 3/3] fixup: address comment --- test/parallel/test-worker-process-cwd.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-worker-process-cwd.js b/test/parallel/test-worker-process-cwd.js index 268afc29fdc08c..e24515cce777a3 100644 --- a/test/parallel/test-worker-process-cwd.js +++ b/test/parallel/test-worker-process-cwd.js @@ -16,6 +16,8 @@ if (!process.env.HAS_STARTED_WORKER) { // Normalize the current working dir to also work with the root folder. process.chdir(__dirname); + assert(!process.cwd.toString().includes('Atomics.load')); + // 1. Start the first worker. const w = new Worker(__filename); w.once('message', common.mustCall((message) => { @@ -57,7 +59,7 @@ if (!process.env.HAS_STARTED_WORKER) { assert(process.cwd.toString().includes('Atomics.load')); parentPort.once('message', common.mustCall((message) => { - // 6. Verify that the current cwd is identical to the received one but + // 7. Verify that the current cwd is identical to the received one but // not with the original one. assert.strictEqual(process.cwd(), message); assert.notStrictEqual(message, cwd);