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
61 changes: 40 additions & 21 deletions src/jsc/bindings/BunProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "bun_dependency_versions.h"
#include <JavaScriptCore/InternalFieldTuple.h>
#include <JavaScriptCore/JSMicrotask.h>
#include <JavaScriptCore/MicrotaskQueue.h>
#include <JavaScriptCore/ObjectConstructor.h>
#include <JavaScriptCore/NumberPrototype.h>
#include "JSCommonJSModule.h"
Expand Down Expand Up @@ -1777,17 +1778,35 @@ static JSValue constructLoadEnvFile(VM& vm, JSObject* processObject)
return JSC::JSFunction::create(vm, globalObject, processObjectInternalsLoadEnvFileCodeGenerator(vm), globalObject);
}

JSC_DECLARE_HOST_FUNCTION(jsFunctionReportUncaughtException);
Comment thread
robobun marked this conversation as resolved.
Outdated

// Lazy PropertyCallback builders run while setUpStaticFunctionSlot /
// reifyAllStaticProperties is reifying the property, i.e. in the middle of a
// property lookup whose walk (JSObject::getPropertySlot) holds the object's
// Structure*. Running the uncaught-exception machinery there re-enters JS,
// which reifies further static properties and transitions structures under
// the walk, tripping the stale-Structure assert in Structure::storedPrototype.
// Queue the report as a microtask so it runs after the lookup completes.
Comment thread
robobun marked this conversation as resolved.
Outdated
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.
Comment thread
robobun marked this conversation as resolved.
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);
reportLazyPropertyBuilderException(globalObject, exception);
return jsUndefined();
}
return result;
Expand Down Expand Up @@ -2755,15 +2774,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).
Comment thread
robobun marked this conversation as resolved.
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);
Expand Down Expand Up @@ -2904,7 +2923,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();
}

Expand Down Expand Up @@ -2965,7 +2984,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;
Expand Down Expand Up @@ -3200,13 +3219,13 @@ static JSValue constructRevision(VM& vm, JSObject* processObject)
static JSValue constructEnv(VM& vm, JSObject* processObject)
{
auto* globalObject = uncheckedDowncast<Zig::GlobalObject>(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).
Comment thread
robobun marked this conversation as resolved.
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;
Expand Down Expand Up @@ -4253,13 +4272,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).
Comment thread
robobun marked this conversation as resolved.
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;
Expand Down Expand Up @@ -4388,23 +4407,23 @@ 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).
Comment thread
robobun marked this conversation as resolved.
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);
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;
Expand All @@ -4428,13 +4447,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).
Comment thread
robobun marked this conversation as resolved.
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()) {
Expand Down
12 changes: 11 additions & 1 deletion src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2543,7 +2543,17 @@

m_processEnvObject.initLater(
[](const JSC::LazyProperty<JSC::JSGlobalObject, JSC::JSObject>::Initializer& init) {
init.set(Bun::createEnvironmentVariablesMap(static_cast<Zig::GlobalObject*>(init.owner)).getObject());
auto scope = DECLARE_THROW_SCOPE(init.vm);
JSValue map = Bun::createEnvironmentVariablesMap(static_cast<Zig::GlobalObject*>(init.owner));
// On Windows the map is built by the windowsEnv builtin, which can
// throw when user code clobbered a global it depends on. A
// LazyProperty initializer must still set a value; leave the
// exception pending for the caller.
Comment thread
robobun marked this conversation as resolved.
Outdated
if (scope.exception() || !map || !map.isObject()) [[unlikely]] {
init.set(JSC::constructEmptyObject(init.owner));
return;

Check warning on line 2554 in src/jsc/bindings/ZigGlobalObject.cpp

View check run for this annotation

Claude / Claude Code Review

Bun.env lazy builder is a same-class site not covered by the fix

The `Bun.env` lazy builder (`constructEnvObject` at src/jsc/bindings/BunObject.cpp:108-111) is a sibling PropertyCallback that also calls `processEnvObject()` but has no exception scope, so on Windows with a clobbered `Proxy` and `Bun.env` accessed first it now returns into `reifyStaticProperty` with the exception still pending. Not a regression (was a hard crash on `LazyProperty::set(nullptr)` before), but per REVIEW.md's "fix the whole class" rule it should get the same clear + `reportLazyProp
Comment thread
claude[bot] marked this conversation as resolved.
}
init.set(map.getObject());
});

m_processObject.initLater(
Expand Down
69 changes: 69 additions & 0 deletions test/js/node/process/process.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2384,3 +2384,72 @@ 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);
});
});