From e99d6d73e3c65ee977ba3d8202aec89f4fb05534 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 3 Aug 2026 04:25:13 +0000 Subject: [PATCH 01/10] bundler: keep BundlerPlugin filter data alive across Worker.terminate() The bundle thread reads a plugin's onLoad/onResolve filter regex lists (via JSBundlerPlugin__anyMatches / hasOnBeforeParsePlugins / callOnBeforeParsePlugins) while a build is in flight. Those lists lived inline in the JSBundlerPlugin GC cell, so terminating a Worker that had kicked off a Bun.build() with a plugin freed them out from under the bundle thread: JSBundlerPlugin.cpp:96: runtime error: reference binding to null pointer of type 'Bun::BundlerPlugin::FilterRegExp' BundlerPlugin is now ThreadSafeRefCounted and heap-allocated separately from the GC cell. The cell holds one ref, and the Rust-side Plugin handle (what JSBundleCompletionTask / BundleV2 carry across threads) holds another, so the filter data outlives VM teardown. The Rust opaque now points at the BundlerPlugin heap allocation directly; every extern C entry point routes JS-thread work through plugin->cell() and the cross-thread ones no longer touch the cell at all. FilterRegExp::match dropped its VM& parameter: Yarr::RegularExpression uses the bytecode interpreter and the MatchingContextHolder it constructed was unused. --- src/jsc/bindings/JSBundlerPlugin.cpp | 176 ++++++++++--------- src/jsc/bindings/JSBundlerPlugin.h | 43 +++-- src/runtime/api/JSBundler.rs | 9 +- src/runtime/api/js_bundle_completion_task.rs | 10 +- test/bundler/bun-build-api.test.ts | 93 ++++++++++ 5 files changed, 227 insertions(+), 104 deletions(-) diff --git a/src/jsc/bindings/JSBundlerPlugin.cpp b/src/jsc/bindings/JSBundlerPlugin.cpp index 8c87b28467d3..6605dbbc39df 100644 --- a/src/jsc/bindings/JSBundlerPlugin.cpp +++ b/src/jsc/bindings/JSBundlerPlugin.cpp @@ -26,7 +26,6 @@ #include #include #include -#include #include "ErrorCode.h" #include "napi_external.h" #include "WebCoreJSBuiltins.h" @@ -77,7 +76,7 @@ void BundlerPlugin::NamespaceList::append(JSC::VM& vm, JSC::RegExp* filter, Stri nsGroup->append(WTF::move(filter_regexp)); } -static bool anyMatchesForNamespace(JSC::VM& vm, BundlerPlugin::NamespaceList& list, BunString* namespaceStr, BunString* path) +static bool anyMatchesForNamespace(BundlerPlugin::NamespaceList& list, BunString* namespaceStr, BunString* path) { auto namespaceString = namespaceStr ? namespaceStr->transferToWTFString() : String(); auto pathString = path->transferToWTFString(); @@ -94,19 +93,19 @@ static bool anyMatchesForNamespace(JSC::VM& vm, BundlerPlugin::NamespaceList& li auto& filters = *group; for (auto& filter : filters) { - if (filter.match(vm, pathString)) { + if (filter.match(pathString)) { return true; } } return false; } -bool BundlerPlugin::anyMatchesCrossThread(JSC::VM& vm, BunString* namespaceStr, BunString* path, bool isOnLoad) +bool BundlerPlugin::anyMatchesCrossThread(BunString* namespaceStr, BunString* path, bool isOnLoad) { if (isOnLoad) { - return anyMatchesForNamespace(vm, this->onLoad, namespaceStr, path); + return anyMatchesForNamespace(this->onLoad, namespaceStr, path); } else { - return anyMatchesForNamespace(vm, this->onResolve, namespaceStr, path); + return anyMatchesForNamespace(this->onResolve, namespaceStr, path); } } @@ -162,7 +161,7 @@ class JSBundlerPlugin final : public JSC::JSDestructibleObject { template void visitAdditionalChildrenInGCThread(Visitor&); - Bun::BundlerPlugin plugin; + Ref plugin; /// These are defined in BundlerPlugin.ts JSC::LazyProperty onLoadFunction; JSC::LazyProperty onResolveFunction; @@ -180,9 +179,10 @@ class JSBundlerPlugin final : public JSC::JSDestructibleObject { JSBundlerPlugin(JSC::VM& vm, JSC::JSGlobalObject* global, JSC::Structure* structure, void* config, BunPluginTarget target, JSBundlerPluginAddErrorCallback addError, JSBundlerPluginOnLoadAsyncCallback onLoadAsync, JSBundlerPluginOnResolveAsyncCallback onResolveAsync) : Base(vm, structure) - , plugin(BundlerPlugin(config, target, addError, onLoadAsync, onResolveAsync)) + , plugin(BundlerPlugin::create(config, target, addError, onLoadAsync, onResolveAsync)) , m_globalObject(global) { + plugin->setCell(this); } ~JSBundlerPlugin() = default; @@ -195,8 +195,8 @@ void JSBundlerPlugin::visitAdditionalChildrenInGCThread(Visitor& visitor) this->onLoadFunction.visit(visitor); this->onResolveFunction.visit(visitor); this->setupFunction.visit(visitor); - this->plugin.deferredPromises.visit(this, visitor); - this->plugin.onBeforeParseExternals.visit(this, visitor); + this->plugin->deferredPromises.visit(this, visitor); + this->plugin->onBeforeParseExternals.visit(this, visitor); } template @@ -225,7 +225,7 @@ const JSC::ClassInfo JSBundlerPlugin::s_info = { "BundlerPlugin"_s, &Base::s_inf JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_addFilter, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) { JSBundlerPlugin* thisObject = uncheckedDowncast(callFrame->thisValue()); - if (thisObject->plugin.tombstoned) { + if (thisObject->plugin->tombstoned) { return JSC::JSValue::encode(JSC::jsUndefined()); } @@ -240,9 +240,9 @@ JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_addFilter, (JSC::JSGlobalObject unsigned index = 0; if (isOnLoad) { - thisObject->plugin.onLoad.append(vm, regExp->regExp(), namespaceStr, index); + thisObject->plugin->onLoad.append(vm, regExp->regExp(), namespaceStr, index); } else { - thisObject->plugin.onResolve.append(vm, regExp->regExp(), namespaceStr, index); + thisObject->plugin->onResolve.append(vm, regExp->regExp(), namespaceStr, index); } return JSC::JSValue::encode(JSC::jsUndefined()); @@ -281,14 +281,13 @@ void BundlerPlugin::NativePluginList::append(JSC::VM& vm, JSC::RegExp* filter, S } } -bool BundlerPlugin::FilterRegExp::match(JSC::VM& vm, const String& path) +bool BundlerPlugin::FilterRegExp::match(const String& path) { WTF::Locker locker { lock }; - Yarr::MatchingContextHolder regExpContext(vm, nullptr, Yarr::MatchFrom::CompilerThread); return regex.match(path) != -1; } -int BundlerPlugin::NativePluginList::call(JSC::VM& vm, BundlerPlugin* plugin, int* shouldContinue, void* bunContextPtr, const BunString* namespaceStr, const BunString* pathString, OnBeforeParseArguments* onBeforeParseArgs, OnBeforeParseResult* onBeforeParseResult) +int BundlerPlugin::NativePluginList::call(BundlerPlugin* plugin, int* shouldContinue, void* bunContextPtr, const BunString* namespaceStr, const BunString* pathString, OnBeforeParseArguments* onBeforeParseArgs, OnBeforeParseResult* onBeforeParseResult) { unsigned index = 0; auto* groupPtr = this->group(namespaceStr->toWTFString(BunString::ZeroCopy), index); @@ -311,7 +310,7 @@ int BundlerPlugin::NativePluginList::call(JSC::VM& vm, BundlerPlugin* plugin, in OnBeforeParseResult__reset(onBeforeParseResult); } - if (filters[i].match(vm, path)) { + if (filters[i].match(path)) { Bun::NapiExternal* external = callbacks[i].external; ASSERT(onBeforeParseArgs != nullptr); if (external) { @@ -341,7 +340,7 @@ JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_onBeforeParse, (JSC::JSGlobalOb auto& vm = JSC::getVM(globalObject); auto scope = DECLARE_THROW_SCOPE(vm); JSBundlerPlugin* thisObject = uncheckedDowncast(callFrame->thisValue()); - if (thisObject->plugin.tombstoned) { + if (thisObject->plugin->tombstoned) { return JSC::JSValue::encode(JSC::jsUndefined()); } @@ -404,10 +403,10 @@ JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_onBeforeParse, (JSC::JSGlobalOb Bun::throwError(globalObject, scope, ErrorCode::ERR_INVALID_ARG_TYPE, "Expected external (3rd argument) to be a NAPI external"_s); return {}; } - thisObject->plugin.onBeforeParseExternals.append(vm, thisObject, externalPtr); + thisObject->plugin->onBeforeParseExternals.append(vm, thisObject, externalPtr); } - thisObject->plugin.onBeforeParse.append(vm, newRegexp, namespaceStr, callback, native_plugin_name ? *native_plugin_name : nullptr, externalPtr); + thisObject->plugin->onBeforeParse.append(vm, newRegexp, namespaceStr, callback, native_plugin_name ? *native_plugin_name : nullptr, externalPtr); return JSC::JSValue::encode(JSC::jsUndefined()); } @@ -415,10 +414,10 @@ JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_onBeforeParse, (JSC::JSGlobalOb JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_addError, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) { JSBundlerPlugin* thisObject = uncheckedDowncast(callFrame->thisValue()); - if (!thisObject->plugin.tombstoned) { - thisObject->plugin.addError( + if (!thisObject->plugin->tombstoned) { + thisObject->plugin->addError( UNWRAP_BUNDLER_PLUGIN(callFrame), - thisObject, + thisObject->plugin.ptr(), JSValue::encode(callFrame->argument(1)), JSValue::encode(callFrame->argument(2))); } @@ -428,10 +427,10 @@ JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_addError, (JSC::JSGlobalObject JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_onLoadAsync, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) { JSBundlerPlugin* thisObject = uncheckedDowncast(callFrame->thisValue()); - if (!thisObject->plugin.tombstoned) { - thisObject->plugin.onLoadAsync( + if (!thisObject->plugin->tombstoned) { + thisObject->plugin->onLoadAsync( UNWRAP_BUNDLER_PLUGIN(callFrame), - thisObject->plugin.config, + thisObject->plugin->config, JSValue::encode(callFrame->argument(1)), JSValue::encode(callFrame->argument(2))); } @@ -441,10 +440,10 @@ JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_onLoadAsync, (JSC::JSGlobalObje JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_onResolveAsync, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) { JSBundlerPlugin* thisObject = uncheckedDowncast(callFrame->thisValue()); - if (!thisObject->plugin.tombstoned) { - thisObject->plugin.onResolveAsync( + if (!thisObject->plugin->tombstoned) { + thisObject->plugin->onResolveAsync( UNWRAP_BUNDLER_PLUGIN(callFrame), - thisObject->plugin.config, + thisObject->plugin->config, JSValue::encode(callFrame->argument(1)), JSValue::encode(callFrame->argument(2)), JSValue::encode(callFrame->argument(3))); @@ -453,13 +452,14 @@ JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_onResolveAsync, (JSC::JSGlobalO return JSC::JSValue::encode(JSC::jsUndefined()); } -extern "C" JSC::EncodedJSValue JSBundlerPlugin__appendDeferPromise(Bun::JSBundlerPlugin* pluginObject) +extern "C" JSC::EncodedJSValue JSBundlerPlugin__appendDeferPromise(Bun::BundlerPlugin* plugin) { - auto* vm = &pluginObject->vm(); - auto* globalObject = pluginObject->globalObject(); + auto* cell = plugin->cell(); + auto* vm = &cell->vm(); + auto* globalObject = cell->globalObject(); JSPromise* ret = JSPromise::create(*vm, globalObject->promiseStructure()); - pluginObject->plugin.deferredPromises.append(*vm, pluginObject, ret); + plugin->deferredPromises.append(*vm, cell, ret); return JSC::JSValue::encode(ret); } @@ -507,16 +507,17 @@ void JSBundlerPlugin::finishCreation(JSC::VM& vm) reifyStaticProperties(vm, JSBundlerPlugin::info(), JSBundlerPluginHashTable, *this); } -extern "C" bool JSBundlerPlugin__anyMatches(Bun::JSBundlerPlugin* pluginObject, BunString* namespaceString, BunString* path, bool isOnLoad) +extern "C" bool JSBundlerPlugin__anyMatches(Bun::BundlerPlugin* plugin, BunString* namespaceString, BunString* path, bool isOnLoad) { - return pluginObject->plugin.anyMatchesCrossThread(pluginObject->vm(), namespaceString, path, isOnLoad); + return plugin->anyMatchesCrossThread(namespaceString, path, isOnLoad); } -extern "C" void JSBundlerPlugin__matchOnLoad(Bun::JSBundlerPlugin* plugin, BunString* namespaceString, BunString* path, void* context, uint8_t defaultLoaderId, bool isServerSide) +extern "C" void JSBundlerPlugin__matchOnLoad(Bun::BundlerPlugin* plugin, BunString* namespaceString, BunString* path, void* context, uint8_t defaultLoaderId, bool isServerSide) { - JSC::JSGlobalObject* globalObject = plugin->globalObject(); + auto* cell = plugin->cell(); + JSC::JSGlobalObject* globalObject = cell->globalObject(); - JSFunction* function = plugin->onLoadFunction.get(plugin); + JSFunction* function = cell->onLoadFunction.get(cell); if (!function) [[unlikely]] return; @@ -525,7 +526,7 @@ extern "C" void JSBundlerPlugin__matchOnLoad(Bun::JSBundlerPlugin* plugin, BunSt if (callData.type == JSC::CallData::Type::None) [[unlikely]] return; - auto scope = DECLARE_TOP_EXCEPTION_SCOPE(plugin->vm()); + auto scope = DECLARE_TOP_EXCEPTION_SCOPE(cell->vm()); JSC::MarkedArgumentBuffer arguments; arguments.append(WRAP_BUNDLER_PLUGIN(context)); arguments.append(path->transferToJS(globalObject)); @@ -535,14 +536,14 @@ extern "C" void JSBundlerPlugin__matchOnLoad(Bun::JSBundlerPlugin* plugin, BunSt arguments.append(JSC::jsNumber(defaultLoaderId)); arguments.append(JSC::jsBoolean(isServerSide)); - call(globalObject, function, callData, plugin, arguments); + call(globalObject, function, callData, cell, arguments); if (scope.exception()) [[unlikely]] { auto exception = scope.exception(); (void)scope.tryClearException(); - if (!plugin->plugin.tombstoned) { + if (!plugin->tombstoned) { // which = 1 (Load). JSBundlerPlugin__addError casts ctx based on this value. - plugin->plugin.addError( + plugin->addError( context, plugin, JSC::JSValue::encode(exception), @@ -551,11 +552,12 @@ extern "C" void JSBundlerPlugin__matchOnLoad(Bun::JSBundlerPlugin* plugin, BunSt } } -extern "C" void JSBundlerPlugin__matchOnResolve(Bun::JSBundlerPlugin* plugin, BunString* namespaceString, BunString* path, BunString* importer, void* context, uint8_t kindId) +extern "C" void JSBundlerPlugin__matchOnResolve(Bun::BundlerPlugin* plugin, BunString* namespaceString, BunString* path, BunString* importer, void* context, uint8_t kindId) { - JSC::JSGlobalObject* globalObject = plugin->globalObject(); + auto* cell = plugin->cell(); + JSC::JSGlobalObject* globalObject = cell->globalObject(); - JSFunction* function = plugin->onResolveFunction.get(plugin); + JSFunction* function = cell->onResolveFunction.get(cell); if (!function) [[unlikely]] return; @@ -564,7 +566,7 @@ extern "C" void JSBundlerPlugin__matchOnResolve(Bun::JSBundlerPlugin* plugin, Bu if (callData.type == JSC::CallData::Type::None) [[unlikely]] return; - auto scope = DECLARE_TOP_EXCEPTION_SCOPE(plugin->vm()); + auto scope = DECLARE_TOP_EXCEPTION_SCOPE(cell->vm()); JSC::MarkedArgumentBuffer arguments; arguments.append(path->transferToJS(globalObject)); RETURN_IF_EXCEPTION(scope, void()); @@ -575,14 +577,14 @@ extern "C" void JSBundlerPlugin__matchOnResolve(Bun::JSBundlerPlugin* plugin, Bu arguments.append(WRAP_BUNDLER_PLUGIN(context)); arguments.append(JSC::jsNumber(kindId)); - call(globalObject, function, callData, plugin, arguments); + call(globalObject, function, callData, cell, arguments); if (scope.exception()) [[unlikely]] { auto exception = JSValue(scope.exception()); (void)scope.tryClearException(); - if (!plugin->plugin.tombstoned) { + if (!plugin->tombstoned) { // which = 0 (Resolve). JSBundlerPlugin__addError casts ctx based on this value. - plugin->plugin.addError( + plugin->addError( context, plugin, JSC::JSValue::encode(exception), @@ -592,9 +594,12 @@ extern "C" void JSBundlerPlugin__matchOnResolve(Bun::JSBundlerPlugin* plugin, Bu } } -extern "C" Bun::JSBundlerPlugin* JSBundlerPlugin__create(Zig::GlobalObject* globalObject, BunPluginTarget target) +// Returns the heap-allocated BundlerPlugin with a +1 ref for the caller. The owning +// JSBundlerPlugin GC cell (which holds the other ref) is gcProtect'd here and released +// in JSBundlerPlugin__destroy. +extern "C" Bun::BundlerPlugin* JSBundlerPlugin__create(Zig::GlobalObject* globalObject, BunPluginTarget target) { - return JSBundlerPlugin::create( + auto* cell = JSBundlerPlugin::create( globalObject->vm(), globalObject, // TODO: cache this structure on the global object @@ -604,16 +609,27 @@ extern "C" Bun::JSBundlerPlugin* JSBundlerPlugin__create(Zig::GlobalObject* glob globalObject->objectPrototype()), nullptr, target); + JSC::gcProtect(cell); + cell->plugin->ref(); + return cell->plugin.ptr(); } -extern "C" JSC::EncodedJSValue JSBundlerPlugin__loadAndResolvePluginsForServe(Bun::JSBundlerPlugin* plugin, JSC::EncodedJSValue encodedPlugins, JSC::EncodedJSValue encodedBunfigFolder) +extern "C" void JSBundlerPlugin__destroy(Bun::BundlerPlugin* plugin) { - auto& vm = plugin->vm(); + plugin->tombstone(); + JSC::gcUnprotect(plugin->cell()); + plugin->deref(); +} + +extern "C" JSC::EncodedJSValue JSBundlerPlugin__loadAndResolvePluginsForServe(Bun::BundlerPlugin* plugin, JSC::EncodedJSValue encodedPlugins, JSC::EncodedJSValue encodedBunfigFolder) +{ + auto* cell = plugin->cell(); + auto& vm = cell->vm(); auto scope = DECLARE_THROW_SCOPE(vm); - auto* loadAndResolvePluginsForServeBuiltinFn = JSC::JSFunction::create(vm, plugin->globalObject(), WebCore::bundlerPluginLoadAndResolvePluginsForServeCodeGenerator(vm), plugin->globalObject()); + auto* loadAndResolvePluginsForServeBuiltinFn = JSC::JSFunction::create(vm, cell->globalObject(), WebCore::bundlerPluginLoadAndResolvePluginsForServeCodeGenerator(vm), cell->globalObject()); - auto* runSetupFn = plugin->setupFunction.get(plugin); + auto* runSetupFn = cell->setupFunction.get(cell); JSC::CallData callData = JSC::getCallData(loadAndResolvePluginsForServeBuiltinFn); if (callData.type == JSC::CallData::Type::None) [[unlikely]] @@ -624,21 +640,22 @@ extern "C" JSC::EncodedJSValue JSBundlerPlugin__loadAndResolvePluginsForServe(Bu arguments.append(JSValue::decode(encodedBunfigFolder)); arguments.append(runSetupFn); - RELEASE_AND_RETURN(scope, JSC::JSValue::encode(JSC::profiledCall(plugin->globalObject(), ProfilingReason::API, loadAndResolvePluginsForServeBuiltinFn, callData, plugin, arguments))); + RELEASE_AND_RETURN(scope, JSC::JSValue::encode(JSC::profiledCall(cell->globalObject(), ProfilingReason::API, loadAndResolvePluginsForServeBuiltinFn, callData, cell, arguments))); } extern "C" JSC::EncodedJSValue JSBundlerPlugin__runSetupFunction( - Bun::JSBundlerPlugin* plugin, + Bun::BundlerPlugin* plugin, JSC::EncodedJSValue encodedSetupFunction, JSC::EncodedJSValue encodedConfig, JSC::EncodedJSValue encodedOnstartPromisesArray, JSC::EncodedJSValue encodedIsLast, JSC::EncodedJSValue encodedIsBake) { - auto& vm = plugin->vm(); + auto* cell = plugin->cell(); + auto& vm = cell->vm(); auto scope = DECLARE_THROW_SCOPE(vm); - auto* setupFunction = plugin->setupFunction.get(plugin); + auto* setupFunction = cell->setupFunction.get(cell); if (!setupFunction) [[unlikely]] return JSValue::encode(jsUndefined()); @@ -654,25 +671,26 @@ extern "C" JSC::EncodedJSValue JSBundlerPlugin__runSetupFunction( arguments.append(JSValue::decode(encodedIsBake)); auto* lexicalGlobalObject = uncheckedDowncast(JSValue::decode(encodedSetupFunction))->globalObject(); - auto result = JSC::profiledCall(lexicalGlobalObject, ProfilingReason::API, setupFunction, callData, plugin, arguments); + auto result = JSC::profiledCall(lexicalGlobalObject, ProfilingReason::API, setupFunction, callData, cell, arguments); RETURN_IF_EXCEPTION(scope, {}); // should be able to use RELEASE_AND_RETURN, no? observed it returning undefined with exception active return JSValue::encode(result); } -extern "C" void JSBundlerPlugin__setConfig(Bun::JSBundlerPlugin* plugin, void* config) +extern "C" void JSBundlerPlugin__setConfig(Bun::BundlerPlugin* plugin, void* config) { - plugin->plugin.config = config; + plugin->config = config; } -extern "C" void JSBundlerPlugin__drainDeferred(Bun::JSBundlerPlugin* pluginObject, bool rejected) +extern "C" void JSBundlerPlugin__drainDeferred(Bun::BundlerPlugin* plugin, bool rejected) { - auto* globalObject = pluginObject->globalObject(); + auto* cell = plugin->cell(); + auto* globalObject = cell->globalObject(); MarkedArgumentBuffer arguments; - pluginObject->plugin.deferredPromises.drainTo(pluginObject, arguments); + plugin->deferredPromises.drainTo(cell, arguments); ASSERT(!arguments.hasOverflowed()); - auto& vm = pluginObject->vm(); + auto& vm = cell->vm(); auto scope = DECLARE_THROW_SCOPE(vm); for (auto promiseValue : arguments) { JSPromise* promise = uncheckedDowncast(promiseValue); @@ -686,16 +704,12 @@ extern "C" void JSBundlerPlugin__drainDeferred(Bun::JSBundlerPlugin* pluginObjec RETURN_IF_EXCEPTION(scope, ); } -extern "C" void JSBundlerPlugin__tombstone(Bun::JSBundlerPlugin* plugin) -{ - plugin->plugin.tombstone(); -} - -extern "C" JSC::EncodedJSValue JSBundlerPlugin__runOnEndCallbacks(Bun::JSBundlerPlugin* plugin, JSC::EncodedJSValue encodedBuildPromise, JSC::EncodedJSValue encodedBuildResult, JSC::EncodedJSValue encodedRejection) +extern "C" JSC::EncodedJSValue JSBundlerPlugin__runOnEndCallbacks(Bun::BundlerPlugin* plugin, JSC::EncodedJSValue encodedBuildPromise, JSC::EncodedJSValue encodedBuildResult, JSC::EncodedJSValue encodedRejection) { - auto& vm = plugin->vm(); + auto* cell = plugin->cell(); + auto& vm = cell->vm(); auto scope = DECLARE_THROW_SCOPE(vm); - auto* globalObject = plugin->globalObject(); + auto* globalObject = cell->globalObject(); // TODO: have a prototype for JSBundlerPlugin that this is put on instead of re-creating the function on each usage auto* runOnEndCallbacksFn = JSC::JSFunction::create(vm, globalObject, @@ -713,14 +727,14 @@ extern "C" JSC::EncodedJSValue JSBundlerPlugin__runOnEndCallbacks(Bun::JSBundler // TODO: use AsyncContextFrame? auto result - = JSC::profiledCall(globalObject, ProfilingReason::API, runOnEndCallbacksFn, callData, plugin, arguments); + = JSC::profiledCall(globalObject, ProfilingReason::API, runOnEndCallbacksFn, callData, cell, arguments); RETURN_IF_EXCEPTION(scope, {}); return JSValue::encode(result); } extern "C" int JSBundlerPlugin__callOnBeforeParsePlugins( - Bun::JSBundlerPlugin* plugin, + Bun::BundlerPlugin* plugin, void* bunContextPtr, const BunString* namespaceStr, const BunString* pathString, @@ -728,17 +742,17 @@ extern "C" int JSBundlerPlugin__callOnBeforeParsePlugins( OnBeforeParseResult* onBeforeParseResult, int* shouldContinue) { - return plugin->plugin.onBeforeParse.call(plugin->vm(), &plugin->plugin, shouldContinue, bunContextPtr, namespaceStr, pathString, onBeforeParseArgs, onBeforeParseResult); + return plugin->onBeforeParse.call(plugin, shouldContinue, bunContextPtr, namespaceStr, pathString, onBeforeParseArgs, onBeforeParseResult); } -extern "C" int JSBundlerPlugin__hasOnBeforeParsePlugins(Bun::JSBundlerPlugin* plugin) +extern "C" int JSBundlerPlugin__hasOnBeforeParsePlugins(Bun::BundlerPlugin* plugin) { - return plugin->plugin.onBeforeParse.namespaceCallbacks.size() > 0 || plugin->plugin.onBeforeParse.fileCallbacks.size() > 0; + return plugin->onBeforeParse.namespaceCallbacks.size() > 0 || plugin->onBeforeParse.fileCallbacks.size() > 0; } -extern "C" JSC::JSGlobalObject* JSBundlerPlugin__globalObject(Bun::JSBundlerPlugin* plugin) +extern "C" JSC::JSGlobalObject* JSBundlerPlugin__globalObject(Bun::BundlerPlugin* plugin) { - return plugin->m_globalObject; + return plugin->cell()->m_globalObject; } } // namespace Bun diff --git a/src/jsc/bindings/JSBundlerPlugin.h b/src/jsc/bindings/JSBundlerPlugin.h index 9ab506e4654c..4a30d2936059 100644 --- a/src/jsc/bindings/JSBundlerPlugin.h +++ b/src/jsc/bindings/JSBundlerPlugin.h @@ -7,6 +7,7 @@ #include #include "napi_external.h" #include +#include #include "WriteBarrierList.h" typedef void (*JSBundlerPluginAddErrorCallback)(void*, void*, JSC::EncodedJSValue, JSC::EncodedJSValue); @@ -18,7 +19,13 @@ namespace Bun { using namespace JSC; -class BundlerPlugin final { +class JSBundlerPlugin; + +// Heap-allocated and ref-counted separately from the JSBundlerPlugin GC cell so the +// bundle thread can read filter lists after a Worker's VM (and its JSC heap) is torn +// down. The owning JSBundlerPlugin cell holds one ref; the Rust-side Plugin handle +// holds another. +class BundlerPlugin final : public ThreadSafeRefCounted { public: /// In native plugins, the regular expression could be called concurrently on multiple threads. /// Therefore, we need a mutex to synchronize access. @@ -41,7 +48,7 @@ class BundlerPlugin final { { } - bool match(JSC::VM& vm, const String& path); + bool match(const String& path); }; class NamespaceList { @@ -95,7 +102,7 @@ class BundlerPlugin final { PerNamespaceCallbackList fileCallbacks = {}; Vector namespaceCallbacks = {}; - int call(JSC::VM& vm, BundlerPlugin* plugin, int* shouldContinue, void* bunContextPtr, const BunString* namespaceStr, const BunString* pathString, OnBeforeParseArguments* onBeforeParseArgs, OnBeforeParseResult* onBeforeParseResult); + 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, NapiExternal* external); Vector* group(const String& namespaceStr, unsigned& index) @@ -118,18 +125,17 @@ 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 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 = {}; @@ -145,6 +151,19 @@ class BundlerPlugin final { 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 diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index 768cd971b324..9b06ebc015a8 100644 --- a/src/runtime/api/JSBundler.rs +++ b/src/runtime/api/JSBundler.rs @@ -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, @@ -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 { diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index 063ea3a45582..4e05c63e69b8 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -190,11 +190,11 @@ impl JSBundleCompletionTask { /// /// Centralises the `Option → 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 diff --git a/test/bundler/bun-build-api.test.ts b/test/bundler/bun-build-api.test.ts index 951bfb5cd694..92c1555efb1c 100644 --- a/test/bundler/bun-build-api.test.ts +++ b/test/bundler/bun-build-api.test.ts @@ -1525,3 +1525,96 @@ 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 = {}; + 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]; + for (let r = 0; r < 40; 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. + const ATTEMPTS = 8; + const frames: string[] = []; + 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; + + 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")}`); + } + if (stdout.includes("ok") && stderr === "") { + // Reached the end with no sanitizer report at all; nothing left to probe. + break; + } + } + + expect(frames).toEqual([]); + }, + 120_000, +); From 889519c49c93f26a1f063d6f94552c8457638378 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 04:27:15 +0000 Subject: [PATCH 02/10] [autofix.ci] apply automated fixes --- test/bundler/bun-build-api.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/bundler/bun-build-api.test.ts b/test/bundler/bun-build-api.test.ts index 92c1555efb1c..04e060d1fb28 100644 --- a/test/bundler/bun-build-api.test.ts +++ b/test/bundler/bun-build-api.test.ts @@ -1602,9 +1602,7 @@ test.skipIf(!isASAN)( const [stdout, stderr] = await Promise.all([proc.stdout.text(), proc.stderr.text()]); await proc.exited; - const pluginFrames = stderr - .split("\n") - .filter(l => /JSBundlerPlugin\.cpp|BundlerPlugin::|FilterRegExp/.test(l)); + 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")}`); } From c2fa3121552aec4b0d9037b8bacdb4a01e5f88e6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 3 Aug 2026 04:40:56 +0000 Subject: [PATCH 03/10] address review: snapshot NapiExternal value() at append time; trim comments; update stale bake prose --- src/jsc/bindings/JSBundlerPlugin.cpp | 20 ++++++++------------ src/jsc/bindings/JSBundlerPlugin.h | 10 ++++++---- src/runtime/bake/bake_body.rs | 8 +++----- 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/jsc/bindings/JSBundlerPlugin.cpp b/src/jsc/bindings/JSBundlerPlugin.cpp index 6605dbbc39df..dabee8d899ad 100644 --- a/src/jsc/bindings/JSBundlerPlugin.cpp +++ b/src/jsc/bindings/JSBundlerPlugin.cpp @@ -248,7 +248,7 @@ JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_addFilter, (JSC::JSGlobalObject return JSC::JSValue::encode(JSC::jsUndefined()); } -void BundlerPlugin::NativePluginList::append(JSC::VM& vm, JSC::RegExp* filter, String& namespaceString, JSBundlerPluginNativeOnBeforeParseCallback callback, const char* name, NapiExternal* external) +void BundlerPlugin::NativePluginList::append(JSC::VM& vm, JSC::RegExp* filter, String& namespaceString, JSBundlerPluginNativeOnBeforeParseCallback callback, const char* name, void* externalValue) { unsigned index = 0; @@ -270,14 +270,14 @@ void BundlerPlugin::NativePluginList::append(JSC::VM& vm, JSC::RegExp* filter, S if (index == std::numeric_limits::max()) { this->fileCallbacks.append(NativePluginCallback { callback, - external, + externalValue, name, }); } else { if (this->namespaceCallbacks.size() <= index) { this->namespaceCallbacks.grow(index + 1); } - this->namespaceCallbacks[index].append(NativePluginCallback { callback, external, name }); + this->namespaceCallbacks[index].append(NativePluginCallback { callback, externalValue, name }); } } @@ -311,13 +311,8 @@ int BundlerPlugin::NativePluginList::call(BundlerPlugin* plugin, int* shouldCont } if (filters[i].match(path)) { - Bun::NapiExternal* external = callbacks[i].external; ASSERT(onBeforeParseArgs != nullptr); - if (external) { - onBeforeParseArgs->external = external->value(); - } else { - onBeforeParseArgs->external = nullptr; - } + onBeforeParseArgs->external = callbacks[i].externalValue; JSBundlerPluginNativeOnBeforeParseCallback callback = callbacks[i].callback; const char* name = callbacks[i].name ? callbacks[i].name : ""; @@ -396,17 +391,18 @@ JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_onBeforeParse, (JSC::JSGlobalOb JSBundlerPluginNativeOnBeforeParseCallback callback = reinterpret_cast(on_before_parse_symbol_ptr); JSC::JSValue external = callFrame->argument(4); - NapiExternal* externalPtr = nullptr; + void* externalValue = nullptr; if (!external.isUndefinedOrNull()) { - externalPtr = dynamicDowncast(external); + NapiExternal* externalPtr = dynamicDowncast(external); if (!externalPtr) [[unlikely]] { Bun::throwError(globalObject, scope, ErrorCode::ERR_INVALID_ARG_TYPE, "Expected external (3rd argument) to be a NAPI external"_s); return {}; } thisObject->plugin->onBeforeParseExternals.append(vm, thisObject, externalPtr); + externalValue = externalPtr->value(); } - thisObject->plugin->onBeforeParse.append(vm, newRegexp, namespaceStr, callback, native_plugin_name ? *native_plugin_name : nullptr, externalPtr); + thisObject->plugin->onBeforeParse.append(vm, newRegexp, namespaceStr, callback, native_plugin_name ? *native_plugin_name : nullptr, externalValue); return JSC::JSValue::encode(JSC::jsUndefined()); } diff --git a/src/jsc/bindings/JSBundlerPlugin.h b/src/jsc/bindings/JSBundlerPlugin.h index 4a30d2936059..65349b8ce4d1 100644 --- a/src/jsc/bindings/JSBundlerPlugin.h +++ b/src/jsc/bindings/JSBundlerPlugin.h @@ -81,7 +81,9 @@ class BundlerPlugin final : public ThreadSafeRefCounted { 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 /// @@ -103,7 +105,7 @@ class BundlerPlugin final : public ThreadSafeRefCounted { Vector namespaceCallbacks = {}; 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, NapiExternal* external); + void append(JSC::VM& vm, JSC::RegExp* filter, String& namespaceString, JSBundlerPluginNativeOnBeforeParseCallback callback, const char* name, void* externalValue); Vector* group(const String& namespaceStr, unsigned& index) { @@ -142,8 +144,8 @@ class BundlerPlugin final : public ThreadSafeRefCounted { BunPluginTarget target { BunPluginTargetBrowser }; WriteBarrierList deferredPromises = {}; - // The raw `NapiExternal*` stored in `NativePluginCallback` is dereferenced - // off the JS thread; this list keeps those cells alive for GC. + // Keeps each registered NapiExternal alive for GC so its finalizer does not + // release the user context while the build is still using it. WriteBarrierList onBeforeParseExternals = {}; JSBundlerPluginAddErrorCallback addError; diff --git a/src/runtime/bake/bake_body.rs b/src/runtime/bake/bake_body.rs index b657063d83ea..787f141d7c33 100644 --- a/src/runtime/bake/bake_body.rs +++ b/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, From 4b52831420371f0df77e373b0811d8c26b40a9b5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 3 Aug 2026 04:43:03 +0000 Subject: [PATCH 04/10] trim comments flagged by comment-cop; tighten test loop --- src/jsc/bindings/JSBundlerPlugin.cpp | 4 +--- src/jsc/bindings/JSBundlerPlugin.h | 6 ++---- test/bundler/bun-build-api.test.ts | 6 ++---- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/jsc/bindings/JSBundlerPlugin.cpp b/src/jsc/bindings/JSBundlerPlugin.cpp index dabee8d899ad..36d1d197cafc 100644 --- a/src/jsc/bindings/JSBundlerPlugin.cpp +++ b/src/jsc/bindings/JSBundlerPlugin.cpp @@ -590,9 +590,7 @@ extern "C" void JSBundlerPlugin__matchOnResolve(Bun::BundlerPlugin* plugin, BunS } } -// Returns the heap-allocated BundlerPlugin with a +1 ref for the caller. The owning -// JSBundlerPlugin GC cell (which holds the other ref) is gcProtect'd here and released -// in JSBundlerPlugin__destroy. +// Returns +1 BundlerPlugin* and gcProtect's its owning cell; both released in __destroy. extern "C" Bun::BundlerPlugin* JSBundlerPlugin__create(Zig::GlobalObject* globalObject, BunPluginTarget target) { auto* cell = JSBundlerPlugin::create( diff --git a/src/jsc/bindings/JSBundlerPlugin.h b/src/jsc/bindings/JSBundlerPlugin.h index 65349b8ce4d1..18575a4ef46b 100644 --- a/src/jsc/bindings/JSBundlerPlugin.h +++ b/src/jsc/bindings/JSBundlerPlugin.h @@ -21,10 +21,8 @@ using namespace JSC; class JSBundlerPlugin; -// Heap-allocated and ref-counted separately from the JSBundlerPlugin GC cell so the -// bundle thread can read filter lists after a Worker's VM (and its JSC heap) is torn -// down. The owning JSBundlerPlugin cell holds one ref; the Rust-side Plugin handle -// holds another. +// 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 { public: /// In native plugins, the regular expression could be called concurrently on multiple threads. diff --git a/test/bundler/bun-build-api.test.ts b/test/bundler/bun-build-api.test.ts index 04e060d1fb28..a894f8e53d86 100644 --- a/test/bundler/bun-build-api.test.ts +++ b/test/bundler/bun-build-api.test.ts @@ -1605,14 +1605,12 @@ test.skipIf(!isASAN)( 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")}`); - } - if (stdout.includes("ok") && stderr === "") { - // Reached the end with no sanitizer report at all; nothing left to probe. break; } + if (stdout.includes("ok") && stderr === "") break; } expect(frames).toEqual([]); }, - 120_000, + 180_000, ); From 6c34a4f69b7d65e4d3d89aa9ab3c25b87a5b172a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 3 Aug 2026 05:20:52 +0000 Subject: [PATCH 05/10] review: wall-clock-bound the test attempt, add positive sanitizer assertion, fix stale Plugin-opaque comments --- src/bundler/bundle_v2.rs | 15 ++++++++------- src/jsc/bindings/JSBundlerPlugin.h | 3 +-- src/runtime/api/JSBundler.rs | 8 ++++---- src/runtime/bake/mod.rs | 10 +++++----- test/bundler/bun-build-api.test.ts | 17 ++++++++++++++--- 5 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 0d220609b934..e7ce22a956dc 100644 --- a/src/bundler/bundle_v2.rs +++ b/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 diff --git a/src/jsc/bindings/JSBundlerPlugin.h b/src/jsc/bindings/JSBundlerPlugin.h index 18575a4ef46b..b53a846c5e61 100644 --- a/src/jsc/bindings/JSBundlerPlugin.h +++ b/src/jsc/bindings/JSBundlerPlugin.h @@ -142,8 +142,7 @@ class BundlerPlugin final : public ThreadSafeRefCounted { BunPluginTarget target { BunPluginTargetBrowser }; WriteBarrierList deferredPromises = {}; - // Keeps each registered NapiExternal alive for GC so its finalizer does not - // release the user context while the build is still using it. + // Roots each registered NapiExternal against normal GC while the build runs. WriteBarrierList onBeforeParseExternals = {}; JSBundlerPluginAddErrorCallback addError; diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index 9b06ebc015a8..62e7ef6736a7 100644 --- a/src/runtime/api/JSBundler.rs +++ b/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; @@ -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( diff --git a/src/runtime/bake/mod.rs b/src/runtime/bake/mod.rs index 12136842c27d..ca314a2bed7a 100644 --- a/src/runtime/bake/mod.rs +++ b/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::*; } diff --git a/test/bundler/bun-build-api.test.ts b/test/bundler/bun-build-api.test.ts index a894f8e53d86..cbab810d06cc 100644 --- a/test/bundler/bun-build-api.test.ts +++ b/test/bundler/bun-build-api.test.ts @@ -1571,7 +1571,8 @@ test.skipIf(!isASAN)( files["parent.mjs"] = ` import { Worker } from "node:worker_threads"; const dir = process.argv[2]; - for (let r = 0; r < 40; r++) { + 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 } }); @@ -1589,9 +1590,13 @@ test.skipIf(!isASAN)( // 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. + // 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)], @@ -1602,14 +1607,20 @@ test.skipIf(!isASAN)( 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 (stdout.includes("ok") && stderr === "") 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, From 85687ea9b471f7ac53550d928e899982bb9e7935 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 3 Aug 2026 05:31:08 +0000 Subject: [PATCH 06/10] review: pass *mut Plugin to __destroy; capture exitCode/signal/stderrTail in test diagnostics --- src/runtime/api/JSBundler.rs | 7 +++++-- test/bundler/bun-build-api.test.ts | 10 ++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index 62e7ef6736a7..4032347fcde2 100644 --- a/src/runtime/api/JSBundler.rs +++ b/src/runtime/api/JSBundler.rs @@ -1635,7 +1635,7 @@ pub mod js_bundler { global: &JSGlobalObject, target: jsc::BunPluginTarget, ) -> *mut Plugin; - safe fn JSBundlerPlugin__destroy(plugin: &Plugin); + fn JSBundlerPlugin__destroy(plugin: *mut Plugin); safe fn JSBundlerPlugin__runOnEndCallbacks( plugin: &mut Plugin, build_promise: JSValue, @@ -1737,7 +1737,10 @@ pub mod js_bundler { fn destroy(this: *mut Plugin) { jsc::mark_binding(); - JSBundlerPlugin__destroy(Plugin::opaque_ref(this)); + debug_assert!(!this.is_null()); + // SAFETY: `this` is the +1 handle returned by `JSBundlerPlugin__create`; + // the callee releases it and may free the allocation. + unsafe { JSBundlerPlugin__destroy(this) }; } fn global_object(&self) -> &JSGlobalObject { diff --git a/test/bundler/bun-build-api.test.ts b/test/bundler/bun-build-api.test.ts index cbab810d06cc..32d59d0f9a63 100644 --- a/test/bundler/bun-build-api.test.ts +++ b/test/bundler/bun-build-api.test.ts @@ -1595,6 +1595,7 @@ test.skipIf(!isASAN)( // expect(exitCode).toBe(0). const ATTEMPTS = 8; const frames: string[] = []; + const outcomes: Array<{ exitCode: number | null; signalCode: string | null; stderrTail: string }> = []; let sawSanitizerReport = false; let sawCleanExit = false; for (let attempt = 0; attempt < ATTEMPTS; attempt++) { @@ -1606,9 +1607,14 @@ test.skipIf(!isASAN)( }); const [stdout, stderr] = await Promise.all([proc.stdout.text(), proc.stderr.text()]); await proc.exited; + outcomes.push({ + exitCode: proc.exitCode, + signalCode: proc.signalCode, + stderrTail: stderr.split("\n").slice(-20).join("\n"), + }); if (/AddressSanitizer|runtime error:|SUMMARY: /.test(stderr)) sawSanitizerReport = true; - if (stdout.includes("ok") && stderr === "") sawCleanExit = true; + if (stdout.includes("ok") && stderr.trim() === "") sawCleanExit = true; const pluginFrames = stderr.split("\n").filter(l => /JSBundlerPlugin\.cpp|BundlerPlugin::|FilterRegExp/.test(l)); if (pluginFrames.length > 0) { @@ -1620,7 +1626,7 @@ test.skipIf(!isASAN)( // 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({ meaningful: sawSanitizerReport || sawCleanExit, outcomes }).toMatchObject({ meaningful: true }); expect(frames).toEqual([]); }, 180_000, From 4f281e1dd6ec7f42c088dda8e3c76a8f4b5d93b1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 3 Aug 2026 05:53:27 +0000 Subject: [PATCH 07/10] review: release ServePlugins' Plugin handle via Plugin::destroy; update stale trait doc --- src/runtime/api/JSBundler.rs | 4 ++-- src/runtime/server/server_body.rs | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index 4032347fcde2..bbac8171f587 100644 --- a/src/runtime/api/JSBundler.rs +++ b/src/runtime/api/JSBundler.rs @@ -1675,8 +1675,8 @@ pub mod js_bundler { build_result: JSValue, rejection: JsResult, ) -> JsResult; - /// `this` must be a live handle previously returned by `Plugin::create`; - /// non-null is checked via `Plugin::opaque_ref` (panics on null). + /// `this` must be the non-null +1 handle returned by `Plugin::create` + /// (debug-asserted); the callee releases it and may free the allocation. fn destroy(this: *mut Plugin); fn global_object(&self) -> &JSGlobalObject; fn append_defer_promise(&mut self) -> JSValue; diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 36d0aa29b2b6..472abe84f411 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -1045,7 +1045,10 @@ impl ServePlugins { // below (Stacked Borrows), making the eventual `heap::take` in `deref_` UB. let plugin = JSBundler::Plugin::create(global, bun_jsc::BunPluginTarget::Browser); - // SAFETY: `Plugin::create` returns a freshly-boxed `*mut Plugin` (single owner). + // `Plugin` is an `opaque_ffi!` ZST; boxing the handle lets the enum own it + // by value. `Box` drop is a no-op, so release goes through + // `Plugin::destroy` (see `handle_on_reject` / `Drop for ServePlugins`). + // SAFETY: `Plugin::create` returns a non-null +1 handle. let plugin: Box = unsafe { bun_core::heap::take(plugin) }; let mut bunstring_array: Vec = Vec::with_capacity(plugin_list.len()); for raw_plugin in &plugin_list { @@ -1172,7 +1175,7 @@ impl ServePlugins { else { unreachable!() }; - drop(plugin); // pending.plugin.deinit() + JSBundler::Plugin::destroy(Box::into_raw(plugin)); drop(promise); // Drop on JscStrong releases the slot. for route in html_bundle_routes { @@ -1224,10 +1227,12 @@ impl Drop for ServePluginsRef { impl Drop for ServePlugins { fn drop(&mut self) { - match &self.state { + match mem::replace(&mut self.state, ServePluginsState::Err) { ServePluginsState::Unqueued(_) => {} ServePluginsState::Pending { .. } => debug_assert!(false), // should have one ref while pending! - ServePluginsState::Loaded(_) => {} // Box drops + ServePluginsState::Loaded(plugin) => { + JSBundler::Plugin::destroy(Box::into_raw(plugin)); + } ServePluginsState::Err => {} } } From bfeb7d650e010b01fb6d8f4e3f1653f436d393cc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 3 Aug 2026 06:04:26 +0000 Subject: [PATCH 08/10] review: guard ServePlugins Plugin handle across early-return; require exitCode 0 for sawCleanExit --- src/runtime/server/server_body.rs | 5 +++-- test/bundler/bun-build-api.test.ts | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 472abe84f411..f0754d55cf70 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -1047,9 +1047,10 @@ impl ServePlugins { let plugin = JSBundler::Plugin::create(global, bun_jsc::BunPluginTarget::Browser); // `Plugin` is an `opaque_ffi!` ZST; boxing the handle lets the enum own it // by value. `Box` drop is a no-op, so release goes through - // `Plugin::destroy` (see `handle_on_reject` / `Drop for ServePlugins`). + // `Plugin::destroy` (guard below / `handle_on_reject` / `Drop for ServePlugins`). // SAFETY: `Plugin::create` returns a non-null +1 handle. let plugin: Box = unsafe { bun_core::heap::take(plugin) }; + let plugin = scopeguard::guard(plugin, |p| JSBundler::Plugin::destroy(Box::into_raw(p))); let mut bunstring_array: Vec = Vec::with_capacity(plugin_list.len()); for raw_plugin in &plugin_list { bunstring_array.push(BunString::init(&***raw_plugin)); @@ -1059,7 +1060,7 @@ impl ServePlugins { self.state = ServePluginsState::Pending { promise: jsc::JSPromiseStrong::init(global), - plugin, + plugin: scopeguard::ScopeGuard::into_inner(plugin), html_bundle_routes: Vec::new(), dev_server: None, }; diff --git a/test/bundler/bun-build-api.test.ts b/test/bundler/bun-build-api.test.ts index 32d59d0f9a63..be2f8b112745 100644 --- a/test/bundler/bun-build-api.test.ts +++ b/test/bundler/bun-build-api.test.ts @@ -1614,7 +1614,9 @@ test.skipIf(!isASAN)( }); if (/AddressSanitizer|runtime error:|SUMMARY: /.test(stderr)) sawSanitizerReport = true; - if (stdout.includes("ok") && stderr.trim() === "") sawCleanExit = true; + if (stdout.includes("ok") && stderr.trim() === "" && proc.exitCode === 0 && proc.signalCode === null) { + sawCleanExit = true; + } const pluginFrames = stderr.split("\n").filter(l => /JSBundlerPlugin\.cpp|BundlerPlugin::|FilterRegExp/.test(l)); if (pluginFrames.length > 0) { From ddda35cb350151fc58ea955324bdeebbbd224aee Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 3 Aug 2026 06:06:41 +0000 Subject: [PATCH 09/10] review: release Plugin handle in ServePlugins Drop Pending arm too --- src/runtime/server/server_body.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index f0754d55cf70..65ab4b163f02 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -1230,7 +1230,12 @@ impl Drop for ServePlugins { fn drop(&mut self) { match mem::replace(&mut self.state, ServePluginsState::Err) { ServePluginsState::Unqueued(_) => {} - ServePluginsState::Pending { .. } => debug_assert!(false), // should have one ref while pending! + ServePluginsState::Pending { plugin, .. } => { + // Reachable only if the setup host call threw before the promise + // `.then()` took its +1 ref; release the handle we created. + JSBundler::Plugin::destroy(Box::into_raw(plugin)); + debug_assert!(false); // should have one ref while pending! + } ServePluginsState::Loaded(plugin) => { JSBundler::Plugin::destroy(Box::into_raw(plugin)); } From d53dca045b12e9e4f690629ed150fa1f473aaf56 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 3 Aug 2026 06:23:56 +0000 Subject: [PATCH 10/10] fix: clear borrowed plugins pointer before cancelled early-return so deinit does not double-destroy the ServePlugins-owned handle --- src/runtime/api/js_bundle_completion_task.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index 4e05c63e69b8..2d7bbad3ecfe 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -544,12 +544,16 @@ impl JSBundleCompletionTask { // SAFETY: `vm` is the live per-thread VM (`global_this.bun_vm_ptr()`). this.poll_ref .unref(unsafe { jsc::virtual_machine::VirtualMachine::event_loop_ctx(vm) }); + if this.html_build_task.is_some() { + // The HTML-bundle path borrows `plugins` from `ServePluginsState::Loaded`; + // clear it before `deinit` (cancelled or not) so only the owner destroys it. + this.plugins = None; + } if this.cancelled { return Ok(()); } if let Some(html_build_task) = this.html_build_task { - this.plugins = None; // SAFETY: `html_build_task` is a backref set by `HTMLBundle::Route` which // bumped its own refcount before scheduling and stays alive until this returns. // R-2: deref as shared — `on_complete` takes `&self`.