diff --git a/src/jsc/bindings/BunProcess.cpp b/src/jsc/bindings/BunProcess.cpp index 3de0456f309..c22acb25ab9 100644 --- a/src/jsc/bindings/BunProcess.cpp +++ b/src/jsc/bindings/BunProcess.cpp @@ -1801,19 +1801,13 @@ static JSValue constructLoadEnvFile(VM& vm, JSObject* processObject) return JSC::JSFunction::create(vm, globalObject, processObjectInternalsLoadEnvFileCodeGenerator(vm), globalObject); } -// 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. +// TopExceptionScope, not ThrowScope: JSC checks a failed builder with vm.exceptionForInspection(). static JSValue callLazyProcessBuilder(VM& vm, JSC::JSGlobalObject* globalObject, JSC::FunctionExecutable* (*generator)(VM&), const JSC::ArgList& args) { auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); auto* function = JSC::JSFunction::create(vm, globalObject, generator(vm), globalObject); auto result = JSC::profiledCall(globalObject, ProfilingReason::API, function, JSC::getCallData(function), globalObject->globalThis(), args); - if (auto* exception = scope.exception()) [[unlikely]] { - (void)scope.tryClearException(); - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); - return jsUndefined(); - } + RETURN_IF_EXCEPTION(scope, {}); return result; } @@ -2773,17 +2767,11 @@ static JSValue constructProcessConfigObject(VM& vm, JSObject* processObject) // v8_use_snapshot: 1 // } // } - // Lazy property builder: exceptions must not propagate into - // reifyStaticProperty, which performs no exception check. auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); JSC::JSObject* config = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 2); JSC::JSObject* variables = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 2); JSC::JSArray* shareableBuiltins = JSC::constructEmptyArray(globalObject, nullptr); - if (auto* exception = scope.exception()) [[unlikely]] { - (void)scope.tryClearException(); - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); - return JSC::jsUndefined(); - } + RETURN_IF_EXCEPTION(scope, {}); variables->putDirect(vm, JSC::Identifier::fromString(vm, "v8_enable_i18n_support"_s), JSC::jsNumber(1), 0); variables->putDirect(vm, JSC::Identifier::fromString(vm, "enable_lto"_s), JSC::jsBoolean(false), 0); // Node 26's common.gypi evaluates enable_thin_lto/lto_jobs conditions; gyp @@ -2920,11 +2908,7 @@ static JSValue constructStdioWriteStream(JSC::JSGlobalObject* globalObject, JSC: JSC::CallData callData = JSC::getCallData(getStdioWriteStream); auto result = JSC::profiledCall(globalObject, ProfilingReason::API, getStdioWriteStream, callData, globalObject->globalThis(), args); - if (auto* exception = scope.exception()) { - (void)scope.tryClearException(); - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); - return jsUndefined(); - } + RETURN_IF_EXCEPTION(scope, {}); ASSERT_WITH_MESSAGE(JSC::isJSArray(result), "Expected an array from getStdioWriteStream"); JSC::JSArray* resultObject = uncheckedDowncast(result); @@ -2947,10 +2931,14 @@ static JSValue constructStdioWriteStream(JSC::JSGlobalObject* globalObject, JSC: forceSync = true; #endif if (forceSync) { - Bun__ForceFileSinkToBeSynchronousForProcessObjectStdio(globalObject, JSValue::encode(resultObject->getIndex(globalObject, 1))); + JSValue sink = resultObject->getIndex(globalObject, 1); + RETURN_IF_EXCEPTION(scope, {}); + Bun__ForceFileSinkToBeSynchronousForProcessObjectStdio(globalObject, JSValue::encode(sink)); } - return resultObject->getIndex(globalObject, 0); + JSValue stream = resultObject->getIndex(globalObject, 0); + RETURN_IF_EXCEPTION(scope, {}); + return stream; } static JSValue constructStdout(VM& vm, JSObject* processObject) @@ -2981,11 +2969,7 @@ static JSValue constructStdin(VM& vm, JSObject* processObject) JSC::CallData callData = JSC::getCallData(getStdinStream); auto result = JSC::profiledCall(globalObject, ProfilingReason::API, getStdinStream, callData, globalObject, args); - if (auto* exception = scope.exception()) { - (void)scope.tryClearException(); - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); - return jsUndefined(); - } + RETURN_IF_EXCEPTION(scope, {}); return result; } @@ -3218,15 +3202,9 @@ static JSValue constructRevision(VM& vm, JSObject* processObject) static JSValue constructEnv(VM& vm, JSObject* processObject) { auto* globalObject = uncheckedDowncast(processObject->globalObject()); - // Lazy property builder: exceptions must not propagate into - // reifyStaticProperty, which performs no exception check. auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); JSValue env = globalObject->processEnvObject(); - if (auto* exception = scope.exception()) [[unlikely]] { - (void)scope.tryClearException(); - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); - return JSC::jsUndefined(); - } + RETURN_IF_EXCEPTION(scope, {}); return env; } @@ -4272,15 +4250,9 @@ JSC_DEFINE_HOST_FUNCTION(Process_stubFunctionReturningArray, (JSGlobalObject * g static JSValue Process_stubEmptyArray(VM& vm, JSObject* processObject) { - // Lazy property builder: exceptions must not propagate into - // reifyStaticProperty, which performs no exception check. auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); JSC::JSArray* array = JSC::constructEmptyArray(processObject->globalObject(), nullptr); - if (auto* exception = scope.exception()) [[unlikely]] { - (void)scope.tryClearException(); - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(processObject->globalObject(), exception); - return JSC::jsUndefined(); - } + RETURN_IF_EXCEPTION(scope, {}); return array; } @@ -4407,25 +4379,15 @@ extern "C" void Bun__Process__queueNextTick2(GlobalObject* globalObject, Encoded // return require.cache.get(Bun.main) static JSValue constructMainModuleProperty(VM& vm, JSObject* processObject) { - // Lazy property builder: exceptions must not propagate into - // reifyStaticProperty, which performs no exception check. auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); auto* globalObject = defaultGlobalObject(processObject->globalObject()); auto* bun = globalObject->bunObject(); auto& builtinNames = Bun::builtinNames(vm); JSValue mainValue = bun->get(globalObject, builtinNames.mainPublicName()); - if (auto* exception = scope.exception()) [[unlikely]] { - (void)scope.tryClearException(); - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); - return JSC::jsUndefined(); - } + RETURN_IF_EXCEPTION(scope, {}); auto* requireMap = globalObject->requireMap(); JSValue mainModule = requireMap->get(globalObject, mainValue); - if (auto* exception = scope.exception()) [[unlikely]] { - (void)scope.tryClearException(); - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); - return JSC::jsUndefined(); - } + RETURN_IF_EXCEPTION(scope, {}); return mainModule; } @@ -4447,15 +4409,9 @@ JSValue Process::constructNextTickFn(JSC::VM& vm, Zig::GlobalObject* globalObjec args.append(JSC::JSFunction::create(vm, globalObject, 1, String(), jsFunctionDrainMicrotaskQueue, ImplementationVisibility::Private)); args.append(JSC::JSFunction::create(vm, globalObject, 1, String(), jsFunctionReportUncaughtException, ImplementationVisibility::Private)); - // Lazy property builder: exceptions must not propagate into - // reifyStaticProperty, which performs no exception check. auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); JSValue nextTickFunction = JSC::profiledCall(globalObject, ProfilingReason::API, initializer, JSC::getCallData(initializer), globalObject->globalThis(), args); - if (auto* exception = scope.exception()) [[unlikely]] { - (void)scope.tryClearException(); - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); - return JSC::jsUndefined(); - } + RETURN_IF_EXCEPTION(scope, {}); if (nextTickFunction && nextTickFunction.isObject()) { this->m_nextTickFunction.set(vm, this, nextTickFunction.getObject()); } diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index bab0d2aae4b..e8d28b8b4bb 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -2930,12 +2930,13 @@ JSC_DEFINE_CUSTOM_GETTER(getConsoleConstructor, (JSGlobalObject * globalObject, JSC_DEFINE_CUSTOM_GETTER(getConsoleStdout, (JSGlobalObject * globalObject, EncodedJSValue thisValue, PropertyName property)) { auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); auto console = JSValue::decode(thisValue).getObject(); auto global = uncheckedDowncast(globalObject); // instead of calling the constructor builtin, go through the process.stdout getter to ensure it's only created once. auto stdoutValue = global->processObject()->get(globalObject, Identifier::fromString(vm, "stdout"_s)); - if (!stdoutValue) return {}; + RETURN_IF_EXCEPTION(scope, {}); console->putDirect(vm, property, stdoutValue, PropertyAttribute::DontEnum | 0); return JSValue::encode(stdoutValue); @@ -2945,12 +2946,13 @@ JSC_DEFINE_CUSTOM_GETTER(getConsoleStdout, (JSGlobalObject * globalObject, Encod JSC_DEFINE_CUSTOM_GETTER(getConsoleStderr, (JSGlobalObject * globalObject, EncodedJSValue thisValue, PropertyName property)) { auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); auto console = JSValue::decode(thisValue).getObject(); auto global = uncheckedDowncast(globalObject); - // instead of calling the constructor builtin, go through the process.stdout getter to ensure it's only created once. + // instead of calling the constructor builtin, go through the process.stderr getter to ensure it's only created once. auto stderrValue = global->processObject()->get(globalObject, Identifier::fromString(vm, "stderr"_s)); - if (!stderrValue) return {}; + RETURN_IF_EXCEPTION(scope, {}); console->putDirect(vm, property, stderrValue, PropertyAttribute::DontEnum | 0); return JSValue::encode(stderrValue); diff --git a/test/js/node/process/process-stdio-stack-overflow-fixture.js b/test/js/node/process/process-stdio-stack-overflow-fixture.js index e0de6778ed2..1114139285c 100644 --- a/test/js/node/process/process-stdio-stack-overflow-fixture.js +++ b/test/js/node/process/process-stdio-stack-overflow-fixture.js @@ -1,9 +1,9 @@ // Exercise lazy process.stdin / process.stdout / process.stderr creation near -// the stack limit. The property callbacks that create these streams report the -// exception if stream construction fails with a stack overflow; that reporting -// path must not observe the exception as still pending when it re-enters JS -// to invoke the uncaughtException listener (Interpreter::executeCallImpl -// asserts no pending exception on entry). +// the stack limit. Stream construction fails with a stack overflow there, which +// the property read throws (caught below). Nothing on that path may re-enter JS +// while the exception is pending (Interpreter::executeCallImpl asserts no +// pending exception on entry); an uncaughtException listener is what such a +// re-entry would call. process.on("uncaughtException", () => {}); const which = process.argv[2]; function F(a, ...b) { diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index 3a2acd1a975..e19db4ce4b9 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -1,7 +1,7 @@ import { spawnSync, which } from "bun"; import { describe, expect, it } from "bun:test"; import { familySync } from "detect-libc"; -import { bunEnv, bunExe, isMacOS, isWindows, tempDir, tmpdirSync } from "harness"; +import { bunEnv, bunExe, isASAN, isDebug, isMacOS, isWindows, tempDir, tmpdirSync } from "harness"; import { basename, join, resolve } from "path"; const process_sleep = resolve(import.meta.dir, "process-sleep.js"); @@ -2546,3 +2546,272 @@ it("no socket close handler runs after the 'exit' event", async () => { expect(stdout).toBe("exit\n"); expect(exitCode).toBe(0); }); + +// process.stdout/stderr/stdin/nextTick/finalization/allowedNodeEnvironmentFlags (and a few +// more) are lazy: the first read runs a native builder that calls into JS. When that JS throws, +// the read must throw and the property must stay unbuilt so that the next read builds it. The +// builders used to swallow the error, report it as an uncaught exception (exit code 1 with no +// handler) and store undefined in the property for the rest of the process. +describe.concurrent("lazy process properties whose builder throws", () => { + async function run(code) { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", code], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + it("a builder that runs out of stack throws, and the next read builds the property", async () => { + // Only the deepest frame reaches the catch block. There is less than one JS frame of stack + // left at that point, so every builder that has to call into JS fails with the RangeError. + const { stdout, stderr, exitCode } = await run(` + const names = ["stdout", "stderr", "stdin", "nextTick", "finalization", "allowedNodeEnvironmentFlags"]; + const atLimit = {}; + function recurse() { + try { + recurse(); + } catch { + for (const name of names) { + try { + atLimit[name] = typeof process[name]; + } catch (e) { + atLimit[name] = "threw " + e.constructor.name; + } + } + for (const name of ["_stdout", "_stderr"]) { + try { + atLimit["console" + name] = typeof console[name]; + } catch (e) { + atLimit["console" + name] = "threw " + e.constructor.name; + } + } + } + } + recurse(); + const afterwards = {}; + for (const name of names) afterwards[name] = typeof process[name]; + afterwards.console_stdout = console._stdout === process.stdout; + afterwards.console_stderr = console._stderr === process.stderr; + process.nextTick(() => process.stdout.write(JSON.stringify({ atLimit, afterwards }) + "\\n")); + `); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ + atLimit: { + stdout: "threw RangeError", + stderr: "threw RangeError", + stdin: "threw RangeError", + nextTick: "threw RangeError", + finalization: "threw RangeError", + allowedNodeEnvironmentFlags: "threw RangeError", + console_stdout: "threw RangeError", + console_stderr: "threw RangeError", + }, + afterwards: { + stdout: "object", + stderr: "object", + stdin: "object", + nextTick: "function", + finalization: "object", + allowedNodeEnvironmentFlags: "object", + console_stdout: true, + console_stderr: true, + }, + }); + expect(exitCode).toBe(0); + }); + + // Windows: process.env is built by a JS builtin and has a JS custom inspect function. Building + // it or inspecting it at the stack limit still aborts the process (initializers of JSC + // LazyPropertys, which cannot fail), so Bun.inspect(process) cannot be run there. + it.skipIf(isWindows)("inspecting process while out of stack leaves every property buildable", async () => { + // Bun.inspect(process) reads every property, so it runs all of the builders in one go, and at + // the stack limit every one of them that calls into JS fails. They must only be missing from + // that one inspection. + const listUndefinedProperties = ` + const undefinedProperties = Object.getOwnPropertyNames(process).filter(name => process[name] === undefined); + process.nextTick(() => process.stdout.write(JSON.stringify(undefinedProperties) + "\\n")); + `; + const [outOfStack, baseline] = await Promise.all([ + run(` + function recurse() { + try { + recurse(); + } catch { + try { + Bun.inspect(process, { depth: 0 }); + } catch {} + } + } + recurse(); + ${listUndefinedProperties} + `), + run(listUndefinedProperties), + ]); + expect(baseline.stderr).toBe(""); + // e.g. ["channel", "disconnect", "exitCode", "mainModule", "send"] + expect(JSON.parse(baseline.stdout)).not.toContain("stdout"); + expect(baseline.exitCode).toBe(0); + expect(outOfStack).toEqual(baseline); + }); + + it("a builder that throws is retried once the cause is gone, without reporting an uncaught exception", async () => { + // buildAllowedNodeEnvironmentFlags constructs a Set. + const { stdout, stderr, exitCode } = await run(` + process.on("uncaughtException", e => console.log("uncaughtException: " + e.constructor.name)); + const RealSet = Set; + globalThis.Set = undefined; + let brokenRead; + try { + brokenRead = typeof process.allowedNodeEnvironmentFlags; + } catch (e) { + brokenRead = "threw " + e.constructor.name; + } + globalThis.Set = RealSet; + const flags = process.allowedNodeEnvironmentFlags; + console.log(JSON.stringify({ + brokenRead, + sameObjectOnEveryRead: flags === process.allowedNodeEnvironmentFlags, + has: [flags.has("--no-warnings"), flags.has("--not-a-flag")], + })); + `); + expect(stderr).toBe(""); + expect(stdout).toBe( + JSON.stringify({ brokenRead: "threw TypeError", sameObjectOnEveryRead: true, has: [true, false] }) + "\n", + ); + expect(exitCode).toBe(0); + }); + + it("a builder failure during bulk reification throws from the operation that triggered it", async () => { + // Spreading process (like deleting one of its properties) makes JSC build all of its remaining + // lazy properties first. The ones after the failing builder stay lazy and are built by the + // next such operation. The stdio builders load modules, which may need Set themselves, so + // they are built up front to make allowedNodeEnvironmentFlags the one builder that fails. + const { stdout, stderr, exitCode } = await run(` + process.stdout; process.stderr; process.stdin; process.nextTick; + const RealSet = Set; + globalThis.Set = undefined; + let brokenSpread; + try { + brokenSpread = "copied " + typeof { ...process }.allowedNodeEnvironmentFlags; + } catch (e) { + brokenSpread = "threw " + e.constructor.name; + } + globalThis.Set = RealSet; + console.log(JSON.stringify({ + brokenSpread, + spreadAfterwards: typeof { ...process }.allowedNodeEnvironmentFlags, + flags: process.allowedNodeEnvironmentFlags.has("--no-warnings"), + finalization: typeof process.finalization.register, + })); + `); + expect(stderr).toBe(""); + expect(stdout).toBe( + JSON.stringify({ + brokenSpread: "threw TypeError", + spreadAfterwards: "object", + flags: true, + finalization: "function", + }) + "\n", + ); + expect(exitCode).toBe(0); + }); + + it("a failed builder only hides its own property from Bun.inspect", async () => { + // A failed lookup leaves its exception pending. Bun.inspect's property walk has to clear it + // before looking up the next property, or the lazy properties declared after the failed one + // (loadEnvFile, finalization, arch, ... up to the first non-lazy one) are reported as missing + // too. As above, allowedNodeEnvironmentFlags is made the one builder that fails. process.env + // is replaced because on Windows it has a custom inspect function, whose machinery needs + // modules that do not load without Set either. + const { stdout, stderr, exitCode } = await run(` + process.stdout; process.stderr; process.stdin; process.nextTick; + Object.defineProperty(process, "env", { value: {}, writable: true, configurable: true, enumerable: true }); + const keysOf = inspected => [...inspected.matchAll(/^ ([A-Za-z_$][\\w$]*):/gm)].map(match => match[1]); + const RealSet = Set; + globalThis.Set = undefined; + const whileBroken = keysOf(Bun.inspect(process, { depth: 0 })); + globalThis.Set = RealSet; + const intact = keysOf(Bun.inspect(process, { depth: 0 })); + console.log(JSON.stringify({ + hiddenWhileBroken: intact.filter(key => !whileBroken.includes(key)), + extraWhileBroken: whileBroken.filter(key => !intact.includes(key)), + })); + `); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ hiddenWhileBroken: ["allowedNodeEnvironmentFlags"], extraWhileBroken: [] }); + expect(exitCode).toBe(0); + }); + + it("internal nextTick users see the builder failure and work once it can be built", async () => { + // process.emitWarning queues through the native Process::queueNextTick, which reads + // process.nextTick and so runs its builder on the first use. It used to fail for good after + // one failed build ("Failed to call nextTick" from every later warning, Worker, ...). + const { stdout, stderr, exitCode } = await run(` + process.removeAllListeners("warning"); + let atLimit; + process.on("warning", warning => { + console.log(JSON.stringify({ atLimit, warning: warning.message, nextTick: typeof process.nextTick })); + }); + function recurse() { + try { + recurse(); + } catch { + try { + process.emitWarning("at the stack limit"); + atLimit = "returned"; + } catch (e) { + atLimit = "threw " + e.constructor.name; + } + } + } + recurse(); + process.emitWarning("after unwinding"); + `); + expect(stderr).toBe(""); + expect(stdout).toBe( + JSON.stringify({ atLimit: "threw RangeError", warning: "after unwinding", nextTick: "function" }) + "\n", + ); + expect(exitCode).toBe(0); + }); + + // Reading the property again every few frames while unwinding: the first reads fail, and the + // one that has enough stack builds it. The stdio builders load the stream modules, which read + // process.nextTick and so add a property to process before the build can still run out of stack; + // JSC's getPropertySlot then continues with the structure it read before the build, which is an + // assertion failure on builds with assertions (harmless otherwise: the prototype is the same). + // Until the engine reloads the structure there, those three only run on release builds. + const rebuiltWhileUnwinding = [ + ["nextTick", "function"], + ["allowedNodeEnvironmentFlags", "object"], + ]; + if (!isDebug && !isASAN) rebuiltWhileUnwinding.push(["stdout", "object"], ["stderr", "object"], ["stdin", "object"]); + it.each(rebuiltWhileUnwinding)("process.%s is built by a later read while unwinding", async (name, type) => { + // process.env is built first: on Windows its builder calls into JS as well. + const { stdout, stderr, exitCode } = await run(` + process.env; + let unwound = 0; + let failed = 0; + let built; + function recurse() { + try { + recurse(); + } catch {} + if (built !== undefined || unwound++ % 64 !== 0) return; + try { + built = typeof process[${JSON.stringify(name)}]; + } catch (e) { + if (!(e instanceof RangeError)) throw e; + failed++; + } + } + recurse(); + console.log(JSON.stringify({ built, failedFirst: failed > 0, stillBuilt: typeof process[${JSON.stringify(name)}] })); + `); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ built: type, failedFirst: true, stillBuilt: type }); + expect(exitCode).toBe(0); + }); +});