js_printer: resolve string ropes into a local copy instead of the shared AST node

src/ast/e.rs

@@ -1685,6 +1685,23 @@ pub struct EString {
 // Also exported as `String`; `EString` avoids colliding with bun_core::String.
 pub use EString as String;
 
+/// [`EString::flattened`] result: the node itself, or an owned copy when a rope was flattened.
+pub enum Flattened<'a> {
+    Borrowed(&'a EString),
+    Owned(EString),
+}
+
+impl core::ops::Deref for Flattened<'_> {
+    type Target = EString;
+    #[inline]
+    fn deref(&self) -> &EString {
+        match self {
+            Flattened::Borrowed(s) => s,
+            Flattened::Owned(s) => s,
+        }
+    }
+}
+
 impl Default for EString {
     fn default() -> Self {
         Self {
@@ -1875,19 +1892,36 @@ impl EString {
         true
     }
 
+    /// Flatten in place. Parser only; shared-AST readers use [`Self::flattened`].
     pub fn resolve_rope_if_needed(&mut self, bump: &Bump) {
         if self.next.is_none() || !self.is_utf8() {
             return;
         }
+        self.data = Str::new(self.flatten_rope(bump));
+        self.next = None;
+    }
+
+    /// `self` if not a rope, else a copy flattened into `bump`. Never writes to `self`.
+    pub fn flattened(&self, bump: &Bump) -> Flattened<'_> {
+        if self.next.is_none() || !self.is_utf8() {
+            return Flattened::Borrowed(self);
+        }
+        let mut copy = self.shallow_clone();
+        copy.data = Str::new(self.flatten_rope(bump));
+        copy.next = None;
+        Flattened::Owned(copy)
+    }
+
+    /// The rope's bytes, concatenated into a fresh `bump` slice.
+    fn flatten_rope<'b>(&self, bump: &'b Bump) -> &'b [u8] {
         let mut bytes = bun_alloc::ArenaVec::<u8>::with_capacity_in(self.rope_len as usize, bump);
         bytes.extend_from_slice(&self.data);
         let mut str_ = self.next;
         while let Some(part) = str_ {
             bytes.extend_from_slice(&part.get().data);
             str_ = part.get().next;
         }
-        self.data = Str::new(bytes.into_bump_slice());
-        self.next = None;
+        bytes.into_bump_slice()
     }
 
     /// Return UTF-8 bytes, transcoding if UTF-16.

src/bundler/linker_context/generateCodeForFileInChunkJS.rs

