Skip to content
Closed
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
38 changes: 22 additions & 16 deletions src/runtime/bake/production.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,13 @@ pub fn build_command(ctx: Context) -> crate::Result<()> {
// SAFETY: `init_bake` returns a freshly-allocated VM owned by this thread;
// unique access for the rest of this function.
let vm = unsafe { &mut *vm_ptr };
// defer vm.deinit() — handled by `vm.destroy()` on the unwind path below.
// Runs only on the `Err` return below (a bundler or I/O failure). Every
// other way out of this function exits the process, and the normal one,
// `global_exit()` at the end, tears the VM down itself.
Comment thread
robobun marked this conversation as resolved.
Outdated
// Note: pass `vm_ptr` by value into the guard so the drop closure does
// not borrow the local (`defer!` would capture `&vm_ptr`, which under
// edition-2024 disjoint-capture rules collides with the `&mut *vm_ptr`
// re-borrows on the JSError path).
// re-borrows below).
let _vm_guard = scopeguard::guard(vm_ptr, |p| {
// SAFETY: p is the unique live VM on this thread; its loop is alive, so
// queued work is released here rather than by a thread teardown.
Expand Down Expand Up @@ -211,28 +213,32 @@ pub fn build_command(ctx: Context) -> crate::Result<()> {
// LIFO order — under the API lock, before the VM is destroyed.
let mut pt = PerThread::placeholder(vm_ptr);

match build_with_vm(ctx, &cwd, &mut pt) {
let result = build_with_vm(ctx, &cwd, &mut pt);
// SAFETY: the reborrows `build_with_vm` made through `pt.vm` died when it
// returned, so this frame has exclusive access again; the VM stays
// allocated until `global_exit` (or `_vm_guard`) destroys it.
let vm = unsafe { &mut *vm_ptr };
match result {
Ok(()) => {}
Err(crate::Error::JSError) => {
// SAFETY: vm.global is live for VM lifetime.
let global = unsafe { &*(*vm_ptr).global };
let err_value = global.take_exception(jsc::JsError::Thrown);
// SAFETY: see above.
unsafe {
(*vm_ptr)
.print_error_like_object_to_console(err_value.to_error().unwrap_or(err_value))
};
// SAFETY: see above.
let vm = unsafe { &mut *vm_ptr };
let err_value = vm.global().take_exception(jsc::JsError::Thrown);
vm.print_error_like_object_to_console(err_value.to_error().unwrap_or(err_value));
if vm.exit_handler.exit_code == 0 {
vm.exit_handler.exit_code = 1;
}
vm.on_exit();
vm.global_exit();
}
Err(e) => return Err(e),
}
Ok(())

// A rendered build exits the same way a failed one does: `on_exit` runs the
// `process.on("exit")` handlers the config and prerender registered, and
// `global_exit` exits with `process.exitCode`, tearing the VM down first
// under BUN_DESTRUCT_VM_ON_EXIT. Returning `Ok` instead would free only the
// Rust side of the VM (`_vm_guard`); the JSC heap would never be destroyed,
// so the natives still owned by the prerender's JS objects (timers, blobs,
// decoders, ...) would never be freed.
Comment thread
robobun marked this conversation as resolved.
Outdated
vm.on_exit();
vm.global_exit()
}

/// Ported inline from `bun.bun_js.failWithBuildError` to avoid the
Expand Down
99 changes: 97 additions & 2 deletions test/bake/dev/production.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { describe, expect, test } from "bun:test";
import { existsSync } from "fs";
import { bunEnv, bunExe } from "harness";
import { bunEnv, bunExe, isASAN } from "harness";
import path from "path";
import { tempDirWithBakeDeps } from "../bake-harness";
import { tempDirWithBakeDeps, WAIT_MULTIPLIER } from "../bake-harness";

const normalizePath = (path: string) => (process.platform === "win32" ? path.replaceAll("\\", "/") : path);
const platformPath = (path: string) => (process.platform === "win32" ? path.replaceAll("/", "\\") : path);
Expand Down Expand Up @@ -594,4 +594,99 @@ export default function IndexPage() {
// Verify NO JavaScript imports are included in the HTML
expect(htmlContent).not.toContain('<script type="module"');
});

// A build that succeeds has to leave through the build VM's exit sequence
// (`on_exit` + `global_exit`), like a build that throws already does: that is
// what runs the 'exit' handlers, applies process.exitCode and, under
// BUN_DESTRUCT_VM_ON_EXIT, destroys the JSC heap. Before, it returned and
// exited the process directly, skipping all three.
const onExitConfig = `
process.on("exit", code => console.log("exit event: " + code));
export default { app: { framework: "react" } };
`;

test.concurrent(
"rendered build runs 'exit' handlers and exits with process.exitCode",
async () => {
const dir = await tempDirWithBakeDeps("bake-production-exit-rendered", {
"app.ts": onExitConfig,
"pages/index.tsx": `
process.exitCode = 3;
export default function IndexPage() {
return <div>Hello World</div>;
}
`,
});

const { stdout, exitCode } = await Bun.$`${bunExe()} build --app ./app.ts`
.cwd(dir)
.env(bunEnv)
.quiet()
.throws(false);

expect(await Bun.file(path.join(dir, "dist", "index.html")).text()).toContain("Hello World");
expect(stdout.toString()).toBe("done\nexit event: 3\n");
expect(exitCode).toBe(3);
},
30_000 * WAIT_MULTIPLIER,
);

test.concurrent(
"build with nothing to render runs 'exit' handlers",
async () => {
const dir = await tempDirWithBakeDeps("bake-production-exit-no-routes", {
"app.ts": onExitConfig,
});

const { stdout, exitCode } = await Bun.$`${bunExe()} build --app ./app.ts`
.cwd(dir)
.env(bunEnv)
.quiet()
.throws(false);

expect(stdout.toString()).toBe("done\nexit event: 0\n");
expect(exitCode).toBe(0);
},
30_000 * WAIT_MULTIPLIER,
);

// The natives behind these wrappers are freed by the wrappers' finalizers, so
// they stay allocated until the VM is destroyed; LSan sees them as leaked when
// the build exits without destroying it. This checks the report's contents
// rather than requiring it to be empty because the build's own transpilers
// are still leaked (separate fix), which keeps this file in
// test/no-validate-leaksan.txt for now.
test.concurrent.skipIf(!isASAN)(
"rendered build destroys its VM under BUN_DESTRUCT_VM_ON_EXIT",
async () => {
const dir = await tempDirWithBakeDeps("bake-production-exit-teardown", {
"app.ts": `export default { app: { framework: "react" } };`,
"pages/index.tsx": `
globalThis.keepUntilExit = [new TextDecoder(), new Blob(["prerender"]), setImmediate(() => {})];
export default function IndexPage() {
return <div>Hello World</div>;
}
`,
});

const { stdout, stderr } = await Bun.$`${bunExe()} build --app ./app.ts`
.cwd(dir)
.env({
...bunEnv,
BUN_DESTRUCT_VM_ON_EXIT: "1",
ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"),
LSAN_OPTIONS: `print_suppressions=0:suppressions=${path.join(import.meta.dirname, "../../leaksan.supp")}`,
})
.quiet()
.throws(false);

expect(await Bun.file(path.join(dir, "dist", "index.html")).text()).toContain("Hello World");
expect(stdout.toString()).toBe("done\n");
const leaked = ["TextDecoder", "Blob", "ImmediateObject"].filter(type => stderr.toString().includes(type));
expect(leaked).toEqual([]);
},
// LSan symbolizes every reported stack through llvm-symbolizer, which is
// slow against a debug binary.
60_000 * WAIT_MULTIPLIER,
);
});