diff --git a/src/js/node/worker_threads.ts b/src/js/node/worker_threads.ts index f9d221865a82..c5e0b4fa445c 100644 --- a/src/js/node/worker_threads.ts +++ b/src/js/node/worker_threads.ts @@ -460,33 +460,20 @@ 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; + // Assignment, unlike Object.defineProperty, does not build the lazy fd-backed streams first. + 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"); @@ -864,6 +851,7 @@ function fakeParentPort() { if (!isMainThread && _isNodeWorker) { applyWorkerProcessOverrides(); } +// Never `delete` from process here (it builds every lazy property); worker VMs lack the main-only internals. function applyWorkerProcessOverrides() { const proc: any = process; // node defaults debugPort to 9229 in workers (still settable). Per-object property: @@ -871,12 +859,6 @@ function applyWorkerProcessOverrides() { 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) { diff --git a/src/jsc/bindings/BunProcess.cpp b/src/jsc/bindings/BunProcess.cpp index dd5b7bda072f..9f30b26509f7 100644 --- a/src/jsc/bindings/BunProcess.cpp +++ b/src/jsc/bindings/BunProcess.cpp @@ -1801,8 +1801,7 @@ static JSValue constructLoadEnvFile(VM& vm, JSObject* processObject) } // Lazy PropertyCallback builders that enter JS. reifyAllStaticProperties wraps these in -// DeferTerminationForAWhile; a non-termination throw is cleared+reported so the worker's -// reifyAllStaticProperties (node:worker_threads preload) doesn't leave a pending exception. +// DeferTerminationForAWhile; a non-termination throw is cleared+reported so a reify-all (`delete process.x`, `Object.entries(process)`) doesn't leave a pending exception. static JSValue callLazyProcessBuilder(VM& vm, JSC::JSGlobalObject* globalObject, JSC::FunctionExecutable* (*generator)(VM&), const JSC::ArgList& args) { auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); @@ -4840,8 +4839,6 @@ 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 @@ -4849,8 +4846,6 @@ extern "C" void Process__emitErrorEvent(Zig::GlobalObject* global, EncodedJSValu _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 @@ -4970,6 +4965,14 @@ 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); + + // Main-thread-only in node; not table entries because deleting one in a worker would reify every lazy property. + 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); + } + // 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. diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index cc7acb16c723..1a5b211b41c3 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -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"; @@ -1925,6 +1925,98 @@ 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]); + // stdout is the JSON list of fds the worker created that alias fd 1 or 2. + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ stdout: "[]", stderr: "", exitCode: 0 }); + }, +); + +// A user-visible consequence of the reify-all: process.mainModule is built from the require +// map when it is first read. Built during the bootstrap, before the entry module exists, it +// was permanently undefined in every worker; built on first read it is the entry module, as +// in node (and as it was before the bootstrap reified it). +test("process.mainModule in a worker is the worker's entry module, like node", async () => { + using dir = tempDir("worker-main-module", { + "entry.js": `require("worker_threads").parentPort.postMessage({ + mainModuleIsEntry: process.mainModule === module, + requireMainIsEntry: require.main === module, + });`, + }); + const worker = new Worker(join(String(dir), "entry.js")); + const [result] = await once(worker, "message"); + await worker.terminate(); + expect(result).toEqual({ mainModuleIsEntry: true, requireMainIsEntry: true }); +}); + // 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),