Skip to content

Commit 4aebee6

Browse files
BridgeARcodebytere
authored andcommitted
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: #39717 Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #41023 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 164bfe8 commit 4aebee6

File tree

5 files changed

+73
-17
lines changed

5 files changed

+73
-17
lines changed

benchmark/worker/atomics-wait.js

+8-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
'use strict';
2-
/* global SharedArrayBuffer */
2+
3+
if (typeof SharedArrayBuffer === 'undefined') {
4+
throw new Error('SharedArrayBuffers must be enabled to run this benchmark');
5+
}
6+
7+
if (typeof Atomics === 'undefined') {
8+
throw new Error('Atomics must be enabled to run this benchmark');
9+
}
310

411
const common = require('../common.js');
512
const bench = common.createBenchmark(main, {

lib/internal/main/worker_thread.js

+20-15
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ const {
1010
ObjectDefineProperty,
1111
PromisePrototypeThen,
1212
RegExpPrototypeExec,
13-
globalThis: { Atomics },
13+
globalThis: {
14+
Atomics,
15+
SharedArrayBuffer,
16+
},
1417
} = primordials;
1518

1619
const {
@@ -106,21 +109,23 @@ port.on('message', (message) => {
106109

107110
require('internal/worker').assignEnvironmentData(environmentData);
108111

109-
// The counter is only passed to the workers created by the main thread, not
110-
// to workers created by other workers.
111-
let cachedCwd = '';
112-
let lastCounter = -1;
113-
const originalCwd = process.cwd;
114-
115-
process.cwd = function() {
116-
const currentCounter = Atomics.load(cwdCounter, 0);
117-
if (currentCounter === lastCounter)
112+
if (SharedArrayBuffer !== undefined && Atomics !== undefined) {
113+
// The counter is only passed to the workers created by the main thread,
114+
// not to workers created by other workers.
115+
let cachedCwd = '';
116+
let lastCounter = -1;
117+
const originalCwd = process.cwd;
118+
119+
process.cwd = function() {
120+
const currentCounter = Atomics.load(cwdCounter, 0);
121+
if (currentCounter === lastCounter)
122+
return cachedCwd;
123+
lastCounter = currentCounter;
124+
cachedCwd = originalCwd();
118125
return cachedCwd;
119-
lastCounter = currentCounter;
120-
cachedCwd = originalCwd();
121-
return cachedCwd;
122-
};
123-
workerIo.sharedCwdCounter = cwdCounter;
126+
};
127+
workerIo.sharedCwdCounter = cwdCounter;
128+
}
124129

125130
if (manifestSrc) {
126131
require('internal/process/policy').setup(manifestSrc, manifestURL);

lib/internal/worker.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@ let cwdCounter;
9595

9696
const environmentData = new SafeMap();
9797

98-
if (isMainThread) {
98+
// SharedArrayBuffers can be disabled with --no-harmony-sharedarraybuffer.
99+
// Atomics can be disabled with --no-harmony-atomics.
100+
if (isMainThread && SharedArrayBuffer !== undefined && Atomics !== undefined) {
99101
cwdCounter = new Uint32Array(new SharedArrayBuffer(4));
100102
const originalChdir = process.chdir;
101103
process.chdir = function(path) {
+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Flags: --no-harmony-atomics
2+
3+
'use strict';
4+
5+
const common = require('../common');
6+
const assert = require('assert');
7+
const { Worker } = require('worker_threads');
8+
9+
// Regression test for https://github.com/nodejs/node/issues/39717.
10+
11+
// Do not use isMainThread so that this test itself can be run inside a Worker.
12+
if (!process.env.HAS_STARTED_WORKER) {
13+
process.env.HAS_STARTED_WORKER = 1;
14+
const w = new Worker(__filename);
15+
16+
w.on('exit', common.mustCall((status) => {
17+
assert.strictEqual(status, 2);
18+
}));
19+
} else {
20+
process.exit(2);
21+
}

test/parallel/test-worker-no-sab.js

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Flags: --no-harmony-sharedarraybuffer
2+
3+
'use strict';
4+
5+
const common = require('../common');
6+
const assert = require('assert');
7+
const { Worker } = require('worker_threads');
8+
9+
// Regression test for https://github.com/nodejs/node/issues/39717.
10+
11+
// Do not use isMainThread so that this test itself can be run inside a Worker.
12+
if (!process.env.HAS_STARTED_WORKER) {
13+
process.env.HAS_STARTED_WORKER = 1;
14+
const w = new Worker(__filename);
15+
16+
w.on('exit', common.mustCall((status) => {
17+
assert.strictEqual(status, 2);
18+
}));
19+
} else {
20+
process.exit(2);
21+
}

0 commit comments

Comments
 (0)