Skip to content

bundler: give the bundle thread a 16 MiB stack #38862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alii wants to merge 1 commit into from
Closed

bundler: give the bundle thread a 16 MiB stack #38862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions src/bundler/BundleThread.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ impl<C: CompletionStruct> BundleThread<C> {
let ptr = SendPtr(instance);
let thread = std::thread::Builder::new()
.name("Bundler".into())
// The linker's per-edge graph walks recurse deeply; Rust's 2 MiB
// default is not enough (Zig's std.Thread default was 16 MiB).
.stack_size(16 * 1024 * 1024)
.spawn(move || {
let ptr = ptr;
// SAFETY: caller guarantees `instance` is valid for 'static; `thread_main`
Expand Down
18 changes: 18 additions & 0 deletions test/bundler/bun-build-api.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,24 @@
expect(await build.outputs[0].text()).toEqualIgnoringWhitespace(".hello{color:#00f}.hi{color:red}\n");
});

test("css @import chain of 1000 files does not overflow the bundle thread stack", async () => {
const files: Record<string, string> = {};
const n = 1000;
for (let i = 0; i < n; i++) {
files[`c${i}.css`] = (i + 1 < n ? `@import "./c${i + 1}.css";\n` : "") + `.a${i} { color: red; }\n`;
}
const dir = tempDirWithFiles("bun-build-api-css-chain", files);

const build = await Bun.build({
entrypoints: [join(dir, "c0.css")],
write: false,

Check warning on line 56 in test/bundler/bun-build-api.test.ts

View check run for this annotation

Claude / Claude Code Review

write: false is not a Bun.build option 

`write` is not a `Bun.build` option — it's not in the `BuildConfig` interface (packages/bun-types/bun.d.ts) and `JSBundler.rs` never reads it, so this line is a silently-ignored no-op that will fail typecheck against the published types. Bun.build already returns in-memory outputs whenever `outdir` is omitted (see the neighboring "css works" test), so just drop the `write: false` line.

@claude claude Bot Aug 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 write is not a Bun.build option — it's not in the BuildConfig interface (packages/bun-types/bun.d.ts) and JSBundler.rs never reads it, so this line is a silently-ignored no-op that will fail typecheck against the published types. Bun.build already returns in-memory outputs whenever outdir is omitted (see the neighboring "css works" test), so just drop the write: false line.

Extended reasoning...

What the bug is

The new test at test/bundler/bun-build-api.test.ts:56 passes write: false to Bun.build(), but write is not a recognized BuildConfig option in Bun. This is an esbuild-ism — esbuild's build() API uses write: false to request in-memory output instead of writing to disk, and the author likely copied the convention from there (test/js/third_party/esbuild/esbuild-test.js uses it for esbuild.build calls).

The specific evidence

  1. Not in the type definition: The BuildConfig interface in packages/bun-types/bun.d.ts (lines ~2899-3292) enumerates every accepted property — entrypoints, outdir, target, format, naming, splitting, plugins, external, define, loader, sourcemap, minify, bytecode, compile, etc. There is no write field.

  2. Not read at runtime: src/runtime/api/JSBundler.rs, which parses the Bun.build config object, contains ~39 config.get* property reads. None of them reads a property named "write". The option is silently ignored — Bun.build does not perform excess-property validation on unknown keys.

  3. Explicitly documented as n/a: docs/bundler/esbuild.mdx:174 lists esbuild's write option as "n/a" for Bun.build, with the note that output is in-memory whenever outdir/outfile is unset.

Why the test still works

Bun.build already returns in-memory BuildArtifact blobs whenever no outdir is provided. The neighboring "css works" test at line 36 of the same file calls Bun.build with only entrypoints and minify, then reads build.outputs[0].text() — no write option needed. So the new test happens to work correctly, but only because the write: false is dead: omitting outdir is what actually produces the in-memory output.

Impact

  • Runtime: none — the option is ignored and the test passes.
  • Typecheck: this will produce a TypeScript excess-property error against the published .d.ts (Object literal may only specify known properties, and 'write' does not exist in type 'BuildConfig').
  • Maintenance: it's misleading — a future reader may assume write is a real Bun.build option and copy it elsewhere.

Per REVIEW.md: "Every line you add must be demonstrably live … parsed-but-never-read is a red flag".

Step-by-step proof

  1. Call site: Bun.build({ entrypoints: [join(dir, "c0.css")], write: false }).
  2. Bun.build config parsing lives in src/runtime/api/JSBundler.rs. Grepping every get_truthy / get_optional / property-name string in that file yields no "write" — the property is never read off the config object.
  3. Since no outdir is passed, JSBundler leaves options.outdir empty, and the completion path returns BuildArtifact blobs in memory (the same path the "css works" test on line 36 relies on).
  4. Therefore removing write: false produces byte-identical behavior; keeping it only adds a line that the runtime never reads and that TypeScript will flag against BuildConfig.

Fix

Delete line 56:

     const build = await Bun.build({
       entrypoints: [join(dir, "c0.css")],
-      write: false,
     });

No outdir already means in-memory output.

});

expect(build.success).toBe(true);
expect(build.outputs).toHaveLength(1);
expect(await build.outputs[0].text()).toContain(`.a${n - 1}`);
});

