diff --git a/src/jsc/bindings/BunObject.cpp b/src/jsc/bindings/BunObject.cpp index 8a022228eb4c..d05b1d744f1d 100644 --- a/src/jsc/bindings/BunObject.cpp +++ b/src/jsc/bindings/BunObject.cpp @@ -107,7 +107,13 @@ static JSValue constructWebViewObject(VM& vm, JSObject* bunObject); static JSValue constructEnvObject(VM& vm, JSObject* object) { - return uncheckedDowncast(object->globalObject())->processEnvObject(); + auto scope = DECLARE_THROW_SCOPE(vm); + // The env map build can throw (Windows builds it in JS). Return empty like + // Bun.$: a non-empty value would be reified while the lookup reports + // not-found, staling the in-progress walk. + JSObject* env = uncheckedDowncast(object->globalObject())->processEnvObject(); + RETURN_IF_EXCEPTION(scope, {}); + return env; } JSC::EncodedJSValue flattenArrayOfBuffersIntoArrayBufferOrUint8Array(JSGlobalObject* lexicalGlobalObject, JSValue arrayValue, size_t maxLength, bool asUint8Array) diff --git a/src/jsc/bindings/BunProcess.cpp b/src/jsc/bindings/BunProcess.cpp index ce39ea85cff8..3a67768cb47d 100644 --- a/src/jsc/bindings/BunProcess.cpp +++ b/src/jsc/bindings/BunProcess.cpp @@ -10,6 +10,7 @@ #include "bun_dependency_versions.h" #include #include +#include #include #include #include "JSCommonJSModule.h" @@ -1777,9 +1778,22 @@ static JSValue constructLoadEnvFile(VM& vm, JSObject* processObject) return JSC::JSFunction::create(vm, globalObject, processObjectInternalsLoadEnvFileCodeGenerator(vm), globalObject); } +// Builders run inside a property lookup whose walk caches the object's +// Structure*. Reporting synchronously runs the uncaught-exception machinery +// (arbitrary JS) right there, transitioning structures under the walk, so +// queue the report as a microtask instead. +static void reportLazyPropertyBuilderException(JSC::JSGlobalObject* globalObject, JSC::Exception* exception) +{ + auto& vm = JSC::getVM(globalObject); + auto* report = JSC::JSFunction::create(vm, globalObject, 1, String(), jsFunctionReportUncaughtException, JSC::ImplementationVisibility::Private); + JSC::QueuedTask task { nullptr, JSC::InternalMicrotask::BunInvokeJobWithArguments, 0, globalObject, report, JSC::JSValue(exception) }; + vm.queueMicrotask(WTF::move(task)); +} + // 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 (and reported through +// reportLazyPropertyBuilderException) so the worker's reifyAllStaticProperties +// (node:worker_threads preload) 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); @@ -1787,7 +1801,7 @@ static JSValue callLazyProcessBuilder(VM& vm, JSC::JSGlobalObject* 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); + reportLazyPropertyBuilderException(globalObject, exception); return jsUndefined(); } return result; @@ -2755,15 +2769,15 @@ 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. + // Lazy property builder: a throw is cleared, not propagated + // (see callLazyProcessBuilder). 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); + reportLazyPropertyBuilderException(globalObject, exception); return JSC::jsUndefined(); } variables->putDirect(vm, JSC::Identifier::fromString(vm, "v8_enable_i18n_support"_s), JSC::jsNumber(1), 0); @@ -2904,7 +2918,7 @@ static JSValue constructStdioWriteStream(JSC::JSGlobalObject* globalObject, JSC: 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); + reportLazyPropertyBuilderException(globalObject, exception); return jsUndefined(); } @@ -2965,7 +2979,7 @@ static JSValue constructStdin(VM& vm, JSObject* processObject) auto result = JSC::profiledCall(globalObject, ProfilingReason::API, getStdinStream, callData, globalObject, args); if (auto* exception = scope.exception()) { (void)scope.tryClearException(); - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); + reportLazyPropertyBuilderException(globalObject, exception); return jsUndefined(); } return result; @@ -3200,13 +3214,13 @@ 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. + // Lazy property builder: a throw is cleared, not propagated + // (see callLazyProcessBuilder). 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); + reportLazyPropertyBuilderException(globalObject, exception); return JSC::jsUndefined(); } return env; @@ -4253,13 +4267,13 @@ 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. + // Lazy property builder: a throw is cleared, not propagated + // (see callLazyProcessBuilder). 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); + reportLazyPropertyBuilderException(processObject->globalObject(), exception); return JSC::jsUndefined(); } return array; @@ -4388,8 +4402,8 @@ 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. + // Lazy property builder: a throw is cleared, not propagated + // (see callLazyProcessBuilder). auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); auto* globalObject = defaultGlobalObject(processObject->globalObject()); auto* bun = globalObject->bunObject(); @@ -4397,14 +4411,14 @@ static JSValue constructMainModuleProperty(VM& vm, JSObject* processObject) JSValue mainValue = bun->get(globalObject, builtinNames.mainPublicName()); if (auto* exception = scope.exception()) [[unlikely]] { (void)scope.tryClearException(); - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); + reportLazyPropertyBuilderException(globalObject, exception); return JSC::jsUndefined(); } auto* requireMap = globalObject->requireMap(); JSValue mainModule = requireMap->get(globalObject, mainValue); if (auto* exception = scope.exception()) [[unlikely]] { (void)scope.tryClearException(); - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); + reportLazyPropertyBuilderException(globalObject, exception); return JSC::jsUndefined(); } return mainModule; @@ -4428,13 +4442,13 @@ 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. + // Lazy property builder: a throw is cleared, not propagated + // (see callLazyProcessBuilder). 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); + reportLazyPropertyBuilderException(globalObject, exception); return JSC::jsUndefined(); } if (nextTickFunction && nextTickFunction.isObject()) { diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index 3022c09cafe3..7c27290f42c6 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -2543,7 +2543,15 @@ void GlobalObject::finishCreation(VM& vm) m_processEnvObject.initLater( [](const JSC::LazyProperty::Initializer& init) { - init.set(Bun::createEnvironmentVariablesMap(static_cast(init.owner)).getObject()); + auto scope = DECLARE_THROW_SCOPE(init.vm); + JSValue map = Bun::createEnvironmentVariablesMap(static_cast(init.owner)); + // The windowsEnv builtin can throw; a LazyProperty initializer + // must still set a value. Leave the exception pending. + if (scope.exception() || !map || !map.isObject()) [[unlikely]] { + init.set(JSC::constructEmptyObject(init.owner)); + return; + } + init.set(map.getObject()); }); m_processObject.initLater( diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index 3f4e0c0a2cb9..6e4787c4614a 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -2384,3 +2384,135 @@ it("no socket close handler runs after the 'exit' event", async () => { expect(stdout).toBe("exit\n"); expect(exitCode).toBe(0); }); + +describe.concurrent("lazy property builders", () => { + // Lazy process properties are reified in the middle of a property lookup. + // When the builder throws (user code clobbered a global it depends on), the + // error must not be reported from inside the lookup: the uncaughtException + // machinery runs arbitrary JS, which reifies more static properties and + // transitions object structures under the in-progress prototype-chain walk + // (stale-Structure assert in debug builds). The report is deferred to a + // microtask, so the handler observes it after the statement completes. + it("defers a builder failure report until after the property lookup", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `globalThis.Set = 123; + const order = []; + process.on("uncaughtException", e => order.push("uncaught:" + e.constructor.name)); + order.push("before"); + order.push("value:" + String(process.allowedNodeEnvironmentFlags)); + order.push("after"); + process.on("exit", () => console.log(order.join(",")));`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout).toBe("before,value:undefined,after,uncaught:TypeError\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); + + // On Windows process.env is built by a JS builtin, so a clobbered global can + // fail it while another lazy property (Bun.$ evaluates shell.ts, which reads + // process.env) is being reified. The failed builder must leave a usable + // process.env behind, and the deferred report keeps the uncaughtException + // handler (which reifies more Bun properties here) from running while the + // Bun.$ prototype-chain walk is still on the stack. + it("survives a clobbered global breaking the env builder mid-walk", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `Bun.inspect; + process.on("uncaughtException", e => { + Bun.gc; + Bun.SHA1; + console.log("uncaught:" + e.constructor.name); + }); + globalThis.Proxy = 123; + globalThis.Symbol = 123; + try { + Bun.$; + } catch (e) { + console.log("caught:" + e.constructor.name); + } + console.log("alive");`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const expected = isWindows ? "caught:TypeError\nalive\nuncaught:TypeError\n" : "caught:TypeError\nalive\n"; + expect(stdout).toBe(expected); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); + + // Bun.env shares the lazy env map. When its construction fails, the builder + // must return empty with the exception pending (like Bun.$) instead of + // handing reifyStaticProperty a value: putDirect would transition the Bun + // object's structure while the failed lookup is reported as not-found, and + // the in-progress walk would advance through a stale Structure*. + // Bun.inspect is pre-reified because the windowsEnv builtin reads + // Bun.inspect.custom while building the map, which transitions the Bun + // object mid-walk on its own; that separate bug is fixed in #37175. + it("Bun.env access with a broken env builder throws instead of crashing", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `Bun.inspect; + globalThis.Proxy = 123; + try { + console.log("typeof:" + typeof Bun.env); + } catch (e) { + console.log("caught:" + e.constructor.name); + } + console.log("recovered:" + typeof Bun.env); + console.log("alive");`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // The second access returns the cached fallback object instead of throwing + // again or crashing. + const expected = isWindows + ? "caught:TypeError\nrecovered:object\nalive\n" + : "typeof:object\nrecovered:object\nalive\n"; + expect(stdout).toBe(expected); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); + + // constructStdioWriteStream and constructStdin have their own clear+report + // blocks (they don't go through callLazyProcessBuilder), so pin their + // deferred ordering too. + it("defers stdio builder failure reports until after the lookup", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `globalThis.Symbol = 123; + const order = []; + process.on("uncaughtException", e => order.push("uncaught:" + e.constructor.name)); + order.push("stdout:" + String(process.stdout)); + order.push("stdin:" + String(process.stdin)); + process.on("exit", () => console.log(order.join(",")));`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout).toBe("stdout:undefined,stdin:undefined,uncaught:TypeError,uncaught:TypeError\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); +});