diff --git a/src/bun.js/bindings/JSBundlerPlugin.cpp b/src/bun.js/bindings/JSBundlerPlugin.cpp index 0f24159f72..9b129148a1 100644 --- a/src/bun.js/bindings/JSBundlerPlugin.cpp +++ b/src/bun.js/bindings/JSBundlerPlugin.cpp @@ -29,7 +29,7 @@ #include #include "ErrorCode.h" #include "napi_external.h" -#include + #include #if OS(WINDOWS) @@ -117,9 +117,9 @@ static const HashTableValue JSBundlerPluginHashTable[] = { { "generateDeferPromise"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete), NoIntrinsic, { HashTableValue::NativeFunctionType, jsBundlerPluginFunction_generateDeferPromise, 0 } }, }; -class JSBundlerPlugin final : public JSC::JSNonFinalObject { +class JSBundlerPlugin final : public JSC::JSDestructibleObject { public: - using Base = JSC::JSNonFinalObject; + using Base = JSC::JSDestructibleObject; static JSBundlerPlugin* create(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::Structure* structure, @@ -156,6 +156,9 @@ public: } DECLARE_VISIT_CHILDREN; + DECLARE_VISIT_OUTPUT_CONSTRAINTS; + + template void visitAdditionalChildren(Visitor&); Bun::BundlerPlugin plugin; /// These are defined in BundlerPlugin.ts @@ -165,30 +168,53 @@ public: JSC::JSGlobalObject* m_globalObject; + static void destroy(JSC::JSCell* cell) + { + JSBundlerPlugin* thisObject = static_cast(cell); + thisObject->~JSBundlerPlugin(); + } + private: JSBundlerPlugin(JSC::VM& vm, JSC::JSGlobalObject* global, JSC::Structure* structure, void* config, BunPluginTarget target, JSBundlerPluginAddErrorCallback addError, JSBundlerPluginOnLoadAsyncCallback onLoadAsync, JSBundlerPluginOnResolveAsyncCallback onResolveAsync) - : JSC::JSNonFinalObject(vm, structure) + : Base(vm, structure) , plugin(BundlerPlugin(config, target, addError, onLoadAsync, onResolveAsync)) , m_globalObject(global) { } + ~JSBundlerPlugin() = default; void finishCreation(JSC::VM&); }; +template +void JSBundlerPlugin::visitAdditionalChildren(Visitor& visitor) +{ + this->onLoadFunction.visit(visitor); + this->onResolveFunction.visit(visitor); + this->setupFunction.visit(visitor); + this->plugin.deferredPromises.visit(this, visitor); +} + template void JSBundlerPlugin::visitChildrenImpl(JSCell* cell, Visitor& visitor) { JSBundlerPlugin* thisObject = jsCast(cell); ASSERT_GC_OBJECT_INHERITS(thisObject, info()); Base::visitChildren(thisObject, visitor); - thisObject->onLoadFunction.visit(visitor); - thisObject->onResolveFunction.visit(visitor); - thisObject->setupFunction.visit(visitor); + thisObject->visitAdditionalChildren(visitor); } DEFINE_VISIT_CHILDREN(JSBundlerPlugin); +template +void JSBundlerPlugin::visitOutputConstraintsImpl(JSCell* cell, Visitor& visitor) +{ + JSBundlerPlugin* thisObject = jsCast(cell); + ASSERT_GC_OBJECT_INHERITS(thisObject, info()); + thisObject->visitAdditionalChildren(visitor); +} +DEFINE_VISIT_OUTPUT_CONSTRAINTS(JSBundlerPlugin); + const JSC::ClassInfo JSBundlerPlugin::s_info = { "BundlerPlugin"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSBundlerPlugin) }; /// `BundlerPlugin.prototype.addFilter(filter: RegExp, namespace: string, isOnLoad: 0 | 1): void` @@ -425,10 +451,11 @@ JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_onResolveAsync, (JSC::JSGlobalO extern "C" JSC::EncodedJSValue JSBundlerPlugin__appendDeferPromise(Bun::JSBundlerPlugin* pluginObject) { - JSC::JSGlobalObject* globalObject = pluginObject->globalObject(); - Strong strong_promise = JSC::Strong(globalObject->vm(), JSPromise::create(globalObject->vm(), globalObject->promiseStructure())); - JSPromise* ret = strong_promise.get(); - pluginObject->plugin.deferredPromises.append(strong_promise); + auto* vm = &pluginObject->vm(); + auto* globalObject = pluginObject->globalObject(); + + JSPromise* ret = JSPromise::create(*vm, globalObject->promiseStructure()); + pluginObject->plugin.deferredPromises.append(*vm, pluginObject, ret); return JSC::JSValue::encode(ret); } @@ -638,15 +665,22 @@ extern "C" void JSBundlerPlugin__setConfig(Bun::JSBundlerPlugin* plugin, void* c extern "C" void JSBundlerPlugin__drainDeferred(Bun::JSBundlerPlugin* pluginObject, bool rejected) { - auto deferredPromises = std::exchange(pluginObject->plugin.deferredPromises, {}); - for (auto& promise : deferredPromises) { + auto* globalObject = pluginObject->globalObject(); + MarkedArgumentBuffer arguments; + pluginObject->plugin.deferredPromises.moveTo(pluginObject, arguments); + ASSERT(!arguments.hasOverflowed()); + + auto scope = DECLARE_THROW_SCOPE(pluginObject->vm()); + for (auto promiseValue : arguments) { + JSPromise* promise = jsCast(JSValue::decode(promiseValue)); if (rejected) { - promise->reject(pluginObject->globalObject(), JSC::jsUndefined()); + promise->reject(globalObject, JSC::jsUndefined()); } else { - promise->resolve(pluginObject->globalObject(), JSC::jsUndefined()); + promise->resolve(globalObject, JSC::jsUndefined()); } - promise.clear(); + RETURN_IF_EXCEPTION(scope, ); } + RETURN_IF_EXCEPTION(scope, ); } extern "C" void JSBundlerPlugin__tombstone(Bun::JSBundlerPlugin* plugin) diff --git a/src/bun.js/bindings/JSBundlerPlugin.h b/src/bun.js/bindings/JSBundlerPlugin.h index 7bef5769fa..34fb313075 100644 --- a/src/bun.js/bindings/JSBundlerPlugin.h +++ b/src/bun.js/bindings/JSBundlerPlugin.h @@ -7,6 +7,7 @@ #include #include "napi_external.h" #include +#include "WriteBarrierList.h" typedef void (*JSBundlerPluginAddErrorCallback)(void*, void*, JSC::EncodedJSValue, JSC::EncodedJSValue); typedef void (*JSBundlerPluginOnLoadAsyncCallback)(void*, void*, JSC::EncodedJSValue, JSC::EncodedJSValue); @@ -134,7 +135,7 @@ public: NativePluginList onBeforeParse = {}; BunPluginTarget target { BunPluginTargetBrowser }; - Vector> deferredPromises = {}; + WriteBarrierList deferredPromises = {}; JSBundlerPluginAddErrorCallback addError; JSBundlerPluginOnLoadAsyncCallback onLoadAsync; diff --git a/src/bun.js/bindings/WriteBarrierList.h b/src/bun.js/bindings/WriteBarrierList.h new file mode 100644 index 0000000000..0db6c5811c --- /dev/null +++ b/src/bun.js/bindings/WriteBarrierList.h @@ -0,0 +1,92 @@ +#pragma once + +#include +#include +#include + +namespace Bun { + +/** + * A variable-length list of JSValue objects with garbage collection support. + * + * This class provides a thread-safe container for WriteBarrier objects that can + * dynamically grow and shrink. It includes helper methods for visiting contained + * objects during garbage collection traversal. + * + * Use this class when: + * - The number of items may change at runtime (append/remove operations) + * - You need thread-safe access to the list + * - You need automatic garbage collection support for contained JSValues + * + * For better performance when the length is known and fixed, prefer + * FixedVector> instead. + * + * @tparam T The type of JSC objects to store (must inherit from JSC::JSCell) + */ +template +class WriteBarrierList { +public: + WriteBarrierList() + { + } + + void append(JSC::VM& vm, JSC::JSCell* owner, T* value) + { + WTF::Locker locker { owner->cellLock() }; + m_list.append(JSC::WriteBarrier(vm, owner, value)); + } + + std::span> list() + { + return m_list.mutableSpan(); + } + + void moveTo(JSC::JSCell* owner, JSC::MarkedArgumentBuffer& arguments) + { + WTF::Locker locker { owner->cellLock() }; + for (JSC::WriteBarrier& value : m_list) { + if (auto* cell = value.get()) { + arguments.append(cell); + value.clear(); + } + } + } + + template + void visit(JSC::JSCell* owner, Visitor& visitor) + { + WTF::Locker locker { owner->cellLock() }; + for (auto& value : m_list) { + visitor.append(value); + } + } + + bool isEmpty() const + { + return m_list.isEmpty(); + } + + T* takeFirst(JSC::JSCell* owner) + { + WTF::Locker locker { owner->cellLock() }; + if (m_list.isEmpty()) { + return nullptr; + } + + T* value = m_list.first().get(); + m_list.removeAt(0); + return value; + } + + template + bool removeFirstMatching(JSC::JSCell* owner, const MatchFunction& matches) + { + WTF::Locker locker { owner->cellLock() }; + return m_list.removeFirstMatching(matches); + } + +private: + WTF::Vector> m_list; +}; + +} diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index 7e7a3c0270..fc584871ca 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -1354,10 +1354,10 @@ void GlobalObject::promiseRejectionTracker(JSGlobalObject* obj, JSC::JSPromise* auto* globalObj = static_cast(obj); switch (operation) { case JSPromiseRejectionOperation::Reject: - globalObj->m_aboutToBeNotifiedRejectedPromises.append(JSC::Strong(obj->vm(), promise)); + globalObj->m_aboutToBeNotifiedRejectedPromises.append(obj->vm(), globalObj, promise); break; case JSPromiseRejectionOperation::Handle: - bool removed = globalObj->m_aboutToBeNotifiedRejectedPromises.removeFirstMatching([&](Strong& unhandledPromise) { + bool removed = globalObj->m_aboutToBeNotifiedRejectedPromises.removeFirstMatching(globalObj, [&](JSC::WriteBarrier& unhandledPromise) { return unhandledPromise.get() == promise; }); if (removed) break; @@ -4048,16 +4048,13 @@ void GlobalObject::handleRejectedPromises() { JSC::VM& virtual_machine = vm(); auto scope = DECLARE_CATCH_SCOPE(virtual_machine); - do { - auto unhandledRejections = WTFMove(m_aboutToBeNotifiedRejectedPromises); - for (auto& promise : unhandledRejections) { - if (promise->isHandled(virtual_machine)) - continue; + while (auto* promise = m_aboutToBeNotifiedRejectedPromises.takeFirst(this)) { + if (promise->isHandled(virtual_machine)) + continue; - Bun__handleRejectedPromise(this, promise.get()); - if (auto ex = scope.exception()) this->reportUncaughtExceptionAtEventLoop(this, ex); - } - } while (!m_aboutToBeNotifiedRejectedPromises.isEmpty()); + Bun__handleRejectedPromise(this, promise); + if (auto ex = scope.exception()) this->reportUncaughtExceptionAtEventLoop(this, ex); + } } DEFINE_VISIT_CHILDREN(GlobalObject); @@ -4070,6 +4067,9 @@ void GlobalObject::visitAdditionalChildren(Visitor& visitor) thisObject->globalEventScope->visitJSEventListeners(visitor); + thisObject->m_aboutToBeNotifiedRejectedPromises.visit(thisObject, visitor); + thisObject->m_ffiFunctions.visit(thisObject, visitor); + ScriptExecutionContext* context = thisObject->scriptExecutionContext(); visitor.addOpaqueRoot(context); } @@ -4625,18 +4625,13 @@ void GlobalObject::setNodeWorkerEnvironmentData(JSMap* data) { m_nodeWorkerEnvir void GlobalObject::trackFFIFunction(JSC::JSFunction* function) { - this->m_ffiFunctions.append(JSC::Strong { vm(), function }); + this->m_ffiFunctions.append(vm(), this, function); } bool GlobalObject::untrackFFIFunction(JSC::JSFunction* function) { - for (size_t i = 0; i < this->m_ffiFunctions.size(); ++i) { - if (this->m_ffiFunctions[i].get() == function) { - this->m_ffiFunctions[i].clear(); - this->m_ffiFunctions.removeAt(i); - return true; - } - } - return false; + return this->m_ffiFunctions.removeFirstMatching(this, [&](JSC::WriteBarrier& untrackedFunction) -> bool { + return untrackedFunction.get() == function; + }); } extern "C" void Zig__GlobalObject__destructOnExit(Zig::GlobalObject* globalObject) diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h index 55fb45fd77..7f9f3412b0 100644 --- a/src/bun.js/bindings/ZigGlobalObject.h +++ b/src/bun.js/bindings/ZigGlobalObject.h @@ -57,6 +57,7 @@ class GlobalInternals; #include "BunGlobalScope.h" #include #include +#include "WriteBarrierList.h" namespace Bun { class JSCommonJSExtensions; @@ -735,8 +736,8 @@ private: DOMGuardedObjectSet m_guardedObjects WTF_GUARDED_BY_LOCK(m_gcLock); WebCore::SubtleCrypto* m_subtleCrypto = nullptr; - WTF::Vector> m_aboutToBeNotifiedRejectedPromises; - WTF::Vector> m_ffiFunctions; + Bun::WriteBarrierList m_aboutToBeNotifiedRejectedPromises; + Bun::WriteBarrierList m_ffiFunctions; }; class EvalGlobalObject : public GlobalObject { diff --git a/test/bundler/bundler_defer.test.ts b/test/bundler/bundler_defer.test.ts index 8f16b9918d..44f72a7e9b 100644 --- a/test/bundler/bundler_defer.test.ts +++ b/test/bundler/bundler_defer.test.ts @@ -536,57 +536,79 @@ warn: (msg: string) => console.warn(\`[WARN] \${msg}\`) const outdir = path.join(folder, "dist"); - const result = await Bun.build({ - entrypoints: [entrypoint], - outdir, - plugins: [ - { - name: "xXx123_import_checker_321xXx", - setup(build) { - type Import = { - imported: string[]; - dep: string; - }; - type Export = { - ident: string; - }; - let imports_and_exports: Record; exports: Array }> = {}; - - build.onLoad({ filter: /\.ts/ }, async ({ path }) => { - const contents = await Bun.$`cat ${path}`.quiet().text(); - - const import_regex = /import\s+(?:([\s\S]*?)\s+from\s+)?['"]([^'"]+)['"];/g; - const imports: Array = [...contents.toString().matchAll(import_regex)].map(m => ({ - imported: m - .slice(1, m.length - 1) - .map(match => (match[0] === "{" ? match.slice(2, match.length - 2) : match)), - dep: m[m.length - 1], - })); - - const export_regex = - /export\s+(?:default\s+|const\s+|let\s+|var\s+|function\s+|class\s+|enum\s+|type\s+|interface\s+)?([\w$]+)?(?:\s*=\s*|(?:\s*{[^}]*})?)?[^;]*;/g; - const exports: Array = [...contents.matchAll(export_regex)].map(m => ({ - ident: m[1], - })); - - imports_and_exports[path.replaceAll("\\", "/").split("/").pop()!] = { imports, exports }; - return undefined; - }); - - build.onLoad({ filter: /module_data\.json/ }, async ({ defer }) => { - await defer(); - const contents = JSON.stringify(imports_and_exports); - - return { - contents, - loader: "json", - }; - }); - }, - }, - ], + let onFinalizeCallCount = 0; + let onFinalizeCalledThrice = Promise.withResolvers(); + let onFinalizeCallRegistry = new FinalizationRegistry(() => { + onFinalizeCallCount++; + if (onFinalizeCallCount === 3) { + onFinalizeCalledThrice.resolve(); + } }); + const result = await (async function () { + return await Bun.build({ + entrypoints: [entrypoint], + outdir, + plugins: [ + (() => { + const plugin = { + name: "xXx123_import_checker_321xXx", + setup(build) { + type Import = { + imported: string[]; + dep: string; + }; + type Export = { + ident: string; + }; + let imports_and_exports: Record; exports: Array }> = {}; + + const onLoadTS = async ({ path }) => { + const contents = await Bun.$`cat ${path}`.quiet().text(); + + const import_regex = /import\s+(?:([\s\S]*?)\s+from\s+)?['"]([^'"]+)['"];/g; + const imports: Array = [...contents.toString().matchAll(import_regex)].map(m => ({ + imported: m + .slice(1, m.length - 1) + .map(match => (match[0] === "{" ? match.slice(2, match.length - 2) : match)), + dep: m[m.length - 1], + })); + + const export_regex = + /export\s+(?:default\s+|const\s+|let\s+|var\s+|function\s+|class\s+|enum\s+|type\s+|interface\s+)?([\w$]+)?(?:\s*=\s*|(?:\s*{[^}]*})?)?[^;]*;/g; + const exports: Array = [...contents.matchAll(export_regex)].map(m => ({ + ident: m[1], + })); + + imports_and_exports[path.replaceAll("\\", "/").split("/").pop()!] = { imports, exports }; + return undefined; + }; + + const onLoadModuleData = async ({ defer }) => { + await defer(); + const contents = JSON.stringify(imports_and_exports); + + return { + contents, + loader: "json", + }; + }; + + build.onLoad({ filter: /\.ts/ }, onLoadTS); + + build.onLoad({ filter: /module_data\.json/ }, onLoadModuleData); + + onFinalizeCallRegistry.register(onLoadTS, undefined); + onFinalizeCallRegistry.register(onLoadModuleData, undefined); + }, + }; + onFinalizeCallRegistry.register(plugin.setup, undefined); + return plugin; + })(), + ], + }); + })(); + expect(result.success).toBeTrue(); await Bun.$`${bunExe()} run ${result.outputs[0].path}`; const output = await Bun.$`cat ${path.join(folder, "dist", "output.json")}`.json(); @@ -619,5 +641,8 @@ warn: (msg: string) => console.warn(\`[WARN] \${msg}\`) "exports": [{ "ident": "logger" }], }, }); + Bun.gc(true); + await onFinalizeCalledThrice.promise; + expect(onFinalizeCallCount).toBe(3); }); });