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: 10 additions & 10 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2574,23 +2574,23 @@ 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.
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 {
// Spec caps at 256 (`var errors_stack: [256]JSValue`). On-stack
// array: conservative GC stack scan keeps these JSValues alive
// (see PORTING.md §JSC) — creating later messages can trigger a
// GC, and a heap `Vec` is invisible to the scan.
let mut errors_stack: [JSValue; 256] = [JSValue::default(); 256];
let len = log.msgs.len().min(errors_stack.len());
for (i, msg) in log.msgs.drain(..len).enumerate() {
errors_stack[i] = 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 @@ -2604,7 +2604,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
12 changes: 8 additions & 4 deletions src/jsc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2143,12 +2143,16 @@ 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)?);
// On-stack array: conservative GC stack scan keeps these
// JSValues alive (see PORTING.md §JSC) — creating later
// messages can trigger a GC, and a heap `Vec` is invisible
// to the scan.
let mut errors_stack: [JSValue; 256] = [JSValue::default(); 256];
for (i, msg) in msgs[0..count].iter().enumerate() {
errors_stack[i] = 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
32 changes: 31 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, tempDir } from "harness";
import { join } from "node:path";

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

test("importing a module with many build errors does not crash while reporting them", async () => {
// The AggregateError for a failed module build is assembled from one
// BuildMessage wrapper per log message. Those wrappers used to be collected
// only in a heap Vec (invisible to the conservative GC scan), so a GC during
// the loop could finalize earlier wrappers and free their native
// BuildMessage before the AggregateError was created, causing a
// use-after-free when the unhandled rejection was printed.
using dir = tempDir("build-error-many", {
// 40 declarations + 80 redeclarations -> ~80 build errors in one module
"bad.js": Array.from({ length: 120 }, (_, i) => `const x${i % 40} = 1;`).join("\n"),
"index.js": `import("./bad.js");`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "index.js"],
cwd: String(dir),
env: { ...bunEnv, BUN_JSC_collectContinuously: "1" },
stdout: "pipe",
stderr: "pipe",
});

const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);

// Every error in the AggregateError should have been printed.
expect(stderr).toContain('"x0" has already been declared');
expect(stderr).toContain('"x39" has already been declared');
// Unhandled rejection -> clean exit with code 1, not a crash.
expect(exitCode).toBe(1);
});

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