Skip to content

Commit b92c9b5

Browse files
authored
worker: eliminate race condition in process.cwd()
Fixes a race condition in worker thread cwd caching where the counter is incremented before the directory change completes, allowing workers to cache stale directory values. In lib/internal/worker.js, the main thread's process.chdir() wrapper previously incremented the shared counter before calling the actual chdir(), creating a race window where workers could read the old directory but cache it with the new counter value. This caused subsequent cwd() calls to return incorrect paths until the next chdir(). This fix reorders the operations to change the directory first, then increment the counter, ensuring workers are only notified after the directory change completes. Before fix: 54.28% error rate (311/573 races) After fix: 0% error rate (0/832 races) Refs: https://hackerone.com/reports/3407207 Co-authored-by: Giulio Comi Co-authored-by: Caleb Everett Co-authored-by: Utku Yildirim PR-URL: #61664 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
1 parent f13d7bf commit b92c9b5

File tree

2 files changed

+71
-1
lines changed

2 files changed

+71
-1
lines changed

lib/internal/worker.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,8 @@ if (isMainThread) {
112112
cwdCounter = new Uint32Array(constructSharedArrayBuffer(4));
113113
const originalChdir = process.chdir;
114114
process.chdir = function(path) {
115-
AtomicsAdd(cwdCounter, 0, 1);
116115
originalChdir(path);
116+
AtomicsAdd(cwdCounter, 0, 1);
117117
};
118118
}
119119

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Flags: --expose-internals --no-warnings
2+
'use strict';
3+
4+
const common = require('../common');
5+
const { isMainThread } = require('worker_threads');
6+
7+
if (!isMainThread) {
8+
common.skip('process.chdir is not available in Workers');
9+
}
10+
11+
const { internalBinding } = require('internal/test/binding');
12+
13+
const assert = require('assert');
14+
const { Worker } = require('worker_threads');
15+
16+
const processBinding = internalBinding('process_methods');
17+
const originalChdir = processBinding.chdir;
18+
19+
const cwdOriginal = process.cwd();
20+
const i32 = new Int32Array(new SharedArrayBuffer(12));
21+
22+
processBinding.chdir = common.mustCall(function chdir(path) {
23+
// Signal to the worker that we're inside the chdir call
24+
Atomics.store(i32, 0, 1);
25+
Atomics.notify(i32, 0);
26+
27+
// Pause the chdir call while the worker calls process.cwd(),
28+
// to simulate a race condition
29+
Atomics.wait(i32, 1, 0);
30+
31+
return originalChdir(path);
32+
});
33+
34+
const worker = new Worker(`
35+
const {
36+
parentPort,
37+
workerData: { i32 },
38+
} = require('worker_threads');
39+
40+
// Wait until the main thread has entered the chdir call
41+
Atomics.wait(i32, 0, 0);
42+
43+
const cwdDuringChdir = process.cwd();
44+
45+
// Signal the main thread to continue the chdir call
46+
Atomics.store(i32, 1, 1);
47+
Atomics.notify(i32, 1);
48+
49+
// Wait until the main thread has left the chdir call
50+
Atomics.wait(i32, 2, 0);
51+
52+
const cwdAfterChdir = process.cwd();
53+
parentPort.postMessage({ cwdDuringChdir, cwdAfterChdir });
54+
`, {
55+
eval: true,
56+
workerData: { i32 },
57+
});
58+
59+
worker.on('exit', common.mustCall());
60+
worker.on('error', common.mustNotCall());
61+
worker.on('message', common.mustCall(({ cwdDuringChdir, cwdAfterChdir }) => {
62+
assert.strictEqual(cwdDuringChdir, cwdOriginal);
63+
assert.strictEqual(cwdAfterChdir, process.cwd());
64+
}));
65+
66+
process.chdir('..');
67+
68+
// Signal to the worker that the chdir call is completed
69+
Atomics.store(i32, 2, 1);
70+
Atomics.notify(i32, 2);

0 commit comments

Comments
 (0)