bundler: keep BundlerPlugin filter data alive across Worker.terminate()

src/bundler/bundle_v2.rs

@@ -191,7 +191,7 @@ impl<'a> BundleV2<'a> {
         self.client_transpiler.as_deref()
     }
 
-    /// Safe projection of the `plugins` backref (opaque C++ `BunPlugin`).
+    /// Safe projection of the `plugins` backref (opaque C++ `Bun::BundlerPlugin`).
     /// Set once in `init` from `BakeOptions` / completion config; live for the
     /// bundle pass.
     #[inline]
@@ -694,12 +694,13 @@ pub mod bv2_impl {
             use bun_core::String as BunString;
             use bun_resolver::fs::PathResolverExt as _;
 
-            // `Plugin = opaque {}` — backed by C++ `BunPlugin`. The bundler calls
-            // `has_any_matches` / `match_on_load` / `match_on_resolve` directly
-            // (no JSC types needed — only `BunString` / raw context ptrs). The
-            // JSC-aware methods (`create`, `add_plugin`, `global_object`, …) are
-            // added by `bun_runtime` via the `PluginJscExt` extension trait so
-            // this crate stays free of `JSValue` / `JSGlobalObject`.
+            // `Plugin = opaque {}` — backed by C++ `Bun::BundlerPlugin` (heap,
+            // ThreadSafeRefCounted). The bundler calls `has_any_matches` /
+            // `match_on_load` / `match_on_resolve` directly (no JSC types
+            // needed — only `BunString` / raw context ptrs). The JSC-aware
+            // methods (`create`, `add_plugin`, `global_object`, …) are added by
+            // `bun_runtime` via the `PluginJscExt` extension trait so this
+            // crate stays free of `JSValue` / `JSGlobalObject`.
             bun_opaque::opaque_ffi! { pub struct Plugin; }
             unsafe extern "C" {
                 // The three `safe fn`s below take only Rust references / by-value

src/jsc/bindings/JSBundlerPlugin.h

@@ -7,6 +7,7 @@
 #include <JavaScriptCore/RegularExpression.h>
 #include "napi_external.h"
 #include <JavaScriptCore/Yarr.h>
+#include <wtf/ThreadSafeRefCounted.h>
 #include "WriteBarrierList.h"
 
 typedef void (*JSBundlerPluginAddErrorCallback)(void*, void*, JSC::EncodedJSValue, JSC::EncodedJSValue);
@@ -18,7 +19,11 @@ namespace Bun {
 
 using namespace JSC;
 
-class BundlerPlugin final {
+class JSBundlerPlugin;
+
+// Ref-counted separately from the JSBundlerPlugin GC cell so the bundle thread can
+// read filter lists after a Worker's VM is torn down. Cell holds one ref, Rust another.
+class BundlerPlugin final : public ThreadSafeRefCounted<BundlerPlugin> {
 public:
     /// In native plugins, the regular expression could be called concurrently on multiple threads.
     /// Therefore, we need a mutex to synchronize access.
@@ -41,7 +46,7 @@ class BundlerPlugin final {
         {
         }
 
-        bool match(JSC::VM& vm, const String& path);
+        bool match(const String& path);
     };
 
     class NamespaceList {
@@ -74,7 +79,9 @@ class BundlerPlugin final {
 
     struct NativePluginCallback {
         JSBundlerPluginNativeOnBeforeParseCallback callback;
-        Bun::NapiExternal* external;
+        // NapiExternal::value() captured at append time so the parse thread never
+        // reads the GC cell; onBeforeParseExternals keeps the cell alive for GC.
+        void* externalValue;
         /// This refers to the string exported in the native plugin under
         /// the symbol BUN_PLUGIN_NAME
         ///
@@ -95,8 +102,8 @@ class BundlerPlugin final {
         PerNamespaceCallbackList fileCallbacks = {};
         Vector<PerNamespaceCallbackList> namespaceCallbacks = {};
 
-        int call(JSC::VM& vm, BundlerPlugin* plugin, int* shouldContinue, void* bunContextPtr, const BunString* namespaceStr, const BunString* pathString, OnBeforeParseArguments* onBeforeParseArgs, OnBeforeParseResult* onBeforeParseResult);
-        void append(JSC::VM& vm, JSC::RegExp* filter, String& namespaceString, JSBundlerPluginNativeOnBeforeParseCallback callback, const char* name, NapiExternal* external);
+        int call(BundlerPlugin* plugin, int* shouldContinue, void* bunContextPtr, const BunString* namespaceStr, const BunString* pathString, OnBeforeParseArguments* onBeforeParseArgs, OnBeforeParseResult* onBeforeParseResult);
+        void append(JSC::VM& vm, JSC::RegExp* filter, String& namespaceString, JSBundlerPluginNativeOnBeforeParseCallback callback, const char* name, void* externalValue);
 
         Vector<FilterRegExp>* group(const String& namespaceStr, unsigned& index)
         {
@@ -118,33 +125,44 @@ class BundlerPlugin final {
     };
 
 public:
-    bool anyMatchesCrossThread(JSC::VM&, BunString* namespaceStr, BunString* path, bool isOnLoad);
-    void tombstone() { tombstoned = true; }
-
-    BundlerPlugin(void* config, BunPluginTarget target, JSBundlerPluginAddErrorCallback addError, JSBundlerPluginOnLoadAsyncCallback onLoadAsync, JSBundlerPluginOnResolveAsyncCallback onResolveAsync)
-        : addError(addError)
-        , onLoadAsync(onLoadAsync)
-        , onResolveAsync(onResolveAsync)
+    static Ref<BundlerPlugin> create(void* config, BunPluginTarget target, JSBundlerPluginAddErrorCallback addError, JSBundlerPluginOnLoadAsyncCallback onLoadAsync, JSBundlerPluginOnResolveAsyncCallback onResolveAsync)
     {
-        this->target = target;
-        this->config = config;
+        return adoptRef(*new BundlerPlugin(config, target, addError, onLoadAsync, onResolveAsync));
     }
 
+    bool anyMatchesCrossThread(BunString* namespaceStr, BunString* path, bool isOnLoad);
+    void tombstone() { tombstoned = true; }
+
+    JSBundlerPlugin* cell() const { return m_cell; }
+    void setCell(JSBundlerPlugin* cell) { m_cell = cell; }
+
     NamespaceList onLoad = {};
     NamespaceList onResolve = {};
     NativePluginList onBeforeParse = {};
     BunPluginTarget target { BunPluginTargetBrowser };
 
     WriteBarrierList<JSC::JSPromise> deferredPromises = {};
-    // The raw `NapiExternal*` stored in `NativePluginCallback` is dereferenced
-    // off the JS thread; this list keeps those cells alive for GC.
+    // Roots each registered NapiExternal against normal GC while the build runs.
     WriteBarrierList<NapiExternal> onBeforeParseExternals = {};
 
     JSBundlerPluginAddErrorCallback addError;
     JSBundlerPluginOnLoadAsyncCallback onLoadAsync;
     JSBundlerPluginOnResolveAsyncCallback onResolveAsync;
     void* config { nullptr };
     bool tombstoned { false };
+
+private:
+    BundlerPlugin(void* config, BunPluginTarget target, JSBundlerPluginAddErrorCallback addError, JSBundlerPluginOnLoadAsyncCallback onLoadAsync, JSBundlerPluginOnResolveAsyncCallback onResolveAsync)
+        : addError(addError)
+        , onLoadAsync(onLoadAsync)
+        , onResolveAsync(onResolveAsync)
+    {
+        this->target = target;
+        this->config = config;
+    }
+
+    // Back-pointer into the owning VM's GC heap. Only dereferenced on the JS thread.
+    JSBundlerPlugin* m_cell { nullptr };
 };
 
 } // namespace Zig

src/runtime/api/JSBundler.rs

@@ -1620,7 +1620,7 @@ pub mod js_bundler {
         bv2_mut(this.bv2).on_load_async(this);
     }
 
-    /// Opaque FFI handle for the C++ `JSBundlerPlugin`. The opaque type and
+    /// Opaque FFI handle for the C++ `Bun::BundlerPlugin`. The opaque type and
     /// `has_any_matches` (the one method `bun_bundler` needs) live in the
     /// lower-tier crate; JSC-aware methods are added here via `PluginJscExt`.
     pub use bun_bundler::bundle_v2::api::JSBundler::Plugin;
@@ -1635,7 +1635,7 @@ pub mod js_bundler {
             global: &JSGlobalObject,
             target: jsc::BunPluginTarget,
         ) -> *mut Plugin;
-        safe fn JSBundlerPlugin__tombstone(plugin: &Plugin);
+        safe fn JSBundlerPlugin__destroy(plugin: &Plugin);
         safe fn JSBundlerPlugin__runOnEndCallbacks(
             plugin: &mut Plugin,
             build_promise: JSValue,
@@ -1663,9 +1663,9 @@ pub mod js_bundler {
         ) -> JSValue;
     }
 
-    /// JSC-aware methods on the C++ `JSBundlerPlugin` opaque. The opaque type
-    /// itself is owned by `bun_bundler` (lower tier, no JSC dep), so these are
-    /// added as an extension trait rather than an inherent `impl`.
+    /// JSC-aware methods on the C++ `Bun::BundlerPlugin` opaque. The opaque
+    /// type itself is owned by `bun_bundler` (lower tier, no JSC dep), so these
+    /// are added as an extension trait rather than an inherent `impl`.
     pub trait PluginJscExt {
         fn create(global: &JSGlobalObject, target: jsc::BunPluginTarget) -> *mut Plugin;
         fn run_on_end_callbacks(
@@ -1701,9 +1701,7 @@ pub mod js_bundler {
     impl PluginJscExt for Plugin {
         fn create(global: &JSGlobalObject, target: jsc::BunPluginTarget) -> *mut Plugin {
             jsc::mark_binding();
-            let plugin = JSBundlerPlugin__create(global, target);
-            JSValue::from_cell(plugin).protect();
-            plugin
+            JSBundlerPlugin__create(global, target)
         }
 
         fn run_on_end_callbacks(
@@ -1739,8 +1737,7 @@ pub mod js_bundler {
 
         fn destroy(this: *mut Plugin) {
             jsc::mark_binding();
-            JSBundlerPlugin__tombstone(Plugin::opaque_ref(this));
-            JSValue::from_cell(this).unprotect();
+            JSBundlerPlugin__destroy(Plugin::opaque_ref(this));
         }
 
         fn global_object(&self) -> &JSGlobalObject {

src/runtime/api/js_bundle_completion_task.rs

@@ -190,11 +190,11 @@ impl JSBundleCompletionTask {
     ///
     /// Centralises the `Option<NonNull> → Option<&mut T>` deref so callers
     /// (`to_js_error` / `on_complete_anytask`) stay safe. The plugin is a C++
-    /// `JSBundlerPlugin` opaque created by [`PluginJscExt::create`] and
-    /// `protect()`-ed for the task's lifetime; it is freed only via
-    /// `Plugin::destroy` in `deinit` *after* `take()` clears `self.plugins`.
-    /// While the field is `Some` the pointee is therefore live, pinned, and
-    /// disjoint from `*self` (separate C++-heap allocation).
+    /// `BundlerPlugin` heap allocation created by [`PluginJscExt::create`]
+    /// (which also `gcProtect`s the owning `JSBundlerPlugin` GC cell and takes
+    /// a +1 ref for this task); it is released only via `Plugin::destroy` in
+    /// `deinit` *after* `take()` clears `self.plugins`. While the field is
+    /// `Some` the pointee is therefore live, pinned, and disjoint from `*self`.
     #[inline]
     fn plugins_mut(&mut self) -> Option<&mut Plugin> {
         // SAFETY: see fn doc — C++-heap opaque, live while `self.plugins` is

src/runtime/bake/bake_body.rs

@@ -155,8 +155,7 @@ impl Drop for UserOptions {
         if let Some(p) = self.bundler_options.plugin {
             // `p` is the FFI handle returned by `Plugin::create` in
             // `parse_plugin_array`; `PluginJscExt::destroy` is its paired
-            // (safe) destructor — it null-checks via `opaque_ref` and
-            // unprotect()s the JSCell / tombstones the C++ object.
+            // destructor (tombstones + gcUnprotects the cell + drops the ref).
             Plugin::destroy(p.as_ptr());
         }
     }
@@ -321,8 +320,7 @@ impl SplitBundlerOptions {
             Some(p) => p,
             None => {
                 let p = Plugin::create(global, bun_jsc::BunPluginTarget::Bun);
-                let p = NonNull::new(p)
-                    .expect("JSBundlerPlugin__create returns a non-null protected JSCell");
+                let p = NonNull::new(p).expect("JSBundlerPlugin__create returns non-null");
                 self.plugin = Some(p);
                 p
             }
@@ -360,7 +358,7 @@ impl SplitBundlerOptions {
             };
 
             // `Plugin` is an `opaque_ffi!` ZST — `opaque_mut` is the safe
-            // deref. Handle held live in `self.plugin` (protected JSCell).
+            // deref. Handle held live in `self.plugin` (ref-counted C++ heap).
             let plugin_result = Plugin::opaque_mut(plugin.as_ptr()).add_plugin(
                 function,
                 empty_object,

src/runtime/bake/mod.rs

@@ -40,11 +40,11 @@ pub use bake_body::{PatternBuffer, UserOptions, print_warning};
 
 /// All bake JSC references go through this re-export of `bun_jsc`.
 pub mod jsc {
-    /// `jsc.API.JSBundler.Plugin` — the C++ `BunPlugin` FFI handle. The
-    /// canonical opaque struct lives in `bun_bundler::bundle_v2::api::JSBundler`
-    /// (T5) and is re-exported through `crate::api::js_bundler` so the
-    /// JSC-aware `PluginJscExt` methods are in scope; both paths name the same
-    /// nominal type.
+    /// `jsc.API.JSBundler.Plugin` — the C++ `Bun::BundlerPlugin` FFI handle.
+    /// The canonical opaque struct lives in
+    /// `bun_bundler::bundle_v2::api::JSBundler` (T5) and is re-exported through
+    /// `crate::api::js_bundler` so the JSC-aware `PluginJscExt` methods are in
+    /// scope; both paths name the same nominal type.
     pub(crate) use crate::api::js_bundler::Plugin;
     pub(crate) use crate::jsc::*;
 }

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

@@ -1525,3 +1525,103 @@ test("Bun.build can be called thousands of times in one process without crashing
   expect(stdout.trim()).toBe("OK 400");
   expect(exitCode).toBe(0);
 }, 180_000);
+
+// The bundle thread calls JSBundlerPlugin__anyMatches / hasOnBeforeParsePlugins against
+// the plugin's filter vectors while a build is in flight. Those vectors used to live
+// inline in the JSBundlerPlugin GC cell, so worker.terminate() (which tears down the
+// worker's JSC heap) freed them out from under the bundle thread:
+//
+//   JSBundlerPlugin.cpp:96: runtime error: reference binding to null pointer of type
+//   'Bun::BundlerPlugin::FilterRegExp'
+//
+// The filter data is now a ThreadSafeRefCounted heap allocation that outlives the cell,
+// so the cross-thread read stays valid after the worker's VM is gone.
+//
+// This test cannot assert "exit 0": the same terminate() race also trips the
+// pre-existing, plugin-independent complete_on_bundle_thread use-after-free (posting the
+// build result to the dead worker's event loop), which reproduces on current main with
+// or without this change. So we run the race repeatedly and assert only that no attempt
+// faults inside JSBundlerPlugin.cpp. Malloc=1 routes JSC allocations through system
+// malloc so ASAN can see writes to the freed cell.
+test.skipIf(!isASAN)(
+  "terminating a Worker mid-Bun.build() does not read freed plugin filter data on the bundle thread",
+  async () => {
+    const MODULES = 300;
+    const files: Record<string, string> = {};
+    let entry = "";
+    for (let i = 0; i < MODULES; i++) {
+      files[`m${i}.js`] = `export const v${i} = ${i};\n`;
+      entry += `import { v${i} } from "./m${i}.js";\n`;
+    }
+    entry += `export default 0;\n`;
+    files["entry.js"] = entry;
+    files["w.cjs"] = `
+      const { parentPort, workerData: d } = require("node:worker_threads");
+      async function lane(k) {
+        for (;;) {
+          await Bun.build({
+            entrypoints: [d.dir + "/entry.js"],
+            plugins: [{ name: "p" + k, setup(b) { b.onLoad({ filter: /never-matches-anything/ }, () => undefined); } }],
+          }).catch(() => {});
+        }
+      }
+      parentPort.postMessage("up");
+      for (let k = 0; k < 2; k++) lane(k);
+    `;
+    files["parent.mjs"] = `
+      import { Worker } from "node:worker_threads";
+      const dir = process.argv[2];
+      const t0 = performance.now();
+      for (let r = 0; r < 40 && performance.now() - t0 < 8000; r++) {
+        const ws = [];
+        for (let i = 0; i < 2; i++) {
+          const w = new Worker(dir + "/w.cjs", { workerData: { dir } });
+          w.on("error", () => {});
+          ws.push(w);
+        }
+        await Promise.all(ws.map(w => new Promise(res => { w.once("message", res); setTimeout(res, 3000); })));
+        await Bun.sleep(5 + (r * 7) % 30);
+        await Promise.all(ws.map(w => w.terminate()));
+      }
+      console.log("ok");
+    `;
+
+    using dir = tempDir("bun-build-worker-plugin-terminate", files);
+
+    // The subprocess is expected to crash (the unrelated complete_on_bundle_thread UAF
+    // still exists on main), and on a given crash it may land in either site. Run enough
+    // attempts that, without the fix, at least one lands in JSBundlerPlugin.cpp. Once
+    // #35158 / #35767 land this can become a plain expect(stdout).toContain("ok") +
+    // expect(exitCode).toBe(0).
+    const ATTEMPTS = 8;
+    const frames: string[] = [];
+    let sawSanitizerReport = false;
+    let sawCleanExit = false;
+    for (let attempt = 0; attempt < ATTEMPTS; attempt++) {
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), join(String(dir), "parent.mjs"), String(dir)],
+        env: { ...bunEnv, Malloc: "1" },
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr] = await Promise.all([proc.stdout.text(), proc.stderr.text()]);
+      await proc.exited;
+
+      if (/AddressSanitizer|runtime error:|SUMMARY: /.test(stderr)) sawSanitizerReport = true;
+      if (stdout.includes("ok") && stderr === "") sawCleanExit = true;
+
+      const pluginFrames = stderr.split("\n").filter(l => /JSBundlerPlugin\.cpp|BundlerPlugin::|FilterRegExp/.test(l));
+      if (pluginFrames.length > 0) {
+        frames.push(`attempt ${attempt}:\n${pluginFrames.join("\n")}`);
+        break;
+      }
+      if (sawCleanExit) break;
+    }
+
+    // Prove the race actually produced symbolicated sanitizer output (or ran clean
+    // end-to-end) so the absence check below is meaningful.
+    expect(sawSanitizerReport || sawCleanExit).toBe(true);
+    expect(frames).toEqual([]);
+  },
+  180_000,
+);