Skip to content

Commit 46a22ab

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 <[email protected]> Co-authored-by: Shelley Vohr <[email protected]> PR-URL: #41023 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent 847d740 commit 46a22ab

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 {
@@ -105,21 +108,23 @@ port.on('message', (message) => {
105108

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

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

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

lib/internal/worker.js

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

9292
const environmentData = new SafeMap();
9393

94-
if (isMainThread) {
94+
// SharedArrayBuffers can be disabled with --no-harmony-sharedarraybuffer.
95+
// Atomics can be disabled with --no-harmony-atomics.
96+
if (isMainThread && SharedArrayBuffer !== undefined && Atomics !== undefined) {
9597
cwdCounter = new Uint32Array(new SharedArrayBuffer(4));
9698
const originalChdir = process.chdir;
9799
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)