From d7846e4076271c57896ccc8cb5e8d359af50d94a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 29 Nov 2021 21:54:35 +0100 Subject: [PATCH] lib: do not crash using workers with disabled shared array buffers 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 --- 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 a771b1813731ed..ff4b6fbf10dc59 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 91ca93e6e10062..df2561b61feed7 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -9,7 +9,10 @@ const { ArrayPrototypeSplice, ObjectDefineProperty, PromisePrototypeCatch, - globalThis: { Atomics }, + globalThis: { + Atomics, + SharedArrayBuffer, + }, } = primordials; const { @@ -141,21 +144,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; + } const CJSLoader = require('internal/modules/cjs/loader'); assert(!CJSLoader.hasLoadedAnyUserCJSModule); diff --git a/lib/internal/worker.js b/lib/internal/worker.js index 0cede10ca4bf9b..d60f7b936161f8 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -91,7 +91,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); +}