From a5e54e48faec66fb740ba67ae234853d9ec9182b Mon Sep 17 00:00:00 2001 From: Kai Tamkun Date: Tue, 12 Aug 2025 19:16:56 -0700 Subject: [PATCH] Move NapiRef methods into NapiRef.cpp --- src/bun.js/bindings/NapiRef.cpp | 64 +++++++++++++++++++++++++++++++++ src/bun.js/bindings/napi.h | 61 +++---------------------------- 2 files changed, 69 insertions(+), 56 deletions(-) diff --git a/src/bun.js/bindings/NapiRef.cpp b/src/bun.js/bindings/NapiRef.cpp index d33ac46cef..67b82749b2 100644 --- a/src/bun.js/bindings/NapiRef.cpp +++ b/src/bun.js/bindings/NapiRef.cpp @@ -41,6 +41,70 @@ void NapiRef::clear() globalObject.clear(); weakValueRef.clear(); strongRef.clear(); + if (boundCleanup) { + env->removeFinalizer(*boundCleanup); + } +} + +NapiRef::NapiRef(napi_env env, uint32_t count, Bun::NapiFinalizer finalizer, bool deleteSelf) + : env(env) + , globalObject(JSC::Weak(env->globalObject())) + , finalizer(WTFMove(finalizer)) + , refCount(count) + , m_deleteSelf(deleteSelf) +{ +} + +void NapiRef::setValueInitial(JSC::JSValue value, bool can_be_weak) +{ + if (refCount > 0) { + strongRef.set(globalObject->vm(), value); + } + + // In NAPI non-experimental, types other than object, function and symbol can't be used as values for references. + // In NAPI experimental, they can be, but we must not store weak references to them. + if (can_be_weak) { + weakValueRef.set(value, Napi::NapiRefWeakHandleOwner::weakValueHandleOwner(), this); + } + + if (value.isSymbol()) { + auto* symbol = jsDynamicCast(value); + ASSERT(symbol != nullptr); + if (symbol->uid().isRegistered()) { + // Global symbols must always be retrievable, + // even if garbage collection happens while the ref count is 0. + m_isEternal = true; + if (refCount == 0) { + strongRef.set(globalObject->vm(), symbol); + } + } + } +} + +void NapiRef::callFinalizer() +{ + // Calling the finalizer may delete `this`, so we have to do state changes on `this` before + // calling the finalizer + Bun::NapiFinalizer saved_finalizer = this->finalizer; + this->finalizer.clear(); + saved_finalizer.call(env, nativeObject, !env->mustDeferFinalizers() || !env->inGC()); + + (void)m_deleteSelf; +} + +NapiRef::~NapiRef() +{ + NAPI_LOG("destruct napi ref %p", this); + if (boundCleanup) { + boundCleanup->deactivate(env); + boundCleanup = nullptr; + } + + if (!m_isEternal) { + strongRef.clear(); + } + + weakValueRef.clear(); } } diff --git a/src/bun.js/bindings/napi.h b/src/bun.js/bindings/napi.h index 6449b0f0c8..e92f426de5 100644 --- a/src/bun.js/bindings/napi.h +++ b/src/bun.js/bindings/napi.h @@ -509,13 +509,7 @@ public: void unref(); void clear(); - NapiRef(napi_env env, uint32_t count, Bun::NapiFinalizer finalizer) - : env(env) - , globalObject(JSC::Weak(env->globalObject())) - , finalizer(WTFMove(finalizer)) - , refCount(count) - { - } + NapiRef(napi_env env, uint32_t count, Bun::NapiFinalizer finalizer, bool deleteSelf = false); JSC::JSValue value() const { @@ -526,57 +520,11 @@ public: return strongRef.get(); } - void setValueInitial(JSC::JSValue value, bool can_be_weak) - { - if (refCount > 0) { - strongRef.set(globalObject->vm(), value); - } + void setValueInitial(JSC::JSValue value, bool can_be_weak); - // In NAPI non-experimental, types other than object, function and symbol can't be used as values for references. - // In NAPI experimental, they can be, but we must not store weak references to them. - if (can_be_weak) { - weakValueRef.set(value, Napi::NapiRefWeakHandleOwner::weakValueHandleOwner(), this); - } + void callFinalizer(); - if (value.isSymbol()) { - auto* symbol = jsDynamicCast(value); - ASSERT(symbol != nullptr); - if (symbol->uid().isRegistered()) { - // Global symbols must always be retrievable, - // even if garbage collection happens while the ref count is 0. - m_isEternal = true; - if (refCount == 0) { - strongRef.set(globalObject->vm(), symbol); - } - } - } - } - - void callFinalizer() - { - // Calling the finalizer may delete `this`, so we have to do state changes on `this` before - // calling the finalizer - Bun::NapiFinalizer saved_finalizer = this->finalizer; - this->finalizer.clear(); - saved_finalizer.call(env, nativeObject, !env->mustDeferFinalizers() || !env->inGC()); - } - - ~NapiRef() - { - NAPI_LOG("destruct napi ref %p", this); - if (boundCleanup) { - boundCleanup->deactivate(env); - boundCleanup = nullptr; - } - - if (!m_isEternal) { - strongRef.clear(); - } - - // The weak ref can lead to calling the destructor - // so we must first clear the weak ref before we call the finalizer - weakValueRef.clear(); - } + ~NapiRef(); napi_env env = nullptr; JSC::Weak globalObject; @@ -590,6 +538,7 @@ public: private: bool m_isEternal = false; + bool m_deleteSelf = false; }; static inline napi_ref toNapi(NapiRef* val)