sourcemap: parse source-map JSON in a private AST scope

src/sourcemap/lib.rs

@@ -416,15 +416,6 @@ pub(crate) fn get_source_map_impl<P: SourceProvider + ?Sized>(
     load_hint: SourceMapLoadHint,
     result: ParseUrlResultHint,
 ) -> Option<ParseUrl> {
-    // This was previously 65535 but that is a size that can risk stack overflow
-    // and due to the many layers of indirections and wrappers this function is called in, it
-    // is difficult to reason about how deeply nested of a callstack this
-    // function is called in. 1024 is a safer number.
-    //
-    // TODO: Experiment in debug builds calculating how much stack space we have left and using that to
-    //       adjust the size
-    let arena = bun_alloc::Arena::new();
-
     let (new_load_hint, mut parsed): (SourceMapLoadHint, ParseUrl) = 'parsed: {
         let mut inline_err: Option<crate::Error> = None;
 
@@ -444,7 +435,7 @@ pub(crate) fn get_source_map_impl<P: SourceProvider + ?Sized>(
                     break 'try_inline;
                 };
 
-                match parse_url(&arena, found_url.slice(), result) {
+                match parse_url(found_url.slice(), result) {
                     Ok(parsed) => break 'parsed (SourceMapLoadHint::IsInlineMap, parsed),
                     Err(err) => {
                         inline_err = Some(err);
@@ -818,13 +809,8 @@ impl SourceMapPieces {
 ///
 /// `source` must be in UTF-8 and can be freed after this call.
 /// The mappings are owned by the global allocator.
-/// Temporary allocations are made to the `arena` allocator, which
-/// should be an arena allocator (caller is assumed to call `reset`).
-pub(crate) fn parse_url(
-    arena: &bun_alloc::Arena,
-    source: &[u8],
-    hint: ParseUrlResultHint,
-) -> crate::Result<ParseUrl> {
+pub(crate) fn parse_url(source: &[u8], hint: ParseUrlResultHint) -> crate::Result<ParseUrl> {
+    let decoded_json: Vec<u8>;
     let json_bytes: &[u8] = 'json_bytes: {
         const DATA_PREFIX: &[u8] = b"data:application/json";
 
@@ -846,13 +832,9 @@ pub(crate) fn parse_url(
                         }
                         let base64_data = &after[comma + 1..];
 
-                        let len = bun_base64::decode_len(base64_data);
-                        let bytes = arena.alloc_slice_fill_default::<u8>(len);
-                        let decoded = bun_base64::decode(bytes, base64_data);
-                        if !decoded.is_successful() {
-                            return Err(crate::Error::InvalidBase64);
-                        }
-                        break 'json_bytes &bytes[..decoded.count];
+                        decoded_json = bun_base64::decode_alloc(base64_data)
+                            .map_err(|_| crate::Error::InvalidBase64)?;
+                        break 'json_bytes &decoded_json;
                     }
                     b',' => break 'json_bytes &source[DATA_PREFIX.len() + 1..],
                     _ => break 'try_data_url,
@@ -872,19 +854,19 @@ pub(crate) fn parse_url(
 /// The mappings are owned by the global allocator.
 pub(crate) fn parse_json(source: &[u8], hint: ParseUrlResultHint) -> crate::Result<ParseUrl> {
     use crate::mapping::SourceMap as SourceMapLog;
-    use bun_ast::StoreResetGuard as DataStoreScope;
     use std::sync::Arc;
 
     let json_src = bun_ast::Source::init_path_string("sourcemap.json", source);
     let mut log = bun_ast::Log::init();
     // `defer log.deinit()` → Drop
 
-    // the allocator given to the JS parser is not respected for all parts
-    // of the parse, so we need to remember to reset the ast store on entry
-    // and on every exit path. A source map may be parsed lazily from a thread that never ran the parser (a stack trace
-    // remapped from a collector thread's end phase), so make sure this thread has the stores at all.
-    bun_ast::initialize_store();
-    let _store_scope = DataStoreScope::new();
+    // This can run in a GC end phase: an outer frame may still read this thread's AST store, or the thread has none.
+    let arena = bun_alloc::Arena::new();
+    let mut ast_memory_allocator = bun_ast::ASTMemoryAllocator::borrowing(&arena);
+    let _ast_scope = ast_memory_allocator.enter();
+    // In a debug build, a node that goes to that store panics.
+    let _stmt_guard = bun_ast::stmt::Disabler::scope();
+    let _expr_guard = bun_ast::expr::Disabler::scope();
     bun_core::scoped_log!(SourceMapLog, "parse (JSON, {} bytes)", source.len());
     let parsed = match bun_parsers::json::ParsedJson::parse_json(&json_src, &mut log) {
         Ok(p) => p,

src/standalone_graph/StandaloneModuleGraph.rs

@@ -3880,9 +3880,13 @@ pub(crate) fn serialize_json_source_map_for_standalone(
     let json_src = bun_ast::Source::init_path_string("sourcemap.json", json_source);
     let mut log = bun_ast::Log::init();
 
-    // the allocator given to the JS parser is not respected for all parts
-    // of the parse, so we need to remember to reset the ast store
-    let _reset_guard = bun_ast::StoreResetGuard::new();
+    // An outer frame may still read this thread's AST store.
+    let arena = bun_alloc::Arena::new();
+    let mut ast_memory_allocator = bun_ast::ASTMemoryAllocator::borrowing(&arena);
+    let _ast_scope = ast_memory_allocator.enter();
+    // In a debug build, a node that goes to that store panics.
+    let _stmt_guard = bun_ast::stmt::Disabler::scope();
+    let _expr_guard = bun_ast::expr::Disabler::scope();
 
     let parsed = bun_parsers::json::ParsedJson::parse_json(&json_src, &mut log)
         .map_err(|_| crate::Error::InvalidSourceMap)?;

test/js/bun/sourcemap/external-sourcemap-stack-leak.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from "bun:test";
-import { bunEnv, bunExe, MAX_PATH_BYTES, tempDir } from "harness";
+import { bunEnv, bunExe, isASAN, isDebug, MAX_PATH_BYTES, tempDir } from "harness";
 import { join } from "node:path";
 
 // Formatting a stack trace hands one ZigStackFrame per frame to
@@ -155,3 +155,207 @@ test("prints the stack trace when a `sources` entry is longer than a path buffer
   expect(stderr).toContain(`at t (${join(String(dir), sourceName)}:`);
   expect(exitCode).toBe(1);
 });
+
+// A collection formats the stack of an Error that nothing has read yet once a function in one of its frames is dead.
+// For a frame in a `// @bun` file that is the first use of the file's source map, so the map's JSON is parsed inside the
+// collection, on top of whatever the JS thread was doing when it allocated. Here that is the conversion of a parsed
+// data file into a JS value, which reads AST nodes out of the thread's store. The map parse used to put its own nodes
+// in that store and rewind it, and the next map's nodes then went over the first nodes of the data file: the value
+// came back with keys missing or wrong and exit code 0, or the process crashed.
+const GC_MODULES = 4;
+// Whether a collection formats a given stack is not up to the test: on macOS arm64 it formats three of these four.
+// Two are needed: on a release build the value was damaged once a second source map was parsed during the load.
+const GC_MIN_REMAPPED = 2;
+// The first value is larger than what the heap may grow by before it is collected, so the collection starts in the
+// middle of the conversion. The nodes of the small values after it are the next ones in the store.
+const GC_BIG_LENGTH = 4 * 1024 * 1024;
+const GC_SMALL = 40;
+
+const gcBig = Buffer.alloc(GC_BIG_LENGTH, "b").toString();
+const gcSmall = Array.from({ length: GC_SMALL }, (_, i) => "value-" + String(i).padStart(4, "0"));
+const gcPairs = (pair: (key: string, value: string) => string, separator: string) =>
+  [pair("big", gcBig), ...gcSmall.map((value, i) => pair(`key${i}`, value))].join(separator);
+const quoted = JSON.stringify;
+
+const gcDataFiles: Record<string, () => string> = {
+  "data.jsonc": () => `// a comment\n{\n${gcPairs((key, value) => `${quoted(key)}: ${quoted(value)}`, ",\n")},\n}\n`,
+  "pkg/package.json": () => `{ ${gcPairs((key, value) => `${quoted(key)}: ${quoted(value)}`, ", ")} }`,
+  "data.xml": () => `<root>\n${gcPairs((key, value) => `<${key}>${value}</${key}>`, "\n")}\n</root>\n`,
+  "data.toml": () => `${gcPairs((key, value) => `${key} = ${quoted(value)}`, "\n")}\n`,
+  "data.yaml": () => `${gcPairs((key, value) => `${key}: ${quoted(value)}`, "\n")}\n`,
+  "data.json5": () => `{\n${gcPairs((key, value) => `${key}: ${quoted(value)}`, ",\n")},\n}\n`,
+};
+
+const gcMain = /* js */ `
+    const [how, file] = process.argv.slice(2);
+    const unformatted = "no collection formatted this stack";
+    // Error.prepareStackTrace is not called for a stack that a collection formatted. If it is one day (#46400), the
+    // call sites of such a stack have no function. The functions here are sloppy, so the call sites of a stack that
+    // no collection formatted have one.
+    const where = (error, [site]) =>
+      site.getFunction() === undefined
+        ? site.getFileName() + ":" + site.getLineNumber() + ":" + site.getColumnNumber()
+        : unformatted;
+    (async () => {
+      // A collected heap, and little is allocated from here to the load: the next collection is the one that the
+      // first value of the file brings on.
+      Bun.gc(true);
+      // A collection that runs before the load formats this stack, and this file has no source map file to parse.
+      const canary = (function first() { const error = new Error("canary"); return error; })();
+      const errors = [];
+      for (let i = 0; i < ${GC_MODULES}; i++) errors.push(require("./m" + i + ".js").make());
+      Error.prepareStackTrace = where;
+      const early = canary.stack !== unformatted;
+      Error.prepareStackTrace = undefined;
+
+      let data = how === "import" ? (await import(file)).default : require(file);
+      // A stack that a collection formatted is a string by now. One that it did not format is formatted by this, and
+      // is not counted. With fewer than ${GC_MIN_REMAPPED} counted, the load no longer does what this test is for.
+      Error.prepareStackTrace = where;
+      let remapped = 0;
+      for (let i = 0; i < ${GC_MODULES}; i++) if (errors[i].stack.includes("m" + i + ".ts:5:3")) remapped++;
+      remapped = Math.min(remapped, ${GC_MIN_REMAPPED});
+
+      data = data.root ?? data;
+      let intact = 0;
+      for (let i = 0; i < ${GC_SMALL}; i++) if (data["key" + i] === "value-" + String(i).padStart(4, "0")) intact++;
+      // After the count: this allocation can start a collection.
+      const big = data.big === Buffer.alloc(${GC_BIG_LENGTH}, "b").toString();
+      console.log(JSON.stringify({ keys: Object.keys(data).length, big, intact, remapped, early }));
+    })();
+  `;
+// Each module has a `.map` file. `sourceMappingURL` is what its last line names.
+function gcFixture(sourceMappingURL: (i: number) => string) {
+  const files: Record<string, string> = { "main.cjs": gcMain };
+  for (let i = 0; i < GC_MODULES; i++) {
+    // `inner` is garbage once `make` returns, and the Error holds its frame: a collection that finds `inner` dead
+    // formats the stack.
+    files[`m${i}.js`] =
+      `// @bun @bun-cjs\n(function (exports, require, module, __filename, __dirname) { exports.make = function make() { return (function inner() { const error = new Error("e${i}"); return error; })(); };\n})\n//# sourceMappingURL=${sourceMappingURL(i)}\n`;
+    // Line 2 of the module is line 5, column 3 of its source.
+    files[`m${i}.js.map`] = JSON.stringify({
+      version: 3,
+      sources: [`m${i}.ts`],
+      sourcesContent: [""],
+      names: [],
+      mappings: ";AAIE",
+    });
+  }
+  return files;
+}
+
+async function gcRow(how: string, file: string, sourceMappingURL: (i: number) => string) {
+  using dir = tempDir("sourcemap-gc-data", { ...gcFixture(sourceMappingURL), [file]: gcDataFiles[file]() });
+  // --smol: the heap may grow by 1 MB before it is collected. Without a concurrent collector the collection is over
+  // inside the allocation that starts it. A concurrent one can end after the load, which is short from there.
+  // symbolize=0: a child that crashes under ASAN otherwise spends 5 s in the symbolizer, and the test times out
+  // before it can show the report.
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "--smol", "main.cjs", how, `./${file}`],
+    cwd: String(dir),
+    env: {
+      ...bunEnv,
+      BUN_JSC_useConcurrentGC: "0",
+      ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "symbolize=0"].filter(Boolean).join(":"),
+    },
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({
+    stdout: JSON.stringify({
+      keys: GC_SMALL + 1,
+      big: true,
+      intact: GC_SMALL,
+      remapped: GC_MIN_REMAPPED,
+      early: false,
+    }),
+    stderr: "",
+    exitCode: 0,
+  });
+}
+
+describe.concurrent("a source map that a collection parses leaves the AST store of the thread as it is", () => {
+  test.each([
+    ...Object.keys(gcDataFiles).map(file => ["require", file]),
+    // One file of each kind of tree: the rows of the JSON parser, and nodes.
+    ["import", "data.jsonc"],
+    ["import", "data.toml"],
+  ])("%s of %s", (how, file) => gcRow(how, file, i => `m${i}.js.map`));
+
+  // Each module names a data URL that fails, so its `.map` file is read after it.
+  test.each([
+    // The decoder rejects the payload.
+    ["bad base64", () => "data:application/json;base64,A"],
+    // The JSON is parsed into nodes, and then its mappings are rejected.
+    [
+      "bad mappings",
+      (i: number) =>
+        "data:application/json;base64," +
+        Buffer.from(
+          JSON.stringify({ version: 3, sources: [`inline${i}.ts`], sourcesContent: [""], names: [], mappings: "g" }),
+        ).toString("base64"),
+    ],
+  ] as const)("require of data.jsonc, after an inline source map with %s", (_, sourceMappingURL) =>
+    gcRow("require", "data.jsonc", sourceMappingURL),
+  );
+});
+
+// `Bun.build({ compile })` parses the source map of each chunk on the JS thread when the build completes. Here it
+// completes inside the conversion of a YAML file: a key that is a sequence is named with Array.prototype.join, and the
+// join below calls Bun.build() with a plugin, which waits inside the call for the promise that setup() returns.
+// Debug and ASAN executables are too big to compile a copy of per run.
+test.skipIf(isDebug || isASAN)(
+  "a source map that Bun.build({ compile }) parses leaves the AST store of the thread as it is",
+  async () => {
+    using dir = tempDir("sourcemap-compile-data", {
+      // Two chunks, so two source maps: the value was damaged once a second one was parsed.
+      "split.js": `const lazy = await import("./lazy.js");\nconsole.log(lazy.value);\n`,
+      "lazy.js": `export const value = 1;\n`,
+      "data.yaml": `? []\n: 1\n${gcSmall.map((value, i) => `key${i}: ${quoted(value)}`).join("\n")}\n`,
+      "main.cjs": /* js */ `
+        const { join } = require("path");
+        const build = Bun.build({
+          entrypoints: [join(__dirname, "split.js")],
+          compile: { outfile: join(__dirname, "compiled") },
+          splitting: true,
+          sourcemap: "inline",
+        });
+        let completed = false;
+        build.then(() => (completed = true), () => (completed = true));
+        let completedInJoin = null;
+        const originalJoin = Array.prototype.join;
+        Array.prototype.join = function (...args) {
+          if (completedInJoin === null) {
+            Bun.build({
+              entrypoints: [join(__dirname, "lazy.js")],
+              plugins: [{ name: "wait", setup: () => build.then(() => {}, () => {}) }],
+            }).catch(() => {});
+            completedInJoin = completed;
+          }
+          return originalJoin.apply(this, args);
+        };
+        const data = require("./data.yaml");
+        Array.prototype.join = originalJoin;
+        let intact = 0;
+        for (let i = 0; i < ${GC_SMALL}; i++) if (data["key" + i] === "value-" + String(i).padStart(4, "0")) intact++;
+        build.then(({ success }) =>
+          console.log(JSON.stringify({ keys: Object.keys(data).length, intact, completedInJoin, success })),
+        );
+      `,
+    });
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "main.cjs"],
+      cwd: String(dir),
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({
+      stdout: JSON.stringify({ keys: GC_SMALL + 1, intact: GC_SMALL, completedInJoin: true, success: true }),
+      stderr: "",
+      exitCode: 0,
+    });
+  },
+);