Free the parked Bun.TOML/YAML/JSON5/JSONC/XML.parse arena when a Worker exits

src/ast/ast_memory_allocator.rs

@@ -26,9 +26,14 @@ use crate::stmt;
 // it here; the next `ASTMemoryAllocator` on this thread reclaims it, reusing
 // its committed pages. The pool holds at most one arena (nested scopes — rare
 // — fall back to a fresh `Arena::new()`). `#[thread_local]` (not the
-// `thread_local!` macro) so there is no destructor: a parked arena at thread
-// exit is reclaimed by mimalloc's own thread-teardown, avoiding an unspecified
-// destructor-ordering hazard with `mi_heap_destroy`.
+// `thread_local!` macro) so there is no destructor racing mimalloc's own
+// thread teardown. mimalloc does NOT destroy a parked heap when its thread
+// exits (heaps are not thread-bound in our fork), so an exiting thread strands
+// one empty `mi_heap_t`. That is acceptable only because the threads that park
+// here (bundler/install pool threads, the bundle thread) live as long as the
+// process; per-thread state on threads that come and go (Worker VMs) must be
+// owned by something torn down with the thread instead, like
+// `RuntimeState::text_format_arena` in `bun_runtime`.
 #[thread_local]
 static ARENA_POOL: Cell<Option<Arena>> = Cell::new(None);
 

src/runtime/api.rs

@@ -265,22 +265,27 @@ fn with_text_format_source_encoded<R>(
     use crate::node::{BlobOrStringOrBuffer, StringOrBuffer};
 
     // A private mi_heap costs microseconds to create, more than parsing a
-    // small document: keep one per thread and recycle it between calls.
-    // `#[thread_local]` rather than `thread_local!` so there is no
-    // destructor racing mimalloc's own thread teardown (as in
-    // `ast_memory_allocator.rs`); a parked heap is reclaimed with the thread.
-    #[thread_local]
-    static ARENA: core::cell::Cell<Option<bun_alloc::Arena>> = core::cell::Cell::new(None);
+    // small document: keep one per VM thread (`RuntimeState::text_format_arena`,
+    // destroyed with the VM) and recycle it between calls. The slot is empty
+    // while a call is in flight, so a re-entrant call (the argument's
+    // `toString()` parsing another document) gets its own arena; whichever
+    // parks last wins and the other is destroyed.
     struct Recycle(Option<bun_alloc::Arena>);
     impl Drop for Recycle {
         fn drop(&mut self) {
-            if let Some(mut arena) = self.0.take() {
+            let Some(mut arena) = self.0.take() else {
+                return;
+            };
+            // Re-fetched rather than captured at entry: if the VM state is
+            // gone by now the arena is simply dropped (`mi_heap_destroy`).
+            if let Some(slot) = crate::jsc_hooks::text_format_arena_slot() {
                 arena.reset_retain_with_limit(2 * 1024 * 1024);
-                ARENA.set(Some(arena));
+                slot.set(Some(arena));
             }
         }
     }
-    let recycle = Recycle(Some(ARENA.take().unwrap_or_default()));
+    let parked = crate::jsc_hooks::text_format_arena_slot().and_then(core::cell::Cell::take);
+    let recycle = Recycle(Some(parked.unwrap_or_default()));
     let arena = recycle.0.as_ref().expect("set above");
     let mut ast_memory_allocator = bun_ast::ASTMemoryAllocator::borrowing(arena);
     let _ast_scope = ast_memory_allocator.enter();

src/runtime/jsc_hooks.rs

@@ -103,6 +103,12 @@ pub(crate) struct RuntimeState {
     /// The resolver's PackageManager wake-handler context (module queue + VM
     /// handle); the resolver holds a raw pointer to it. Freed with the state.
     pub(crate) wake_ctx: Option<Box<bun_jsc::async_module::WakeContext>>,
+    /// Parked `mi_heap` recycled between `Bun.{TOML,YAML,JSON5,JSONC,XML}.parse`
+    /// calls on this thread (see `api::with_text_format_source_encoded`).
+    /// Owned here rather than in a `#[thread_local]` so Worker teardown
+    /// destroys it: mimalloc does not free a `mi_heap_t` when the thread that
+    /// created it exits, so a heap parked in TLS leaks with every Worker.
+    pub(crate) text_format_arena: Cell<Option<bun_alloc::Arena>>,
 }
 
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
@@ -239,6 +245,24 @@ pub(crate) fn global_dns_data() -> &'static core::cell::OnceCell<Box<crate::dns_
     unsafe { &(*state).global_dns_data }
 }
 
+/// The slot holding this thread's parked text-format parse arena
+/// ([`RuntimeState::text_format_arena`]), or `None` when no VM state is
+/// installed on this thread (callers then use a throwaway arena). Do not hold
+/// the returned reference across code that can tear the VM down; re-fetch it
+/// instead.
+#[inline]
+pub(crate) fn text_format_arena_slot() -> Option<&'static Cell<Option<bun_alloc::Arena>>> {
+    let state = runtime_state();
+    if state.is_null() {
+        return None;
+    }
+    // SAFETY: `state` is the live per-thread `RuntimeState` box; the field
+    // address is stable until `deinit_runtime_state`, which nulls
+    // `RUNTIME_STATE` before freeing the box, so a non-null `state` here is
+    // never a freed one. Mutation goes through the `Cell`.
+    Some(unsafe { &(*state).text_format_arena })
+}
+
 /// Recover the [`RuntimeState`] owned by a specific `vm` (not the calling
 /// thread's). `WTFTimer` may be entered off the VM's JS thread (the locked
 /// `All.wtf_timers` heap exists for exactly that), and the
@@ -400,6 +424,7 @@ unsafe fn init_runtime_state(
         },
         active_handles: ActiveHandles::default(),
         wake_ctx: None,
+        text_format_arena: Cell::new(None),
     }));
     RUNTIME_STATE.with(|c| c.set(state));
 

test/js/node/worker_threads/worker_destruction.test.ts

@@ -70,4 +70,19 @@ describe("Worker destruction", () => {
     expect(stdout).toBe("worker exit 0\n");
     expect(exitCode).toBe(0);
   });
+
+  // Allocator heaps that APIs park on the Worker's thread (Bun.TOML.parse and friends, the module
+  // loader) must die with the Worker's VM; see the fixture for details.
+  test.concurrent("a Worker that used per-thread allocator heaps does not leak them when it exits", async () => {
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), join(import.meta.dir, "worker-heap-leak-fixture.js")],
+      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)).toEqual({ workers: 3, leaked: 0 });
+    expect(exitCode).toBe(0);
+  });
 });