Check failure on line 62 in test/bundler/bun-build-api.test.ts

View check run for this annotation

Claude / Claude Code Review

Crash-regression test should spawn a subprocess 

This crash-regression test calls `Bun.build()` in-process, but the failure mode it guards against is `AddressSanitizer: stack-overflow` on the bundle thread — a process abort, not a catchable error. Per REVIEW.md ("Crash fixes need the crashing input as a spawned fixture"), and matching the "loader map with an empty-string key" test in this same file, wrap the `Bun.build()` call in a `Bun.spawn([bunExe(), "-e", ...])` subprocess and assert on `{stdout, stderr, exitCode}` so a regression fails ju
Comment on lines +46 to +62

@claude claude Bot Aug 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 This crash-regression test calls Bun.build() in-process, but the failure mode it guards against is AddressSanitizer: stack-overflow on the bundle thread — a process abort, not a catchable error. Per REVIEW.md ("Crash fixes need the crashing input as a spawned fixture"), and matching the "loader map with an empty-string key" test in this same file, wrap the Bun.build() call in a Bun.spawn([bunExe(), "-e", ...]) subprocess and assert on {stdout, stderr, exitCode} so a regression fails just this test instead of killing the whole runner.

Extended reasoning...

What the issue is

The new test "css @import chain of 1000 files does not overflow the bundle thread stack" (test/bundler/bun-build-api.test.ts:46-62) calls Bun.build() directly in the test-runner process. Per the PR description, on an unfixed build this exact input dies with AddressSanitizer: stack-overflow in find_imported_files_in_css_order::Visitor::visit — a hard process abort on the bundle thread. BundleThread.rs itself notes panic = "abort" on that thread, and a stack overflow is not a catchable Rust panic anyway; the whole process is gone.

REVIEW.md's Tests reviewers reject section states explicitly: "Crash fixes need the crashing input as a spawned fixture". This is listed among rules that have blocked merges.

Step-by-step: how it manifests

  1. The fix in this PR is one line: .stack_size(16 * 1024 * 1024) on the bundle thread builder.
  2. Suppose a future change drops that line (a refactor of spawn(), or someone "cleaning up" the builder chain), or per-frame stack usage in the CSS @import DFS grows — the PR description already notes a 3000-file chain still overflows 16 MiB on debug+ASAN, so the margin at 1000 files is finite.
  3. On CI, bun bd test test/bundler/bun-build-api.test.ts reaches this test and calls Bun.build() in-process.
  4. The bundle thread recurses through 1000 CSS @import edges, blows its stack, ASAN prints stack-overflow, and the process aborts.
  5. Every subsequent test in bun-build-api.test.ts — the leak tests, the "can be called thousands of times" test, the sourcemap tests — is never run. CI reports an opaque runner crash / signal exit for the whole file rather than one clean ✗ css @import chain of 1000 files… failure.

Why nothing prevents it

There is no isolation here. await Bun.build(...) runs on the process's singleton bundle thread; a stack overflow there is not surfaced as a rejected promise, it aborts the process. try/catch cannot help.

In-file precedent

This file already encodes the correct pattern. The "loader map with an empty-string key" test carries this exact comment:

"Run in a subprocess so a crash in the bundler thread surfaces as a test failure instead of taking down the test runner."

and spawns bunExe() -e for its Bun.build() call. The "can be called thousands of times in one process without crashing" test — also a crash regression — likewise runs in a spawned subprocess and asserts on {stdout, stderr, exitCode}.

How to fix

Move the Bun.build() into a spawned subprocess:

test("css @import chain of 1000 files does not overflow the bundle thread stack", async () => {
  const files: Record<string, string> = {};
  const n = 1000;
  for (let i = 0; i < n; i++) {
    files[`c${i}.css`] = (i + 1 < n ? `@import "./c${i + 1}.css";\n` : "") + `.a${i} { color: red; }\n`;
  }
  const dir = tempDirWithFiles("bun-build-api-css-chain", files);

  await using proc = Bun.spawn({
    cmd: [
      bunExe(),
      "-e",
      `
        const build = await Bun.build({
          entrypoints: [${JSON.stringify(join(dir, "c0.css"))}],
          write: false,
        });
        if (!build.success) throw new AggregateError(build.logs, "build failed");
        const text = await build.outputs[0].text();
        console.log(JSON.stringify({ success: build.success, outputs: build.outputs.length, hasLast: text.includes(".a${n - 1}") }));
      `,
    ],
    env: bunEnv,
    stdout: "pipe",
    stderr: "pipe",
  });
  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
  expect(stderr).toBe("");
  expect(JSON.parse(stdout.trim())).toEqual({ success: true, outputs: 1, hasLast: true });
  expect(exitCode).toBe(0);
});

A regression then shows up as exitCode !== 0 (SIGSEGV/SIGABRT) with the ASAN report on stderr — one failed test, runner intact.

On the refutation

One verifier flagged bug_002 as a duplicate of bug_001. That is correct and is why these were merged into a single finding; it does not bear on whether the finding itself is real.


test("bytecode works", async () => {
const dir = tempDirWithFiles("bun-build-api-bytecode", {
"package.json": `{}`,
Expand Down