Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 18 additions & 30 deletions src/js/node/worker_threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,33 +460,23 @@ function setupWorkerStdio(stdio) {
const { stdin, stdout, stderr } = stdio;
const stdoutStream = makePortWritable(stdout);
const stderrStream = makePortWritable(stderr);
Object.defineProperty(process, "stdout", {
value: stdoutStream,
writable: true,
configurable: true,
enumerable: true,
});
Object.defineProperty(process, "stderr", {
value: stderrStream,
writable: true,
configurable: true,
enumerable: true,
});
const proc: any = process;
// Plain assignment, not Object.defineProperty: process.stdout/stderr/stdin are
// lazy native properties. Assigning replaces one unbuilt; defineProperty first
// builds the fd-backed stream it is about to discard (loading tty/net/fs to do
// so). Either way the result is a writable, enumerable, configurable own property.
Comment thread
robobun marked this conversation as resolved.
Outdated
proc.stdout = stdoutStream;
proc.stderr = stderrStream;
// node always replaces a worker's process.stdin: port-backed when { stdin: true },
// otherwise an immediately-EOF'd stream — never the process-wide fd 0, which
// would race the main thread (and hang on a TTY).
Object.defineProperty(process, "stdin", {
value: stdin
? makePortReadable(stdin, true)
: new Readable({
read() {
this.push(null);
},
}),
writable: true,
configurable: true,
enumerable: true,
});
proc.stdin = stdin
? makePortReadable(stdin, true)
: new Readable({
read() {
this.push(null);
},
});
// node routes console.log through process.stdout/stderr; Bun's global console
// writes the fd directly, so rebind it to the port-backed streams.
const { Console } = require("node:console");
Expand Down Expand Up @@ -864,19 +854,17 @@ function fakeParentPort() {
if (!isMainThread && _isNodeWorker) {
applyWorkerProcessOverrides();
}
// Only ever assigns or defines individual properties: `delete process.x` makes JSC
// reify every lazy property of the process object (stdio streams, env, versions,
// ...). The main-only internals node deletes in workers (process._debugProcess & co.)
// are therefore never defined on a worker's process to begin with (BunProcess.cpp).
Comment thread
robobun marked this conversation as resolved.
Outdated
function applyWorkerProcessOverrides() {
const proc: any = process;
// node defaults debugPort to 9229 in workers (still settable). Per-object property:
// the static accessor's setter writes a process-global shared across threads.
try {
Object.defineProperty(proc, "debugPort", { value: 9229, writable: true, configurable: true, enumerable: true });
} catch {}
// These main-only internals are absent on a worker's process.
for (const k of ["_startProfilerIdleNotifier", "_stopProfilerIdleNotifier", "_debugProcess", "_debugEnd"]) {
try {
delete proc[k];
} catch {}
}
// process.umask(setMask) is unsupported in workers; the getter still works.
const realUmask = proc.umask;
function umask(mask?: unknown) {
Expand Down
16 changes: 12 additions & 4 deletions src/jsc/bindings/BunProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4840,17 +4840,13 @@ extern "C" void Process__emitErrorEvent(Zig::GlobalObject* global, EncodedJSValu

/* Source for Process.lut.h
@begin processObjectTable
_debugEnd Process_stubEmptyFunction Function 0
_debugProcess Process_stubEmptyFunction Function 0
_eval processGetEval CustomAccessor
_getActiveHandles Process_stubFunctionReturningArray Function 0
_getActiveRequests Process_stubFunctionReturningArray Function 0
_kill Process_functionReallyKill Function 2
_linkedBinding Process_stubEmptyFunction Function 0
_preload_modules Process_stubEmptyArray PropertyCallback
_rawDebug constructRawDebug PropertyCallback
_startProfilerIdleNotifier Process_stubEmptyFunction Function 0
_stopProfilerIdleNotifier Process_stubEmptyFunction Function 0
_tickCallback Process_stubEmptyFunction Function 0
abort Process_functionAbort Function 1
allowedNodeEnvironmentFlags constructAllowedNodeEnvironmentFlags PropertyCallback
Expand Down Expand Up @@ -4970,6 +4966,18 @@ void Process::finishCreation(JSC::VM& vm)

putDirect(vm, vm.propertyNames->toStringTagSymbol, jsString(vm, String("process"_s)), 0);
putDirect(vm, Identifier::fromString(vm, "_exiting"_s), jsBoolean(false), 0);

// Node's worker threads have no process._debugProcess & co. These are own
// properties rather than processObjectTable entries because deleting a
// static-table property makes JSC reify every lazy property in the table
// (the stdio streams, env, versions, config, ...): exactly the work a
// worker's startup is supposed to skip.
Comment thread
robobun marked this conversation as resolved.
Outdated
if (!WebCore::clientData(vm)->isWorkerVM()) {
auto* globalObject = this->globalObject();
for (auto name : { "_debugProcess"_s, "_debugEnd"_s, "_startProfilerIdleNotifier"_s, "_stopProfilerIdleNotifier"_s })
putDirectNativeFunction(vm, globalObject, Identifier::fromString(vm, name), 0, Process_stubEmptyFunction, ImplementationVisibility::Public, NoIntrinsic, 0);
}
Comment thread
robobun marked this conversation as resolved.
Outdated

// Node's addReadOnlyProcessAlias: read-only so `process.noDeprecation = false`
// is ignored, but a per-Process property — a Worker must not flip the main
// thread. Unflagged it stays an ordinary undefined slot user code can set.
Expand Down
80 changes: 79 additions & 1 deletion test/js/node/worker_threads/worker_threads.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, it, setDefaultTimeout, test } from "bun:test";
import { bunEnv, bunExe, isDebug, tempDir, tmpdirSync } from "harness";
import { bunEnv, bunExe, isDebug, isWindows, tempDir, tmpdirSync } from "harness";
import { once } from "node:events";
import fs from "node:fs";
import { join, relative, resolve } from "node:path";
Expand Down Expand Up @@ -1925,6 +1925,84 @@ test("process.debugPort defaults to 9229 on the main thread", async () => {
expect(exitCode).toBe(0);
});

// The worker bootstrap rebinds process.stdout/stderr/stdin and removes/replaces a few
// process methods. It must do so without building the process object's lazy native
// properties: Object.defineProperty over one constructs the fd-backed stdio stream it is
// about to replace, and `delete` makes JSC reify every lazy property (env, versions,
// config, the stdio streams, ...). Constructing all of that was most of a worker's
// startup. Same invariant test/js/bun/util/BunObject.test.ts holds for the Bun object.
test("the worker bootstrap leaves the process object's lazy properties unbuilt", async () => {
const mainOnlyInternals = ["_debugProcess", "_debugEnd", "_startProfilerIdleNotifier", "_stopProfilerIdleNotifier"];
const worker = new Worker(
`const { hasNonReifiedStatic } = require("bun:internal-for-testing");
const lazy = hasNonReifiedStatic(process);
const shape = name => {
const { value, writable, enumerable, configurable } = Object.getOwnPropertyDescriptor(process, name);
return [value.constructor.name, writable, enumerable, configurable];
};
require("worker_threads").parentPort.postMessage({
lazy,
stdout: shape("stdout"),
stderr: shape("stderr"),
stdin: shape("stdin"),
presentInternals: ${JSON.stringify(mainOnlyInternals)}.filter(name => name in process),
});`,
{ eval: true },
);
const [result] = await once(worker, "message");
await worker.terminate();
expect(result).toEqual({
lazy: true,
// The same own data properties the main thread ends up with once it touches them.
stdout: ["Writable", true, true, true],
stderr: ["Writable", true, true, true],
stdin: ["Readable", true, true, true],
presentInternals: [],
});
// ...while the main thread still has node's no-op stubs.
expect(mainOnlyInternals.map(name => typeof process[name])).toEqual(["function", "function", "function", "function"]);
});

// The previous test cannot see the stdio streams themselves being built, because building
// a single lazy property does not count as reifying the table. Building the fd-backed
// stdout/stderr streams of a piped stdio dups fd 1 and 2, so an fd aliasing either of them
// that did not exist before the worker was created is the streams having been built and
// thrown away. A child process is used so that stdio is piped whatever the test runner's is.
test.skipIf(isWindows)(
"the worker bootstrap does not build (and dup the fds of) the stdio streams it replaces",
async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`const { Worker } = require("worker_threads");
// Self-contained: its source is also evaluated inside the worker.
const aliasesOfStdio = () => {
const fs = require("fs");
const id = fd => { try { const { dev, ino } = fs.fstatSync(fd); return dev + ":" + ino; } catch { return null; } };
const stdio = [id(1), id(2)], out = [];
for (let fd = 3; fd < 256; fd++) if (stdio.includes(id(fd))) out.push(fd);
return out;
};
const worker = new Worker(
"const wt = require('worker_threads'); wt.parentPort.postMessage((" + aliasesOfStdio + ")().filter(fd => !wt.workerData.includes(fd)));",
{ eval: true, workerData: aliasesOfStdio() },
);
worker.on("message", created => { console.log(JSON.stringify(created)); worker.terminate(); });`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ fdsAliasingStdioCreatedByTheWorker: JSON.parse(stdout), stderr, exitCode }).toEqual({
fdsAliasingStdioCreatedByTheWorker: [],
stderr: "",
exitCode: 0,
});
Comment thread
robobun marked this conversation as resolved.
Outdated
},
);

// Founding a SHARE_ENV tree replaces the founding thread's process.env object. If the
// replacement were orphaned, the founder's later writes would go nowhere. child_process
// enumerates the JS process.env (a var deleted from the map is invisible to the child),
Expand Down