Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 1 addition & 2 deletions src/jsc/modules/BunJSCModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -498,10 +498,9 @@
globalObject, scope,
createError(globalObject, "Sampling profiler was never started"_s)));

WTF::String jsonString = vm.samplingProfiler()->stackTracesAsJSON()->toJSONString();
JSC::EncodedJSValue result = JSC::JSValue::encode(JSONParse(globalObject, jsonString));
scope.releaseAssertNoException();
return result;
RELEASE_AND_RETURN(scope, JSC::JSValue::encode(JSONParse(globalObject, jsonString)));

Check warning on line 503 in src/jsc/modules/BunJSCModule.h

View check run for this annotation

Claude / Claude Code Review

Sibling site with identical JSONParse-then-releaseAssertNoException pattern left unfixed

One more sibling of this bug class remains: `JSC__JSGlobalObject__generateHeapSnapshot` at `src/jsc/bindings/bindings.cpp:3932-3935` still does `JSONParse(...)` → `scope.releaseAssertNoException()`, reachable via `console.takeHeapSnapshot()`. Per REVIEW.md's whole-class rule the same two-line fix (assert-before-parse, then `RELEASE_AND_RETURN` or plain return) belongs in this PR — heap snapshot JSON on a large heap is at least as OOM-prone as sampling-profiler JSON.
Comment thread
robobun marked this conversation as resolved.
}

JSC_DECLARE_HOST_FUNCTION(functionGetRandomSeed);
Expand Down
39 changes: 39 additions & 0 deletions test/js/bun/jsc/bun-jsc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -567,3 +567,42 @@ it("deserialize applies the same nesting depth limit to arrays as to objects", a
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout, exitCode }).toEqual({ stdout: "rejected\n65\n", exitCode: 0 });
});

it("samplingProfilerStackTraces returns parsed traces and survives BUN_JSC_validateExceptionChecks", async () => {
// samplingProfilerStackTraces JSON-parses the profiler's stack traces, and
// JSONParse can throw (OOM on a large profile), so the enclosing throw scope
// must release before returning instead of asserting no exception after the
// parse. With validateExceptionChecks enabled the process aborts on unchecked
// scopes; on release builds the option is a no-op and this just exercises the
// stack-trace path.
const script = `
const jsc = require("bun:jsc");
try {
jsc.samplingProfilerStackTraces();
console.log("no-throw");
} catch (e) {
console.log("threw", e.message);
}
jsc.startSamplingProfiler();
let j = 0;
for (let i = 0; i < 999999; i++) j += i % 7;
const traces = jsc.samplingProfilerStackTraces();
console.log("ok", typeof traces, Array.isArray(traces.traces));
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", script],
env: { ...bunEnv, BUN_JSC_validateExceptionChecks: "1" },
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
const uncheckedScopes = stderr
.split("\n")
.map(line => line.trim())
.filter(line => line.startsWith("This scope can throw") || line.startsWith("But the exception was unchecked"));
expect({ stdout, uncheckedScopes, exitCode }).toEqual({
stdout: "threw Sampling profiler was never started\nok object true\n",
uncheckedScopes: [],
exitCode: 0,
});
});