From 657359b506d33c2e640c4c95e8878bc8a9aff8ab Mon Sep 17 00:00:00 2001 From: Ammar Ahmed Date: Mon, 9 Nov 2026 11:50:59 +0500 Subject: [PATCH] fix(android): release Java-returned wrappers when JavaScript drops them --- .../jni/jsi/objectmanager/ObjectManager.cpp | 100 +++++++++++++++--- .../ffi/jni/jsi/objectmanager/ObjectManager.h | 33 +++++- .../jni/napi/objectmanager/ObjectManager.cpp | 85 ++++++++++++--- .../jni/napi/objectmanager/ObjectManager.h | 10 +- 4 files changed, 197 insertions(+), 31 deletions(-) diff --git a/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp b/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp index 57029efe..86ebb7aa 100644 --- a/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp +++ b/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp @@ -6,6 +6,7 @@ #include "NativeScriptException.h" #include "Runtime.h" #include "CallbackHandlers.h" +#include "EngineHost.h" #include #include @@ -60,6 +61,9 @@ ObjectManager::ObjectManager(jobject javaRuntimeObject) : void ObjectManager::Init(JsRuntime &rt) { m_rt = &rt; + m_ownerToken = std::make_shared(); + m_ownerToken->manager = this; + m_ownerToken->runtime = &rt; JsFunction jsObjectCtor = JsFunction::createFromHostConstructor( rt, JsPropNameID::forAscii(rt, "JSObject"), 0, @@ -84,6 +88,10 @@ void ObjectManager::OnDisposeRuntime() { m_idToProxy.clear(); m_idToObject.clear(); m_jsObjectCtor = JsFunction(); + if (m_ownerToken != nullptr) { + m_ownerToken->manager = nullptr; + m_ownerToken->runtime = nullptr; + } // Its destructor leaves the weak global refs alive, and the process-wide // weak reference table is finite. @@ -141,15 +149,7 @@ JsValue ObjectManager::GetOrCreateProxy(jint javaObjectID, const JsValue &instan } JsValue proxy = CreateHostObjectProxy(instance, info, /*isPrimary=*/true); - auto javaObjectIdFound = m_weakObjectIds.find(javaObjectID); - if (javaObjectIdFound != m_weakObjectIds.end()) { - m_weakObjectIds.erase(javaObjectID); - JEnv jenv; - jenv.CallVoidMethod(m_javaRuntimeObject, - MAKE_INSTANCE_STRONG_METHOD_ID, - javaObjectID); - DEBUG_WRITE("Making instance strong: %d", javaObjectID); - } + EnsureInstanceStrong(javaObjectID); m_idToProxy.emplace(javaObjectID, engine::WeakObject(*m_rt, proxy)); @@ -608,11 +608,26 @@ JsValue ObjectManager::GetJsObjectByJavaObject(int javaObjectID) { return js_util::undefined(); } - JsValue instance = it->second; + JsValue instance = LockWrapper(it->second); if (js_util::is_null_or_undefined(instance)) return js_util::undefined(); + + auto proxyIt = m_idToProxy.find(javaObjectID); + if (proxyIt != m_idToProxy.end() && !proxyIt->second.empty()) { + JsValue proxy = proxyIt->second.lock(*m_rt); + if (!js_util::is_null_or_undefined(proxy)) return proxy; + } + + MetadataNode *node = GetInstanceNode(instance); + if (node != nullptr && !node->isArray()) return instance; return GetOrCreateProxy(javaObjectID, instance); } +JsValue ObjectManager::LockWrapper(const WrapperHandle &handle) { + if (handle.isStrong) return JsValue(*m_rt, handle.strong); + if (handle.weak.empty()) return js_util::undefined(); + return handle.weak.lock(*m_rt); +} + JsValue ObjectManager::CreateJSWrapper(jint javaObjectID, const std::string &typeName) { return CreateJSWrapperHelper(javaObjectID, typeName, nullptr); @@ -648,18 +663,30 @@ JsValue ObjectManager::CreateJSWrapper(jint javaObjectID, MetadataNode *node, jc JsValue proxy = js_util::undefined(); JsValue jsWrapper = node->CreateJSWrapper(*m_rt, this); if (jsWrapper.isObject()) { - Link(jsWrapper, javaObjectID, clazz, node); + Link(jsWrapper, javaObjectID, clazz, node, /*strongRef*/ false); + EnsureInstanceStrong(javaObjectID); if (node->isArray()) { jsWrapper.asObject(*m_rt).setProperty(*m_rt, "__is__javaArray", true); + proxy = GetOrCreateProxy(javaObjectID, jsWrapper); + } else { + proxy = jsWrapper; } - proxy = GetOrCreateProxy(javaObjectID, jsWrapper); } return proxy; } +void ObjectManager::EnsureInstanceStrong(int javaObjectID) { + auto it = m_weakObjectIds.find(javaObjectID); + if (it == m_weakObjectIds.end()) return; + m_weakObjectIds.erase(it); + JEnv jenv; + jenv.CallVoidMethod(m_javaRuntimeObject, MAKE_INSTANCE_STRONG_METHOD_ID, javaObjectID); + DEBUG_WRITE("Making instance strong: %d", javaObjectID); +} + void ObjectManager::Link(const JsValue &object, uint32_t javaObjectID, jclass clazz, - MetadataNode *node) { + MetadataNode *node, bool strongRef) { if (!IsRuntimeJsObject(object)) { std::string errMsg("Trying to link invalid 'this' to a Java object"); throw NativeScriptException(errMsg); @@ -669,12 +696,57 @@ void ObjectManager::Link(const JsValue &object, uint32_t javaObjectID, jclass cl auto jsInstanceInfo = std::make_shared(javaObjectID, clazz); jsInstanceInfo->node = node; + if (!strongRef) { + jsInstanceInfo->owner = m_ownerToken; + auto rtOwner = Runtime::GetRuntimeUnchecked(*m_rt); + if (rtOwner != nullptr) rtOwner->GetEngineHost()->AdjustExternalMemory(kWrapperExternalCost); + } // One slot, one owner: the native state both carries the record and keeps it // alive, replacing the napi tree's external-plus-wrap pair. object.asObjectBorrowed(*m_rt).setNativeState(*m_rt, jsInstanceInfo); - m_idToObject.emplace(javaObjectID, JsValue(*m_rt, object)); + WrapperHandle handle; + if (strongRef) { + handle.strong = JsValue(*m_rt, object); + handle.isStrong = true; + } else { + handle.weak = engine::WeakObject(*m_rt, object); + } + m_idToObject[javaObjectID] = std::move(handle); +} + +ObjectManager::JSInstanceInfo::~JSInstanceInfo() { + if (owner == nullptr || owner->manager == nullptr || owner->runtime == nullptr) return; + auto *pending = new int(static_cast(JavaObjectID)); + Runtime::PostFinalizer(*owner->runtime, WrapperPostFinalizer, pending, owner->manager); +} + +void ObjectManager::WrapperPostFinalizer(JsRuntime &rt, void *data, void *hint) { + auto *pending = reinterpret_cast(data); + if (pending == nullptr) return; + int javaObjectID = *pending; + delete pending; + + auto rtOwner = Runtime::GetRuntimeUnchecked(rt); + if (rtOwner == nullptr || rtOwner->is_destroying) return; + auto objManager = reinterpret_cast(hint); + if (objManager == nullptr || objManager != rtOwner->GetObjectManager()) return; + + rtOwner->GetEngineHost()->AdjustExternalMemory(-kWrapperExternalCost); + + auto it = objManager->m_idToObject.find(javaObjectID); + if (it != objManager->m_idToObject.end() && + !js_util::is_null_or_undefined(objManager->LockWrapper(it->second))) { + return; + } + if (objManager->m_weakObjectIds.find(javaObjectID) == objManager->m_weakObjectIds.end()) { + objManager->m_weakObjectIds.emplace(javaObjectID); + JEnv jEnv; + jEnv.CallVoidMethod(objManager->m_javaRuntimeObject, objManager->MAKE_INSTANCE_WEAK_METHOD_ID, + javaObjectID); + } + if (it != objManager->m_idToObject.end()) objManager->m_idToObject.erase(it); } bool ObjectManager::CloneLink(const JsValue &src, const JsValue &dest) { diff --git a/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.h b/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.h index 1fc5b21f..f49d134f 100644 --- a/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.h +++ b/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.h @@ -53,7 +53,9 @@ namespace tns { JsValue GetOrCreateProxyWeak(jint javaObjectID, const JsValue &instance); void Link(const JsValue &object, uint32_t javaObjectID, jclass clazz, - MetadataNode *node = nullptr); + MetadataNode *node = nullptr, bool strongRef = true); + + static constexpr int64_t kWrapperExternalCost = 1024; // Returns the class metadata stored on the per-instance JSInstanceInfo // (host proxy's, or the raw instance's native state). Used by @@ -99,13 +101,26 @@ namespace tns { // It derives from engine::HostObject because that is what the native // state slot stores; it overrides none of the traps and is never exposed // to JS as an object of its own. + // Lets a weakly held wrapper's native state reach its manager. The engine + // destroys native state on its own schedule, possibly after the manager is + // gone, so OnDisposeRuntime clears the token. + struct OwnerToken { + ObjectManager *manager = nullptr; + JsRuntime *runtime = nullptr; + }; + struct JSInstanceInfo : public engine::HostObject { public: JSInstanceInfo(uint32_t javaObjectID, jclass claz) : JavaObjectID(javaObjectID), ObjectClazz(claz) { } + // For a weakly held wrapper this runs when the engine collects it, and + // queues the release of the Java side. + ~JSInstanceInfo() override; + uint32_t JavaObjectID; + std::shared_ptr owner; jclass ObjectClazz; // Cached super-call flag (-1 = unresolved, 0 = false, 1 = true). int8_t isSuper = -1; @@ -199,6 +214,10 @@ namespace tns { JsValue CreateJSWrapperHelper(jint javaObjectID, const std::string &typeName, jclass clazz); + void EnsureInstanceStrong(int javaObjectID); + + static void WrapperPostFinalizer(JsRuntime &rt, void *data, void *hint); + jweak GetJavaObjectByID(uint32_t javaObjectID); jobject GetJavaObjectByIDImpl(uint32_t javaObjectID); @@ -216,7 +235,17 @@ namespace tns { // The napi tree stored a weak napi_ref for proxies and a strong one for // instances; those map onto engine::WeakObject and an owned Value. robin_hood::unordered_map m_idToProxy; - robin_hood::unordered_map m_idToObject; + // Strong for JS-constructed instances, weak for wrappers of Java-returned objects. + struct WrapperHandle { + JsValue strong; + engine::WeakObject weak; + bool isStrong = false; + }; + + JsValue LockWrapper(const WrapperHandle &handle); + + robin_hood::unordered_map m_idToObject; + std::shared_ptr m_ownerToken; robin_hood::unordered_set m_weakObjectIds; robin_hood::unordered_set m_markedAsWeakIds; diff --git a/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp b/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp index 92ebe21d..4558eb36 100644 --- a/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp +++ b/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp @@ -199,15 +199,7 @@ napi_value ObjectManager::GetOrCreateProxy(jint javaObjectID, napi_value instanc #endif - auto javaObjectIdFound = m_weakObjectIds.find(javaObjectID); - if (javaObjectIdFound != m_weakObjectIds.end()) { - m_weakObjectIds.erase(javaObjectID); - JEnv jenv; - jenv.CallVoidMethod(m_javaRuntimeObject, - MAKE_INSTANCE_STRONG_METHOD_ID, - javaObjectID); - DEBUG_WRITE("Making instance strong: %d", javaObjectID); - } + EnsureInstanceStrong(javaObjectID); m_idToProxy.emplace(javaObjectID, napi_util::make_ref(m_env, proxy, 0)); @@ -799,6 +791,15 @@ napi_value ObjectManager::GetJsObjectByJavaObject(int javaObjectID) { napi_value instance = napi_util::get_ref_value(m_env, it->second); if (napi_util::is_null_or_undefined(m_env, instance)) return nullptr; + + auto proxyIt = m_idToProxy.find(javaObjectID); + if (proxyIt != m_idToProxy.end() && proxyIt->second != nullptr) { + napi_value proxy = napi_util::get_ref_value(m_env, proxyIt->second); + if (!napi_util::is_null_or_undefined(m_env, proxy)) return proxy; + } + + MetadataNode *node = GetInstanceNode(instance); + if (node != nullptr && !node->isArray()) return instance; return GetOrCreateProxy(javaObjectID, instance); } @@ -839,19 +840,31 @@ napi_value ObjectManager::CreateJSWrapper(jint javaObjectID, MetadataNode *node, napi_value proxy = nullptr; napi_value jsWrapper = node->CreateJSWrapper(m_env, this); if (jsWrapper != nullptr) { - Link(jsWrapper, javaObjectID, clazz, node); + Link(jsWrapper, javaObjectID, clazz, node, /*strongRef*/ false); + EnsureInstanceStrong(javaObjectID); if (node->isArray()) { NAPI_GUARD(napi_set_named_property(m_env, jsWrapper, "__is__javaArray", napi_util::get_true(m_env))) {} + proxy = GetOrCreateProxy(javaObjectID, jsWrapper); + } else { + proxy = jsWrapper; } - proxy = GetOrCreateProxy(javaObjectID, jsWrapper); } return proxy; } +void ObjectManager::EnsureInstanceStrong(int javaObjectID) { + auto it = m_weakObjectIds.find(javaObjectID); + if (it == m_weakObjectIds.end()) return; + m_weakObjectIds.erase(it); + JEnv jenv; + jenv.CallVoidMethod(m_javaRuntimeObject, MAKE_INSTANCE_STRONG_METHOD_ID, javaObjectID); + DEBUG_WRITE("Making instance strong: %d", javaObjectID); +} + void ObjectManager::Link(napi_value object, uint32_t javaObjectID, jclass clazz, - MetadataNode *node) { + MetadataNode *node, bool strongRef) { if (!IsRuntimeJsObject(object)) { std::string errMsg("Trying to link invalid 'this' to a Java object"); throw NativeScriptException(errMsg); @@ -862,8 +875,14 @@ void ObjectManager::Link(napi_value object, uint32_t javaObjectID, jclass clazz, napi_status status; auto jsInstanceInfo = new JSInstanceInfo(javaObjectID, clazz); jsInstanceInfo->node = node; + jsInstanceInfo->weakWrapper = !strongRef; + + napi_ref objectHandle = napi_util::make_ref(m_env, object, strongRef ? 1 : 0); - napi_ref objectHandle = napi_util::make_ref(m_env, object, 1); + if (!strongRef) { + int64_t externalMemory = 0; + js_adjust_external_memory(m_env, kWrapperExternalCost, &externalMemory); + } napi_value jsInfo; NAPI_GUARD(napi_create_external(m_env, jsInstanceInfo, JSObjectFinalizerCallback, jsInstanceInfo, &jsInfo)) {} @@ -873,7 +892,13 @@ void ObjectManager::Link(napi_value object, uint32_t javaObjectID, jclass clazz, NAPI_GUARD(napi_wrap(m_env, object, jsInstanceInfo, [](napi_env env, void *data, void *hint) {}, jsInstanceInfo, nullptr)) {} - m_idToObject.emplace(javaObjectID, objectHandle); + auto existing = m_idToObject.find(javaObjectID); + if (existing != m_idToObject.end()) { + NAPI_GUARD(napi_delete_reference(m_env, existing->second)) {} + existing->second = objectHandle; + } else { + m_idToObject.emplace(javaObjectID, objectHandle); + } } bool ObjectManager::CloneLink(napi_value src, napi_value dest) { @@ -922,6 +947,38 @@ ObjectManager::JSObjectFinalizerCallback(napi_env env, void *finalizeData, void #endif DEBUG_WRITE("JS Object finalizer called for object id: %d", data->JavaObjectID); + Runtime::PostFinalizer(env, JSObjectPostFinalizerCallback, data, nullptr); +} + +void ObjectManager::JSObjectPostFinalizerCallback(napi_env env, void *finalizeData, void *) { + napi_status status; + auto data = reinterpret_cast(finalizeData); + if (data == nullptr) return; + + auto rt = Runtime::GetRuntimeUnchecked(env); + if (data->weakWrapper && rt && !rt->is_destroying) { + int64_t externalMemory = 0; + js_adjust_external_memory(env, -kWrapperExternalCost, &externalMemory); + + auto objManager = rt->GetObjectManager(); + auto it = objManager->m_idToObject.find(data->JavaObjectID); + bool replaced = false; + if (it != objManager->m_idToObject.end()) { + napi_value current = napi_util::get_ref_value(env, it->second); + replaced = !napi_util::is_null_or_undefined(env, current); + if (!replaced) { + NAPI_GUARD(napi_delete_reference(env, it->second)) {} + objManager->m_idToObject.erase(it); + } + } + if (!replaced && objManager->m_weakObjectIds.find(data->JavaObjectID) == + objManager->m_weakObjectIds.end()) { + objManager->m_weakObjectIds.emplace(data->JavaObjectID); + JEnv jEnv; + jEnv.CallVoidMethod(objManager->m_javaRuntimeObject, + objManager->MAKE_INSTANCE_WEAK_METHOD_ID, data->JavaObjectID); + } + } delete data; } diff --git a/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.h b/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.h index 5aaaf947..4e4af967 100644 --- a/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.h +++ b/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.h @@ -53,7 +53,9 @@ namespace tns { napi_value GetOrCreateProxyWeak(jint javaObjectID, napi_value instance); void Link(napi_value object, uint32_t javaObjectID, jclass clazz, - MetadataNode *node = nullptr); + MetadataNode *node = nullptr, bool strongRef = true); + + static constexpr int64_t kWrapperExternalCost = 1024; // Returns the class metadata stored on the per-instance JSInstanceInfo // (host proxy's, or the raw instance's wrap). Used by @@ -108,6 +110,8 @@ namespace tns { // Per-instance class metadata. Under USE_HOST_OBJECT this replaces the // "#instance_metadata" property; reachable via GetInstanceNode. MetadataNode *node = nullptr; + // Set for wrappers held weakly: their finalizer releases the Java side. + bool weakWrapper = false; }; #ifdef USE_HOST_OBJECT @@ -168,6 +172,10 @@ namespace tns { static void JSObjectFinalizerCallback(napi_env env, void *finalizeData, void *finalizeHint); + static void JSObjectPostFinalizerCallback(napi_env env, void *finalizeData, void *finalizeHint); + + void EnsureInstanceStrong(int javaObjectID); + static void JSObjectProxyFinalizerCallback(napi_env env, void *finalizeData, void *finalizeHint);