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
29 changes: 13 additions & 16 deletions src/runtime/bake/production.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,11 @@ 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.
// Only the `Err` return below reaches this; the other exits are process exits.
// 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 +211,25 @@ 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: `build_with_vm` has returned, so its reborrows through `pt.vm` are
// dead; the VM is live 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(())

// Success must exit through the VM too: 'exit' handlers, process.exitCode, VM teardown.
vm.on_exit();
vm.global_exit()
}

/// Ported inline from `bun.bun_js.failWithBuildError` to avoid the
Expand Down
124 changes: 122 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,124 @@ 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.
describe.concurrent("exits through the build VM", () => {
// With BUN_DESTRUCT_VM_ON_EXIT set, global_exit tears the VM down before the
// process exits, so the exit codes asserted below also show that a build
// VM's teardown completes on every platform, not only on the ASAN lane that
// looks at what it frees.
const env = { ...bunEnv, BUN_DESTRUCT_VM_ON_EXIT: "1" };

const onExitConfig = `
process.on("exit", code => console.log("exit event: " + code));
export default { app: { framework: "react" } };
`;

// Each case is a full react build on a debug (and ASAN) binary, and the
// LSan case also symbolizes its report, which the default timeout does not
// cover; WAIT_MULTIPLIER is how the bake harness budgets production builds.
const timeout = 30_000 * WAIT_MULTIPLIER;

test(
"a 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(env)
.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);
},
timeout,
);

test(
"a 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(env)
.quiet()
.throws(false);

expect(stdout.toString()).toBe("done\nexit event: 0\n");
expect(exitCode).toBe(0);
},
timeout,
);

// The natives behind these objects are released by their wrappers'
// finalizers, so a build that exits without destroying its VM leaves all of
// them for LeakSanitizer to report. The page creates them while it renders:
// bun-framework-react's own prerender creates the first three kinds the same
// way (that is how the bug was found), and leaksan.supp would hide the same
// objects if they were created while the module is evaluated. The report
// cannot be required to be empty yet because the build's transpilers still
// leak (#38233), which is also what keeps this file in
// test/no-validate-leaksan.txt.
test.skipIf(!isASAN)(
"a rendered build frees the natives its JS objects own",
async () => {
const dir = await tempDirWithBakeDeps("bake-production-exit-teardown", {
"app.ts": `export default { app: { framework: "react" } };`,
"pages/index.tsx": `
export default function IndexPage() {
globalThis.keepUntilExit = [
new TextDecoder(),
new Blob(["prerender"]),
setImmediate(() => {}),
new Bun.CryptoHasher("sha256"),
];
return <div>Hello World</div>;
}
`,
});

const { stdout, stderr } = await Bun.$`${bunExe()} build --app ./app.ts`
.cwd(dir)
.env({
...env,
ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"),
LSAN_OPTIONS: [
bunEnv.LSAN_OPTIONS,
`print_suppressions=0:suppressions=${path.join(import.meta.dirname, "../../leaksan.supp")}`,
]
.filter(Boolean)
.join(":"),
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
.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", "CryptoHasher"].filter(type =>
stderr.toString().includes(type),
);
expect(leaked).toEqual([]);
},
timeout,
);
});
});
3 changes: 3 additions & 0 deletions test/no-validate-leaksan.txt
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,10 @@ test/napi/node-napi-tests/test/js-native-api/6_object_wrap/do.test.ts
# LSAN per-spawn adds no coverage and slows the file significantly.
test/napi/uv_stub.test.ts

# `bun build --app` still leaks its transpilers (#38233) and, with "use client"
# components, a ServerComponentParseTask (#38004); remove once both have landed.
test/bake/dev/production.test.ts

test/js/third_party/pg-gateway/pglite.test.ts
test/js/bun/test/parallel/test-integration-rspack.ts

Expand Down