Skip to content
Merged
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
10 changes: 10 additions & 0 deletions src/jsc/bindings/FormatStackTraceForJS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "JavaScriptCore/ArgList.h"
#include "JavaScriptCore/CallData.h"
#include "JavaScriptCore/DeferTermination.h"
#include "JavaScriptCore/TopExceptionScope.h"
#include "JavaScriptCore/Error.h"
#include "JavaScriptCore/ErrorInstance.h"
Expand Down Expand Up @@ -639,6 +640,11 @@

JSC::JSValue computeErrorInfoWrapperToJSValue(JSC::VM& vm, Vector<StackFrame>& stackTrace, unsigned int& line_in, unsigned int& column_in, String& sourceURL, JSObject* errorInstance, void* bunErrorData)
{
// ErrorInstance::getOwnPropertySlot doesn't check for exceptions after materializeErrorInfoIfNeeded,
// so a TerminationException raised in here trips getOwnPropertyDescriptor's EXCEPTION_ASSERT.
// https://github.com/oven-sh/bun/issues/34095
JSC::DeferTerminationForAWhile deferTermination(vm);

Check failure on line 646 in src/jsc/bindings/FormatStackTraceForJS.cpp

View check run for this annotation

Claude / Claude Code Review

DeferTerminationForAWhile makes user prepareStackTrace/.message getter un-interruptible by worker.terminate()

The `DeferTerminationForAWhile` scope here spans the `profiledCall` into the user's `Error.prepareStackTrace` (and the user `.message` getter), so a Worker that hangs inside `prepareStackTrace` can no longer be interrupted by `worker.terminate()` — the `NeedTermination` trap is masked at every loop back-edge for the duration. The cited precedents (`LazyProperty::callFunc`, #33966) only defer around bounded C++ initializers, never unbounded user JS; consider narrowing the scope to exclude the `pr
Comment thread
robobun marked this conversation as resolved.
Outdated

OrdinalNumber line = OrdinalNumber::fromOneBasedInt(line_in);
OrdinalNumber column = OrdinalNumber::fromOneBasedInt(column_in);

Expand All @@ -647,6 +653,10 @@
line_in = line.oneBasedInt();
column_in = column.oneBasedInt();

// materializeErrorInfoIfNeeded putDirect()s this unconditionally; an empty JSValue
// in property storage crashes the next read.
if (!result) [[unlikely]]
return jsUndefined();
return result;
}

Expand Down
24 changes: 24 additions & 0 deletions test/js/node/v8/capture-stack-trace.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1003,3 +1003,27 @@ test("printing an error whose message getter calls Error.captureStackTrace on it

expect({ lastLine: stdout.trimEnd().split("\n").pop(), exitCode }).toEqual({ lastLine: "after", exitCode: 0 });
});

// https://github.com/oven-sh/bun/issues/34095
test("lazy error-info materialization does not store an empty stack value when the compute hook throws", async () => {
const src = `
Error.prepareStackTrace = (e, s) => "custom-stack";
const e = new Error("x");
Object.defineProperty(e, "message", { get() { throw new TypeError("msg-boom"); } });
let first = "no-throw";
try { void e.stack; } catch (err) { first = err.message; }
console.log(JSON.stringify({ first, secondType: typeof e.stack }));
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", src],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout: stdout.trim(), signalCode: proc.signalCode }).toEqual({
stdout: JSON.stringify({ first: "msg-boom", secondType: "undefined" }),
signalCode: null,
});
expect(exitCode).toBe(0);
});
Loading