diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index c5951dd147a..e7592e89a2e 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -2558,14 +2558,16 @@ 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` 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 = 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, @@ -2573,8 +2575,8 @@ pub fn process_fetch_log( 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 @@ -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)), ); } } diff --git a/src/jsc/lib.rs b/src/jsc/lib.rs index a23f5a105ec..a8b5ea7997a 100644 --- a/src/jsc/lib.rs +++ b/src/jsc/lib.rs @@ -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 = 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) } } } diff --git a/test/js/bun/resolve/build-error.test.ts b/test/js/bun/resolve/build-error.test.ts index a623aa505e9..740f0f00df8 100644 --- a/test/js/bun/resolve/build-error.test.ts +++ b/test/js/bun/resolve/build-error.test.ts @@ -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 () => { @@ -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