bundler: fix a segfault in bun build --sourcemap when the link step fails

src/bundler/BundleThread.rs

@@ -298,7 +298,7 @@ impl<C: CompletionStruct> BundleThread<C> {
         // Straight-line teardown: log copy
         // runs on both paths; `completeOnBundleThread` only on success (the error
         // path's `set_result(Err)` + complete happens in `thread_main`). The
-        // `deinitWithoutFreeingArena` + wait-group drain live inside `init_and_run`
+        // `deinit_without_freeing_arena` call lives inside `init_and_run`
         // (it owns `this`).
         let mut out_log = bun_ast::Log::init();
         // SAFETY: `transpiler.log` is the arena-allocated `*mut Log` set up by

src/bundler/LinkerContext.rs

@@ -111,7 +111,7 @@ pub struct LinkerContext<'a> {
     /// string buffer containing prefix for each unique keys
     pub(crate) unique_key_prefix: Box<[u8]>,
 
-    pub source_maps: SourceMapData,
+    pub(crate) source_maps: SourceMapData,
 
     /// This will eventually be used for reference-counting LinkerContext
     /// to know whether or not we can free it safely.
@@ -1516,10 +1516,10 @@ pub enum LinkerOptionsMode {
 
 #[derive(Default)]
 pub struct SourceMapData {
-    pub line_offset_wait_group: WaitGroup,
+    pub(crate) line_offset_wait_group: WaitGroup,
     pub(crate) line_offset_tasks: Box<[SourceMapDataTask]>,
 
-    pub quoted_contents_wait_group: WaitGroup,
+    pub(crate) quoted_contents_wait_group: WaitGroup,
     pub(crate) quoted_contents_tasks: Box<[SourceMapDataTask]>,
 }
 

src/bundler/ThreadPool.rs

@@ -436,6 +436,10 @@ impl ThreadPool {
         // SAFETY: `worker` is freshly heap-allocated and exclusive on this
         // thread until published via the map (already inserted above, but no
         // other thread looks it up under a different `id`).
+        // `deinit_without_freeing_arena` reads every entry, so it must not run
+        // while a task is in here. It waits for the source map tasks itself. Its
+        // callers `wait_for_parse` first, except when `enqueue_entry_points_*`
+        // returns an error, which only a failed allocation makes it do.
         unsafe {
             worker.write(Worker {
                 // Placeholder — overwritten by `init()` immediately below.

src/bundler/bundle_v2.rs

@@ -5418,8 +5418,14 @@ pub mod bv2_impl {
         }
 
         pub fn deinit_without_freeing_arena(&mut self) {
+            // A build that stops between `compute_data_for_source_map` and the waits in
+            // `generate_chunks_in_parallel` gets here with those tasks still on the pool,
+            // creating `Worker`s and reading `graph`.
+            self.linker.source_maps.line_offset_wait_group.wait();
+            self.linker.source_maps.quoted_contents_wait_group.wait();
+
             {
-                // We do this first to make it harder for any dangling pointers to data to be used in there.
+                // We do this before the rest to make it harder for any dangling pointers to data to be used in there.
                 let on_parse_finalizers = core::mem::take(&mut self.finalizers);
                 for finalizer in &on_parse_finalizers {
                     finalizer.call();

src/runtime/api/js_bundle_completion_task.rs

@@ -1340,23 +1340,13 @@ impl CompletionStruct for JSBundleCompletionTask {
             .map(|b| &**b)
             .collect();
 
-        let run = bv2.run_from_js_in_new_thread(&entry_points);
-
-        // The AST-allocator pop lives in `generate_in_new_thread`; the
-        // source-map wait-group waits run only on the error path.
-        match run {
-            Ok(build) => {
-                self.set_result(BundleV2Result::Value(build));
-                bv2.deinit_without_freeing_arena();
-                Ok(())
-            }
-            Err(err) => {
-                bv2.linker.source_maps.line_offset_wait_group.wait();
-                bv2.linker.source_maps.quoted_contents_wait_group.wait();
-                bv2.deinit_without_freeing_arena();
-                Err(err)
-            }
-        }
+        let run = bv2
+            .run_from_js_in_new_thread(&entry_points)
+            .map(|build| self.set_result(BundleV2Result::Value(build)));
+
+        // The AST-allocator pop lives in `generate_in_new_thread`.
+        bv2.deinit_without_freeing_arena();
+        run
     }
 }
 

test/bundler/esbuild/default.test.ts

@@ -1,6 +1,6 @@
 import assert from "assert";
-import { describe, expect } from "bun:test";
-import { osSlashes } from "harness";
+import { describe, expect, test } from "bun:test";
+import { bunEnv, bunExe, osSlashes, tempDir } from "harness";
 import path from "path";
 import { dedent, ESBUILD_PATH, itBundled } from "../expectBundled";
 
@@ -3603,6 +3603,71 @@ describe.concurrent("bundler", () => {
       ],
     },
   });
+  const forbiddenRequireWithNamedImport = {
+    "a.js": `
+      import { b } from "./b.js";
+      console.log(b, require("./b.js"));
+    `,
+    "b.js": `export const b = await 0;`,
+  };
+  test("default/TopLevelAwaitForbiddenRequireSourceMapCLI", async () => {
+    using dir = tempDir("tla-forbidden-require-sourcemap", forbiddenRequireWithNamedImport);
+    const build = async () => {
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), "build", "--sourcemap=inline", "a.js"],
+        env: bunEnv,
+        cwd: String(dir),
+        stdout: "ignore",
+        stderr: "pipe",
+      });
+      const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
+      return { error: stderr.match(/^error: .*$/m)?.[0], exitCode };
+    };
+    // The link error races the source map tasks: one build alone does not always show it,
+    // and many builds at once show it less often.
+    const builds: Awaited<ReturnType<typeof build>>[] = [];
+    for (let round = 0; round < 10; round++) {
+      builds.push(...(await Promise.all([build(), build(), build(), build()])));
+    }
+    expect(builds).toEqual(
+      Array(40).fill({
+        error:
+          'error: This require call is not allowed because the transitive dependency "b.js" contains a top-level await',
+        exitCode: 1,
+      }),
+    );
+  });
+  test("default/TopLevelAwaitForbiddenRequireSourceMapAPI", async () => {
+    using dir = tempDir("tla-forbidden-require-sourcemap-api", {
+      ...forbiddenRequireWithNamedImport,
+      "build.js": `
+        const messages = [];
+        for (let i = 0; i < 40; i++) {
+          const { logs } = await Bun.build({ entrypoints: ["./a.js"], sourcemap: "inline", throw: false });
+          messages.push(logs[0].message);
+        }
+        console.log(JSON.stringify(messages));
+      `,
+    });
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "build.js"],
+      env: bunEnv,
+      cwd: String(dir),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect({ stdout, stderr }).toEqual({
+      stdout:
+        JSON.stringify(
+          Array(40).fill(
+            'This require call is not allowed because the transitive dependency "b.js" contains a top-level await',
+          ),
+        ) + "\n",
+      stderr: "",
+    });
+    expect(exitCode).toBe(0);
+  });
   itBundled("default/TopLevelAwaitAllowedImportWithoutSplitting", {
     files: {
       "/entry.js": /* js */ `