mirror of
https://github.com/nodejs/node.git
synced 2026-10-14 10:26:26 -04:00
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: https://github.com/nodejs/node/pull/61664 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
This commit is contained in:
2 files changed
+71
-1
No files matched your search
@@ -112,8 +112,8 @@ if (isMainThread) {
|
||||
cwdCounter = new Uint32Array(constructSharedArrayBuffer(4));
|
||||
const originalChdir = process.chdir;
|
||||
process.chdir = function(path) {
|
||||
AtomicsAdd(cwdCounter, 0, 1);
|
||||
originalChdir(path);
|
||||
AtomicsAdd(cwdCounter, 0, 1);
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
// Flags: --expose-internals --no-warnings
|
||||
'use strict';
|
||||
|
||||
const common = require('../common');
|
||||
const { isMainThread } = require('worker_threads');
|
||||
|
||||
if (!isMainThread) {
|
||||
common.skip('process.chdir is not available in Workers');
|
||||
}
|
||||
|
||||
const { internalBinding } = require('internal/test/binding');
|
||||
|
||||
const assert = require('assert');
|
||||
const { Worker } = require('worker_threads');
|
||||
|
||||
const processBinding = internalBinding('process_methods');
|
||||
const originalChdir = processBinding.chdir;
|
||||
|
||||
const cwdOriginal = process.cwd();
|
||||
const i32 = new Int32Array(new SharedArrayBuffer(12));
|
||||
|
||||
processBinding.chdir = common.mustCall(function chdir(path) {
|
||||
// Signal to the worker that we're inside the chdir call
|
||||
Atomics.store(i32, 0, 1);
|
||||
Atomics.notify(i32, 0);
|
||||
|
||||
// Pause the chdir call while the worker calls process.cwd(),
|
||||
// to simulate a race condition
|
||||
Atomics.wait(i32, 1, 0);
|
||||
|
||||
return originalChdir(path);
|
||||
});
|
||||
|
||||
const worker = new Worker(`
|
||||
const {
|
||||
parentPort,
|
||||
workerData: { i32 },
|
||||
} = require('worker_threads');
|
||||
|
||||
// Wait until the main thread has entered the chdir call
|
||||
Atomics.wait(i32, 0, 0);
|
||||
|
||||
const cwdDuringChdir = process.cwd();
|
||||
|
||||
// Signal the main thread to continue the chdir call
|
||||
Atomics.store(i32, 1, 1);
|
||||
Atomics.notify(i32, 1);
|
||||
|
||||
// Wait until the main thread has left the chdir call
|
||||
Atomics.wait(i32, 2, 0);
|
||||
|
||||
const cwdAfterChdir = process.cwd();
|
||||
parentPort.postMessage({ cwdDuringChdir, cwdAfterChdir });
|
||||
`, {
|
||||
eval: true,
|
||||
workerData: { i32 },
|
||||
});
|
||||
|
||||
worker.on('exit', common.mustCall());
|
||||
worker.on('error', common.mustNotCall());
|
||||
worker.on('message', common.mustCall(({ cwdDuringChdir, cwdAfterChdir }) => {
|
||||
assert.strictEqual(cwdDuringChdir, cwdOriginal);
|
||||
assert.strictEqual(cwdAfterChdir, process.cwd());
|
||||
}));
|
||||
|
||||
process.chdir('..');
|
||||
|
||||
// Signal to the worker that the chdir call is completed
|
||||
Atomics.store(i32, 2, 1);
|
||||
Atomics.notify(i32, 2);
|
||||
Reference in new issue
Block a user