@@ -424,8 +424,10 @@ pub fn generate_code_for_file_in_chunk_js<'r, 'src>(
                     {
                         continue;
                     }
-                    let name = match &mut prop.key.as_mut().unwrap().data {
-                        ExprData::EString(s) => s.slice(temp_arena),
+                    let name: &[u8] = match &prop.key.as_ref().unwrap().data {
+                        ExprData::EString(s) => {
+                            bun_core::handle_oom(s.flattened(temp_arena).string(temp_arena))
+                        }
                         _ => unreachable!(),
                     };
                     if name == b"default" || name == b"__esModule" || !js_lexer::is_identifier(name)

src/bundler/linker_context/generateCodeForLazyExport.rs

@@ -417,11 +417,8 @@ pub(crate) fn generate_code_for_lazy_export(
             if let ExprData::EObject(e_object) = &expr.data {
                 for property in e_object.properties.slice() {
                     let _: &G::Property = property;
-                    // `Expr`/`ExprData`/`StoreRef<_>` are `Copy`. Copy `key` out so
-                    // `key_str: StoreRef<E::EString>` is a mutable local — `slice()` resolves
-                    // the rope in-place via `DerefMut` into the arena slot.
                     let Some(key) = property.key else { continue };
-                    let ExprData::EString(mut key_str) = key.data else {
+                    let ExprData::EString(key_str) = key.data else {
                         continue;
                     };
                     let Some(value) = property.value else {
@@ -436,7 +433,7 @@ pub(crate) fn generate_code_for_lazy_export(
                     // across the `&mut self` call to `generate_named_export_in_file` below.
                     let alloc: &bun_alloc::Arena =
                         unsafe { bun_ptr::detach_lifetime_ref::<bun_alloc::Arena>(this.arena()) };
-                    let name = key_str.slice(alloc);
+                    let name: &[u8] = bun_core::handle_oom(key_str.flattened(alloc).string(alloc));
 
                     // TODO: support non-identifier names
                     if !js_lexer::is_identifier(name) {

src/js_printer/lib.rs

@@ -3449,8 +3449,8 @@ pub(crate) mod __gated_printer {
                     if e.optional_chain.is_none() {
                         flags.insert(ExprFlag::HasNonOptionalChainParent);
 
-                        if let Some(mut str) = e.index.data.as_e_string() {
-                            str.resolve_rope_if_needed(self.bump);
+                        if let Some(str) = e.index.data.as_e_string() {
+                            let str = str.flattened(self.bump);
                             if str.is_utf8() {
                                 if let Some(value) =
                                     self.try_to_get_imported_enum_value(e.target, str.slice8())
@@ -3762,8 +3762,7 @@ pub(crate) mod __gated_printer {
                         return;
                     }
 
-                    let mut e = *e;
-                    e.resolve_rope_if_needed(self.bump);
+                    let e = e.flattened(self.bump);
                     self.add_source_mapping(expr.loc);
 
                     // If this was originally a template literal, print it as one as long as we're not minifying
@@ -3926,12 +3925,12 @@ pub(crate) mod __gated_printer {
                     }
 
                     self.print(b"`");
-                    match &mut e.head {
+                    match &e.head {
                         E::TemplateContents::Raw(raw) => self.print_raw_template_literal(raw),
                         E::TemplateContents::Cooked(cooked) => {
                             if cooked.is_present() {
-                                cooked.resolve_rope_if_needed(self.bump);
-                                self.print_string_characters_e_string(cooked, b'`');
+                                let cooked = cooked.flattened(self.bump);
+                                self.print_string_characters_e_string(&cooked, b'`');
                             }
                         }
                     }
@@ -3944,12 +3943,7 @@ pub(crate) mod __gated_printer {
                             E::TemplateContents::Raw(raw) => self.print_raw_template_literal(raw),
                             E::TemplateContents::Cooked(cooked) => {
                                 if cooked.is_present() {
-                                    // `parts` is `*mut [TemplatePart]` but accessed `&[T]`
-                                    // here. We resolve a local copy of the
-                                    // EString header (the rope chain is StoreRef-linked and Copy) and
-                                    // prints from that — the arena node stays roped.
-                                    let mut local = E::EString { ..*cooked };
-                                    local.resolve_rope_if_needed(self.bump);
+                                    let local = cooked.flattened(self.bump);
                                     self.print_string_characters_e_string(&local, b'`');
                                 }
                             }
@@ -4652,10 +4646,9 @@ pub(crate) mod __gated_printer {
                     self.print_symbol(priv_.ref_);
                 }
                 ExprData::EString(key_str) => {
-                    let mut key_str = *key_str;
+                    let key_str = key_str.flattened(self.bump);
                     self.add_source_mapping(key.loc);
                     if key_str.is_utf8() {
-                        key_str.resolve_rope_if_needed(self.bump);
                         self.print_space_before_identifier();
                         let mut allow_shorthand = true;
                         if !IS_JSON && lexer::is_identifier(key_str.slice8()) {
@@ -4908,8 +4901,7 @@ pub(crate) mod __gated_printer {
 
                                 match &property.key.data {
                                     ExprData::EString(str) => {
-                                        let mut str = *str;
-                                        str.resolve_rope_if_needed(self.bump);
+                                        let str = str.flattened(self.bump);
                                         self.add_source_mapping(property.key.loc);
 
                                         if str.is_utf8() {
@@ -4960,7 +4952,9 @@ pub(crate) mod __gated_printer {
                                                 ) {
                                                     if Self::MAY_HAVE_MODULE_INFO && tlm.is_export {
                                                         // reshaped for borrowck — bump access first.
-                                                        let str8 = str.slice(self.bump);
+                                                        let str8 = bun_core::handle_oom(
+                                                            str.string(self.bump),
+                                                        );
                                                         if let Some(mi) = self.module_info() {
                                                             let name_id = mi.str(str8);
                                                             mi.add_export_info_local(

test/bundler/bun-build-api.test.ts

@@ -1650,6 +1650,75 @@ test("Bun.build can be called thousands of times in one process without crashing
   expect(exitCode).toBe(0);
 }, 180_000);
 
+// A module shared by several entry points is printed once per chunk, and those
+// prints run in parallel on the thread pool against the same AST. The printer
+// used to flatten `"a" + "b" + "c"` ropes in place, through the `StoreRef`, so
+// one thread's write of `data` / `next = None` raced every other thread's read
+// of the same node. Observed results on the unfixed printer: the tail printed
+// twice ("abcbc"), the tail dropped ("a"), or a crash on a torn `next` pointer
+// (a `Bus error` / `Segmentation fault` at a 4 GiB aligned address).
+//
+// The race needs many chunks printing many ropes at the same time, so this
+// builds 64 entry points over one module with 400 folded ropes, twice, and
+// checks every folded string in every output. With the in-place flatten the
+// first build corrupts hundreds of strings on a 16 core machine.
+//
+// Needs an explicit timeout: two real 64-entry bundles on a debug build take
+// well over bun:test's 5s default.
+test("Bun.build does not corrupt folded string ropes shared across chunks", async () => {
+  const ENTRIES = 64;
+  const ROPES = 400;
+  const ROUNDS = 2;
+  let shared = "export function helper(...a) { return a; }\n";
+  for (let i = 0; i < ROPES; i++) {
+    // The rope is a call argument inside an arrow body, the shape the printer
+    // crashed on in the field. It folds only with `minify.syntax`.
+    shared +=
+      `export const fn${i} = helper("first${i}", () => { const q = ${i}; ` +
+      `helper(q, "alpha-${i}-" + "beta-" + "gamma-" + "delta-${i}"); return q; });\n`;
+  }
+  const files: Record<string, string> = { "shared.js": shared };
+  for (let i = 0; i < ENTRIES; i++) {
+    files[`entry${i}.js`] = `import * as s from "./shared.js";\nconsole.log(s, ${i});\n`;
+  }
+  files["run.ts"] = `
+    import { join } from "node:path";
+    const dir = process.argv[2];
+    const entrypoints = Array.from({ length: ${ENTRIES} }, (_, i) => join(dir, "entry" + i + ".js"));
+    let bad = 0;
+    for (let round = 0; round < ${ROUNDS}; round++) {
+      const res = await Bun.build({ entrypoints, minify: { syntax: true }, target: "bun" });
+      if (!res.success) throw new AggregateError(res.logs, "build failed");
+      for (const output of res.outputs) {
+        const text = await output.text();
+        for (let i = 0; i < ${ROPES}; i++) {
+          const expected = '"alpha-' + i + '-beta-gamma-delta-' + i + '"';
+          if (!text.includes(expected)) {
+            bad++;
+            if (bad <= 5) {
+              const actual = text.match(new RegExp('"alpha-' + i + '-[^"]*"'));
+              console.log("BAD round " + round + " " + output.path + " expected " + expected + " got " + actual?.[0]);
+            }
+          }
+        }
+      }
+    }
+    console.log("DONE " + bad);
+  `;
+  const dir = tempDirWithFiles("bun-build-rope-print-race", files);
+
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), join(dir, "run.ts"), dir],
+    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(stdout.trim()).toBe("DONE 0");
+  expect(exitCode).toBe(0);
+}, 180_000);
+
 test("sourcemap sourcesContent is valid JSON when source contains C0 control chars", async () => {
   // RFC 8259 only allows \" \\ \/ \b \f \n \r \t and six-char \u escapes; \v
   // and \xNN are JavaScript-only. A VT (0x0B) or BEL (0x07) in the input used

test/internal/source-lints/printer-rope-in-place.test.ts

@@ -0,0 +1,111 @@
+import { file } from "bun";
+import { expect, test } from "bun:test";
+import { realpathSync } from "fs";
+import path from "path";
+import { globAllSources } from "../../../scripts/glob-sources.ts";
+
+// The printer and the linker's chunk generation read ASTs that other threads
+// are reading at the same time: the bundler prints a module into every chunk
+// that includes it, in parallel, from one AST. An in-place rope flatten there
+// (`E::String::resolve_rope_if_needed`, or anything built on it) writes `data`
+// and `next = None` into the shared node while the other printers read it.
+// The observed results were the tail printed twice, the tail dropped, and a
+// crash on a torn `next` pointer (`Bus error at address 0x56700000000`).
+//
+// `StoreRef<T>` is `Copy` and implements `DerefMut`, so `let mut e = *e;
+// e.resolve_rope_if_needed(bump)` compiles and silently mutates the arena node.
+// The read-only form is `e.flattened(bump)`, which returns a local copy with the
+// rope flattened into `bump`. The `&mut self` rope methods are for the parser,
+// which owns its nodes.
+//
+//   x.resolve_rope_if_needed(bump)   → let x = x.flattened(bump);
+//   x.slice(bump)                    → x.flattened(bump).string(bump)
+//   x.is_identifier(bump)            → is_identifier(x.flattened(bump).slice8())
+//   x.to_utf8(bump)                  → x.flattened(bump).string(bump)
+//   e_string_mut()                   → e_string() (a `StoreRef`, read it only)
+
+const root = path.resolve(import.meta.dir, "..", "..", "..");
+
+// Code that runs on the shared, post-parse AST with other threads.
+const SCOPE = ["src/js_printer/", "src/bundler/linker_context/", "src/bundler/LinkerContext.rs"];
+
+const rustSources = globAllSources().rust.filter(abs => {
+  if (!abs.endsWith(".rs")) return false;
+  const rel = path.relative(root, abs).replaceAll(path.sep, "/");
+  return SCOPE.some(s => (s.endsWith("/") ? rel.startsWith(s) : rel === s));
+});
+
+// Only scan files tracked in HEAD (a `git stash` round-trip can leave stray
+// `.rs` files in the working tree; CI runs on a clean checkout).
+const tracked: Set<string> | null = (() => {
+  const r = Bun.spawnSync({
+    cmd: ["git", "-C", root, "ls-tree", "-r", "--name-only", "-z", "HEAD"],
+    stdout: "pipe",
+    stderr: "ignore",
+  });
+  if (!r.success) return null;
+  return new Set(r.stdout.toString().split("\0").filter(Boolean));
+})();
+
+const BANNED: { name: string; re: RegExp; hint: string }[] = [
+  {
+    name: "E::String::resolve_rope_if_needed",
+    re: /\.resolve_rope_if_needed\(/g,
+    hint: "flattened(bump)",
+  },
+  {
+    // The `E::String` method takes the arena; `StoreSlice::slice()` and the
+    // other no-argument `slice()` accessors do not match.
+    name: "E::String::slice(bump)",
+    re: /\.slice\(\s*[A-Za-z_][\w.:]*\s*\)/g,
+    hint: "flattened(bump).string(bump)",
+  },
+  {
+    // Method form with an argument; the free fn `js_lexer::is_identifier(x)`
+    // has no leading `.`.
+    name: "E::String::is_identifier(bump)",
+    re: /\.is_identifier\(\s*[A-Za-z_]/g,
+    hint: "is_identifier(flattened(bump).slice8())",
+  },
+  {
+    // `bun_core::String::to_utf8()` takes no argument and is fine.
+    name: "E::String::to_utf8(bump)",
+    re: /\.to_utf8\(\s*[A-Za-z_]/g,
+    hint: "flattened(bump).string(bump)",
+  },
+  {
+    name: "ExprData::e_string_mut",
+    re: /\.e_string_mut\(/g,
+    hint: "e_string() and read through the StoreRef",
+  },
+];
+
+const offenders: string[] = [];
+let scanned = 0;
+for (const abs of rustSources) {
+  const source = path.relative(root, abs).replaceAll(path.sep, "/");
+  if (path.relative(root, realpathSync(abs)).replaceAll(path.sep, "/") !== source) continue;
+  if (tracked !== null && !tracked.has(source)) continue;
+  scanned++;
+  const content = await file(abs).text();
+  // Strip full-line comments so prose mentions do not count. `[ \t]*`, not
+  // `\s*`, so the newline before a comment survives and line numbers hold.
+  const stripped = content.replace(/^[ \t]*\/\/.*$/gm, "");
+  const lines = stripped.split("\n");
+  for (let i = 0; i < lines.length; i++) {
+    for (const { name, re, hint } of BANNED) {
+      re.lastIndex = 0;
+      if (re.test(lines[i])) {
+        offenders.push(`${source}:${i + 1}: ${name} (use ${hint}): ${lines[i].trim()}`);
+      }
+    }
+  }
+}
+
+test("scans a non-empty set of tracked Rust sources", () => {
+  expect(scanned).toBeGreaterThan(0);
+});
+
+test("the printer and linker never flatten a string rope in place", () => {
+  expect(offenders).toEqual([]);
+});