Skip to content
Closed
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
20 changes: 11 additions & 9 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2558,23 +2558,25 @@ pub fn process_fetch_log(
}

_ => {
// Spec caps at 256 (`var errors_stack: [256]JSValue`). PERF(port):
// was inline switch — Zig stack-allocated; we heap-allocate the
// exact `len` since `JSValue` is a thin u64 and 256 * 8 B = 2 KiB
// is fine either way, but `Vec` avoids the uninit-array dance.
// Spec caps at 256 (`var errors_stack: [256]JSValue`). The array
// must stay on the stack: the freshly created message cells are
// only reachable through it until the AggregateError adopts them,
// so they have to be visible to JSC's conservative stack scan — a
// heap `Vec<JSValue>` is invisible to the GC and the cells get
// collected if any allocation below triggers a collection.
let len = log.msgs.len().min(256);
let mut errors: alloc::vec::Vec<JSValue> = alloc::vec::Vec::with_capacity(len);
for msg in log.msgs.drain(..len) {
let v = match msg.metadata {
let mut errors_stack = [JSValue::UNDEFINED; 256];
for (slot, msg) in errors_stack.iter_mut().zip(log.msgs.drain(..len)) {
*slot = match msg.metadata {
bun_ast::Metadata::Build => take(BuildMessage::create(global_this, msg)),
bun_ast::Metadata::Resolve(_) => take(ResolveMessage::create(
global_this,
&msg,
referrer_utf8.slice(),
)),
};
errors.push(v);
}
let errors = &errors_stack[..len];

// C++ `Zig::toString` does `createWithoutCopying`, so the buffer
// must outlive the AggregateError. Mark it global so JSC adopts it
Expand All @@ -2588,7 +2590,7 @@ pub fn process_fetch_log(
message.mark_global();
*ret = ErrorableResolvedSource::err(
err,
take(global_this.create_aggregate_error(&errors, &message)),
take(global_this.create_aggregate_error(errors, &message)),
);
}
}
Expand Down
11 changes: 7 additions & 4 deletions src/jsc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2143,12 +2143,15 @@ impl LogJsc for bun_ast::Log {
0 => Ok(JSValue::UNDEFINED),
1 => msg_to_js(&msgs[0], global),
_ => {
let mut errors_stack: Vec<JSValue> = Vec::with_capacity(count);
for msg in &msgs[0..count] {
errors_stack.push(msg_to_js(msg, global)?);
// Stack array (not a heap Vec): the created cells must be
// visible to the conservative stack scan until the
// AggregateError adopts them.
let mut errors_stack = [JSValue::UNDEFINED; 256];
for (slot, msg) in errors_stack.iter_mut().zip(&msgs[0..count]) {
*slot = msg_to_js(msg, global)?;
}
let out = bun_core::ZigString::init(message.as_bytes());
global.create_aggregate_error(&errors_stack, &out)
global.create_aggregate_error(&errors_stack[..count], &out)
}
}
}
Expand Down
56 changes: 55 additions & 1 deletion test/js/bun/resolve/build-error.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { tempDir } from "harness";
import { bunEnv, bunExe, isWindows, tempDir } from "harness";
import { join } from "node:path";

test("BuildError is modifiable", async () => {
Expand All @@ -19,6 +19,60 @@ test("BuildError is modifiable", async () => {
expect(error!.message).not.toBe(message);
});

test("aggregated module load errors survive a GC during error creation", async () => {
// When a module fails to transpile with more than one log message, the
// module loader wraps every BuildMessage/ResolveMessage in a JS cell and
// aggregates them into an AggregateError. Those freshly created cells used
// to be held only in a native heap Vec, which the conservative GC cannot
// see, so a collection during aggregation freed them and printing the
// error afterwards read freed memory (heap-use-after-free under ASAN).
using dir = tempDir("build-error-aggregate-gc", {
// Every line is a recoverable parse error so the transpiler log contains
// many messages and the aggregate path is taken.
"many-errors.js": Array.from({ length: 60 }, (_, i) => `v${i}: 1 2 3`).join("\n") + "\n",
// Same failure, but on a Worker thread (how the fuzzer originally hit it).
"worker-parent.js": `
const w = new Worker(new URL("./many-errors.js", import.meta.url).href);
w.addEventListener("error", () => {});
`,
});

// collectContinuously is extremely slow on Windows; forceRAMSize still
// pressures the GC enough there (same approach as require-esm-gc-roots).
const gcEnv = isWindows
? { ...bunEnv, BUN_JSC_forceRAMSize: String(64 * 1024 * 1024) }
: { ...bunEnv, BUN_JSC_collectContinuously: "1" };

const runs = [
(async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "many-errors.js"],
env: gcEnv,
cwd: String(dir),
stdout: "ignore",
stderr: "pipe",
});
const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
expect(stderr).not.toContain("AddressSanitizer");
expect(stderr).toContain("error:");
expect(exitCode).toBe(1);
})(),
(async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "worker-parent.js"],
env: gcEnv,
cwd: String(dir),
stdout: "ignore",
stderr: "pipe",
});
const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
expect(stderr).not.toContain("AddressSanitizer");
expect(exitCode).toBe(0);
})(),
];
await Promise.all(runs);
}, 90_000);

test("BuildMessage finalize frees with the same allocator it was created with", async () => {
// BuildMessage.create() clones the message with the passed allocator
// but finalize() was freeing it with bun.default_allocator and never
Expand Down
Loading