From fbd55a81390ca29954ada4c82c34f8d0a0b5c34e Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 18 Feb 2023 11:18:04 +0100 Subject: [PATCH] lib: do not crash using workers with disabled shared array buffers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows the repl to function normally while using the `--no-harmony-sharedarraybuffer` V8 flag. It also fixes using workers while using the `--no-harmony-atomics` V8 flag. Fixes: https://github.com/nodejs/node/issues/39717 Signed-off-by: Ruben Bridgewater Co-authored-by: Shelley Vohr PR-URL: https://github.com/nodejs/node/pull/41023 Reviewed-By: James M Snell Reviewed-By: Tobias Nießen Reviewed-By: Antoine du Hamel --- benchmark/worker/atomics-wait.js | 9 ++++++- lib/internal/main/worker_thread.js | 35 ++++++++++++++----------- lib/internal/worker.js | 4 ++- test/parallel/test-worker-no-atomics.js | 21 +++++++++++++++ test/parallel/test-worker-no-sab.js | 21 +++++++++++++++ 5 files changed, 73 insertions(+), 17 deletions(-) create mode 100644 test/parallel/test-worker-no-atomics.js create mode 100644 test/parallel/test-worker-no-sab.js diff --git a/benchmark/worker/atomics-wait.js b/benchmark/worker/atomics-wait.js index 7270f9f01c98ec..2e6e01f881c66a 100644 --- a/benchmark/worker/atomics-wait.js +++ b/benchmark/worker/atomics-wait.js @@ -1,5 +1,12 @@ 'use strict'; -/* global SharedArrayBuffer */ + +if (typeof SharedArrayBuffer === 'undefined') { + throw new Error('SharedArrayBuffers must be enabled to run this benchmark'); +} + +if (typeof Atomics === 'undefined') { + throw new Error('Atomics must be enabled to run this benchmark'); +} const common = require('../common.js'); const bench = common.createBenchmark(main, { diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index 08adb0b425bb3b..ac6c9e48ee908c 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -10,7 +10,10 @@ const { ObjectDefineProperty, PromisePrototypeThen, RegExpPrototypeExec, - globalThis: { Atomics }, + globalThis: { + Atomics, + SharedArrayBuffer, + }, } = primordials; const { @@ -106,21 +109,23 @@ port.on('message', (message) => { require('internal/worker').assignEnvironmentData(environmentData); - // The counter is only passed to the workers created by the main thread, not - // to workers created by other workers. - let cachedCwd = ''; - let lastCounter = -1; - const originalCwd = process.cwd; - - process.cwd = function() { - const currentCounter = Atomics.load(cwdCounter, 0); - if (currentCounter === lastCounter) + if (SharedArrayBuffer !== undefined && Atomics !== undefined) { + // The counter is only passed to the workers created by the main thread, + // not to workers created by other workers. + let cachedCwd = ''; + let lastCounter = -1; + const originalCwd = process.cwd; + + process.cwd = function() { + const currentCounter = Atomics.load(cwdCounter, 0); + if (currentCounter === lastCounter) + return cachedCwd; + lastCounter = currentCounter; + cachedCwd = originalCwd(); return cachedCwd; - lastCounter = currentCounter; - cachedCwd = originalCwd(); - return cachedCwd; - }; - workerIo.sharedCwdCounter = cwdCounter; + }; + workerIo.sharedCwdCounter = cwdCounter; + } if (manifestSrc) { require('internal/process/policy').setup(manifestSrc, manifestURL); diff --git a/lib/internal/worker.js b/lib/internal/worker.js index 0120bc6f4d87dd..ccb4e7d12a5541 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -95,7 +95,9 @@ let cwdCounter; const environmentData = new SafeMap(); -if (isMainThread) { +// SharedArrayBuffers can be disabled with --no-harmony-sharedarraybuffer. +// Atomics can be disabled with --no-harmony-atomics. +if (isMainThread && SharedArrayBuffer !== undefined && Atomics !== undefined) { cwdCounter = new Uint32Array(new SharedArrayBuffer(4)); const originalChdir = process.chdir; process.chdir = function(path) { diff --git a/test/parallel/test-worker-no-atomics.js b/test/parallel/test-worker-no-atomics.js new file mode 100644 index 00000000000000..3dd5a47c127d7c --- /dev/null +++ b/test/parallel/test-worker-no-atomics.js @@ -0,0 +1,21 @@ +// Flags: --no-harmony-atomics + +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker_threads'); + +// Regression test for https://github.com/nodejs/node/issues/39717. + +// Do not use isMainThread so that this test itself can be run inside a Worker. +if (!process.env.HAS_STARTED_WORKER) { + process.env.HAS_STARTED_WORKER = 1; + const w = new Worker(__filename); + + w.on('exit', common.mustCall((status) => { + assert.strictEqual(status, 2); + })); +} else { + process.exit(2); +} diff --git a/test/parallel/test-worker-no-sab.js b/test/parallel/test-worker-no-sab.js new file mode 100644 index 00000000000000..3721795671a140 --- /dev/null +++ b/test/parallel/test-worker-no-sab.js @@ -0,0 +1,21 @@ +// Flags: --no-harmony-sharedarraybuffer + +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker_threads'); + +// Regression test for https://github.com/nodejs/node/issues/39717. + +// Do not use isMainThread so that this test itself can be run inside a Worker. +if (!process.env.HAS_STARTED_WORKER) { + process.env.HAS_STARTED_WORKER = 1; + const w = new Worker(__filename); + + w.on('exit', common.mustCall((status) => { + assert.strictEqual(status, 2); + })); +} else { + process.exit(2); +}