From 1e1f35bd80e26d73dc092fc7a5100f1c0aeee754 Mon Sep 17 00:00:00 2001 From: Nathan Walker Date: Sat, 24 Oct 2026 11:57:37 -0700 Subject: [PATCH 1/3] fix(android): re-attach NativeScript after a JavaScript reload install() returned early on a process-wide flag, so after a reload the new jsi::Runtime had none of NativeScript's globals. The old com.tns.Runtime also stayed subscribed to GC notifications and released handles into the destroyed jsi::Runtime. - The TurboModule tracks the Runtime it attached; React Native creates one module per JS runtime, so isInstalled()/install() are per runtime. - invalidate() (after the JS thread stops, before RN destroys the runtime) detaches: unsubscribes from GcListener, releases every handle held in the jsi::Runtime and deletes the native Runtime and its EngineHost. The next attach becomes the main runtime again; metadata is built once. - GcListener delivers a batch under the lock unsubscribe() takes, so no GC thread can hold a runtime that is being torn down. notifyGc no longer throws on the GC thread for a runtime that is gone. - A Java callback into a reloaded runtime throws a NativeScriptException naming the class and method; ownership is remembered in a weak-keyed map. A failed post to a stopped JS thread throws instead of returning null. - Per-runtime teardown frees the LRU cache's weak global refs, the Java Runtime's global ref, and the static/extension method callback data MetadataNode never tracked. Applied to the napi tree as well. --- NativeScript/ffi/jni/jsi/jni/LRUCache.h | 10 +++ .../ffi/jni/jsi/metadata/MetadataNode.cpp | 16 +++++ .../ffi/jni/jsi/metadata/MetadataNode.h | 3 + .../jni/jsi/objectmanager/ObjectManager.cpp | 4 ++ NativeScript/ffi/jni/napi/jni/LRUCache.h | 10 +++ .../ffi/jni/napi/metadata/MetadataNode.cpp | 11 +++ .../ffi/jni/napi/metadata/MetadataNode.h | 3 + .../jni/napi/objectmanager/ObjectManager.cpp | 4 ++ NativeScript/runtime/android/jsi/EngineHost.h | 2 +- NativeScript/runtime/android/jsi/Runtime.cpp | 19 ++++++ NativeScript/runtime/android/jsi/Runtime.h | 5 ++ .../runtime/android/jsi/com_tns_Runtime.cpp | 8 ++- NativeScript/runtime/android/napi/Runtime.cpp | 2 + .../runtime/android/napi/com_tns_Runtime.cpp | 8 ++- .../src/main/cpp/NativeScriptRnInstall.cpp | 27 ++++---- .../NativeScriptNativeApiModule.java | 42 +++++++++--- .../src/main/java/com/tns/GcListener.java | 8 ++- .../src/main/java/com/tns/Runtime.java | 67 ++++++++++++++++--- 18 files changed, 212 insertions(+), 37 deletions(-) diff --git a/NativeScript/ffi/jni/jsi/jni/LRUCache.h b/NativeScript/ffi/jni/jsi/jni/LRUCache.h index 7b829c3dd..afceeb132 100644 --- a/NativeScript/ffi/jni/jsi/jni/LRUCache.h +++ b/NativeScript/ffi/jni/jsi/jni/LRUCache.h @@ -100,6 +100,16 @@ class LRUCache { } } + void clear() { + if (m_evictCallback != nullptr) { + for (auto& entry : m_key_to_value) { + m_evictCallback(entry.second.first, m_state); + } + } + m_key_to_value.clear(); + m_key_tracker.clear(); + } + void update(const key_type& key, const value_type& value) { jweak ref = m_loadCallback(key, m_state); insert(key, ref); diff --git a/NativeScript/ffi/jni/jsi/metadata/MetadataNode.cpp b/NativeScript/ffi/jni/jsi/metadata/MetadataNode.cpp index 6187a585d..e6189242e 100644 --- a/NativeScript/ffi/jni/jsi/metadata/MetadataNode.cpp +++ b/NativeScript/ffi/jni/jsi/metadata/MetadataNode.cpp @@ -1307,6 +1307,12 @@ std::vector MetadataNode::SetClassMembersFro callbackData->candidates.push_back(std::move(entry)); callbackData->objectManager = objectManager; } + for (const auto &extension: collectedExtensionMethods) { + if (std::find(instanceMethodData.begin(), instanceMethodData.end(), extension.second) == + instanceMethodData.end()) { + cache->methodCallbackData.push_back(extension.second); + } + } auto instanceFieldCount = *reinterpret_cast(curPtr); curPtr += sizeof(uint16_t); for (auto i = 0; i < instanceFieldCount; i++) { @@ -1392,6 +1398,7 @@ std::vector MetadataNode::SetClassMembersFro auto &methodName = entry.getName(); if (methodName != lastMethodName) { callbackData = new MethodCallbackData(this); + cache->methodCallbackData.push_back(callbackData); auto method = JsFunction::createFromHostFunction( rt, engine::PropNameID::forAscii(rt, methodName), 0, [callbackData](JsRuntime &rt, const JsValue &thisVal, const JsValue *args, @@ -2556,6 +2563,7 @@ void MetadataNode::SetMissingBaseMethods( if (callbackData == nullptr) { callbackData = new MethodCallbackData(this); + GetMetadataNodeCache(rt)->methodCallbackData.push_back(callbackData); auto proto = constructor.getPropertyAsObject(rt, "prototype"); auto method = JsFunction::createFromHostFunction( rt, engine::PropNameID::forAscii(rt, methodName), 0, @@ -2582,6 +2590,11 @@ void MetadataNode::SetMissingBaseMethods( } void MetadataNode::BuildMetadata(const std::string &filesPath) { + // Process-wide: the static node caches point into this tree, so a runtime + // attached after another was disposed keeps the one already built. + if (s_metadataReader.GetRoot() != nullptr) { + return; + } s_metadataReader = MetadataBuilder::BuildMetadata(filesPath); } @@ -2613,6 +2626,9 @@ void MetadataNode::onDisposeRuntime(JsRuntime &rt) { for (const auto &entry: it->extendedClassCallbackData) { delete entry; } + for (const auto &entry: it->methodCallbackData) { + delete entry; + } } s_metadata_node_cache.Remove(key); delete it; diff --git a/NativeScript/ffi/jni/jsi/metadata/MetadataNode.h b/NativeScript/ffi/jni/jsi/metadata/MetadataNode.h index 2c06df48a..265783b4a 100644 --- a/NativeScript/ffi/jni/jsi/metadata/MetadataNode.h +++ b/NativeScript/ffi/jni/jsi/metadata/MetadataNode.h @@ -373,6 +373,9 @@ class MetadataNode { std::vector fieldCallbackData; std::vector propertyCallbackData; std::vector extendedClassCallbackData; + // Method callback data no CtorFuncCache entry owns: static methods, + // extension-only methods and methods copied from a skipped base class. + std::vector methodCallbackData; }; static bool s_profilerEnabled; diff --git a/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp b/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp index 394a45446..08e000964 100644 --- a/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp +++ b/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp @@ -85,6 +85,10 @@ void ObjectManager::OnDisposeRuntime() { m_idToObject.clear(); m_jsObjectCtor = JsFunction(); + // Its destructor leaves the weak global refs alive, and the process-wide + // weak reference table is finite. + m_cache.clear(); + // The host-object proxies are owned by the *engine*, and each holds an owned // handle to the instance it wraps. Nothing above reaches them: they are only // destroyed when the engine tears its heap down, at which point diff --git a/NativeScript/ffi/jni/napi/jni/LRUCache.h b/NativeScript/ffi/jni/napi/jni/LRUCache.h index 7b829c3dd..afceeb132 100644 --- a/NativeScript/ffi/jni/napi/jni/LRUCache.h +++ b/NativeScript/ffi/jni/napi/jni/LRUCache.h @@ -100,6 +100,16 @@ class LRUCache { } } + void clear() { + if (m_evictCallback != nullptr) { + for (auto& entry : m_key_to_value) { + m_evictCallback(entry.second.first, m_state); + } + } + m_key_to_value.clear(); + m_key_tracker.clear(); + } + void update(const key_type& key, const value_type& value) { jweak ref = m_loadCallback(key, m_state); insert(key, ref); diff --git a/NativeScript/ffi/jni/napi/metadata/MetadataNode.cpp b/NativeScript/ffi/jni/napi/metadata/MetadataNode.cpp index 6e9e48219..582386eb5 100644 --- a/NativeScript/ffi/jni/napi/metadata/MetadataNode.cpp +++ b/NativeScript/ffi/jni/napi/metadata/MetadataNode.cpp @@ -1584,6 +1584,12 @@ std::vector MetadataNode::SetClassMembersFro callbackData->candidates.push_back(std::move(entry)); callbackData->objectManager = objectManager; } + for (const auto &extension: collectedExtensionMethods) { + if (std::find(instanceMethodData.begin(), instanceMethodData.end(), extension.second) == + instanceMethodData.end()) { + GetMetadataNodeCache(env)->methodCallbackData.push_back(extension.second); + } + } auto instanceFieldCount = *reinterpret_cast(curPtr); curPtr += sizeof(uint16_t); for (auto i = 0; i < instanceFieldCount; i++) { @@ -1657,6 +1663,7 @@ std::vector MetadataNode::SetClassMembersFro auto &methodName = entry.getName(); if (methodName != lastMethodName) { callbackData = new MethodCallbackData(this); + GetMetadataNodeCache(env)->methodCallbackData.push_back(callbackData); napi_value method; NAPI_GUARD(napi_create_function(env, methodName.c_str(), methodName.size(), MethodCallback, callbackData, &method)) {} @@ -3054,6 +3061,7 @@ void MetadataNode::SetMissingBaseMethods( if (callbackData == nullptr) { callbackData = new MethodCallbackData(this); + GetMetadataNodeCache(env)->methodCallbackData.push_back(callbackData); napi_value proto = napi_util::get_prototype(env, constructor); napi_value method; NAPI_GUARD(napi_create_function(env, methodName.c_str(), NAPI_AUTO_LENGTH, MethodCallback, @@ -3105,6 +3113,9 @@ void MetadataNode::onDisposeEnv(napi_env env) { for (const auto &entry: it->fieldCallbackData) { delete entry; } + for (const auto &entry: it->methodCallbackData) { + delete entry; + } } s_metadata_node_cache.Remove(env); delete it; diff --git a/NativeScript/ffi/jni/napi/metadata/MetadataNode.h b/NativeScript/ffi/jni/napi/metadata/MetadataNode.h index b6c699436..fabe0f507 100644 --- a/NativeScript/ffi/jni/napi/metadata/MetadataNode.h +++ b/NativeScript/ffi/jni/napi/metadata/MetadataNode.h @@ -360,6 +360,9 @@ class MetadataNode { robin_hood::unordered_map CtorFuncCache; robin_hood::unordered_map ExtendedCtorFuncCache; std::vector fieldCallbackData; + // Method callback data no CtorFuncCache entry owns: static methods, + // extension-only methods and methods copied from a skipped base class. + std::vector methodCallbackData; }; static bool s_profilerEnabled; diff --git a/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp b/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp index a5c67014a..7bafbfe41 100644 --- a/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp +++ b/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp @@ -92,6 +92,10 @@ void ObjectManager::OnDisposeEnv() { NAPI_GUARD(napi_delete_reference(m_env, entry.second)) {} } m_idToObject.clear(); + + // Its destructor leaves the weak global refs alive, and the process-wide + // weak reference table is finite. + m_cache.clear(); } napi_value ObjectManager::GetOrCreateProxyWeak(jint javaObjectID, napi_value instance) { diff --git a/NativeScript/runtime/android/jsi/EngineHost.h b/NativeScript/runtime/android/jsi/EngineHost.h index 31dffa0be..76c9e9e59 100644 --- a/NativeScript/runtime/android/jsi/EngineHost.h +++ b/NativeScript/runtime/android/jsi/EngineHost.h @@ -58,7 +58,7 @@ class EngineHost { #if defined(NS_JSI_HOST_RUNTIME) // Adopts a runtime the embedder owns. The reference must outlive this host: // React Native tears its runtime down only after the TurboModule that holds - // us has been invalidated, which is what ReleaseEngineState is wired to. + // us has been invalidated, which is what Runtime::Detach is wired to. static std::shared_ptr Adopt(::facebook::jsi::Runtime &runtime); #else static std::shared_ptr Create(); diff --git a/NativeScript/runtime/android/jsi/Runtime.cpp b/NativeScript/runtime/android/jsi/Runtime.cpp index 7991157d2..d68248ab7 100644 --- a/NativeScript/runtime/android/jsi/Runtime.cpp +++ b/NativeScript/runtime/android/jsi/Runtime.cpp @@ -265,6 +265,16 @@ void Runtime::Attach(::facebook::jsi::Runtime &hostRuntime, const std::string &f InitCommon(filesRoot, /*callingDir*/ "", maxLogcatObjectSize, forceLog, /*guest*/ true); } +void Runtime::Detach(Runtime *runtime) { + // `engineHost` is referenced by the JSEnterScope macro below. + std::shared_ptr engineHost = runtime->GetEngineHost(); + { + JSEnterScope + runtime->DestroyRuntime(); + } + delete runtime; +} + #endif // NS_JSI_HOST_RUNTIME // Everything both flavours install. @@ -546,7 +556,16 @@ Runtime::~Runtime() { if (m_mainLooper_fd[1] != -1) { close(m_mainLooper_fd[1]); } + + // A guest's main runtime is deleted when its host runtime goes away, and + // the next one attached may run on a different thread. + m_mainLooper = nullptr; + m_mainLooper_fd[0] = m_mainLooper_fd[1] = -1; + s_main_rt = nullptr; + s_mainThreadInitialized = false; } + + JEnv().DeleteGlobalRef(m_runtime); } std::string Runtime::ReadFileText(const std::string &filePath) { diff --git a/NativeScript/runtime/android/jsi/Runtime.h b/NativeScript/runtime/android/jsi/Runtime.h index 3b41c83a4..dde751437 100644 --- a/NativeScript/runtime/android/jsi/Runtime.h +++ b/NativeScript/runtime/android/jsi/Runtime.h @@ -90,6 +90,11 @@ namespace tns { void Attach(::facebook::jsi::Runtime &hostRuntime, const std::string &filesRoot, bool verboseLoggingEnabled, int maxLogcatObjectSize, bool forceLog); + + // Releases everything held in the host runtime and deletes `runtime`. The + // host runtime must still be alive with no JS running on it, and nothing + // may reach `runtime` through any thread once this begins. + static void Detach(Runtime *runtime); #endif jobject GetJavaRuntime() const; diff --git a/NativeScript/runtime/android/jsi/com_tns_Runtime.cpp b/NativeScript/runtime/android/jsi/com_tns_Runtime.cpp index 20ab03d8f..88e63f93c 100644 --- a/NativeScript/runtime/android/jsi/com_tns_Runtime.cpp +++ b/NativeScript/runtime/android/jsi/com_tns_Runtime.cpp @@ -273,8 +273,12 @@ extern "C" JNIEXPORT jint Java_com_tns_Runtime_generateNewObjectIdLegacy(JNIEnv* // @FastNative ABI: standard JNI signature (registered via RegisterNatives). static jboolean notifyGcFast_impl(JNIEnv* jEnv, jobject obj, jint runtimeId, jintArray object_ids) { - auto runtime = TryGetRuntime(runtimeId); - if (runtime == nullptr) { + // Not TryGetRuntime: on the GC listener's thread a Java exception goes + // uncaught, and a runtime that is gone has nothing left to release. + Runtime* runtime = nullptr; + try { + runtime = Runtime::GetRuntime(runtimeId); + } catch (NativeScriptException&) { return JNI_FALSE; } // The napi version passes false here to skip opening a napi handle scope. diff --git a/NativeScript/runtime/android/napi/Runtime.cpp b/NativeScript/runtime/android/napi/Runtime.cpp index 49fc0455f..c6f0ce75e 100644 --- a/NativeScript/runtime/android/napi/Runtime.cpp +++ b/NativeScript/runtime/android/napi/Runtime.cpp @@ -486,6 +486,8 @@ Runtime::~Runtime() { close(m_mainLooper_fd[1]); } } + + JEnv().DeleteGlobalRef(m_runtime); } std::string Runtime::ReadFileText(const std::string &filePath) { diff --git a/NativeScript/runtime/android/napi/com_tns_Runtime.cpp b/NativeScript/runtime/android/napi/com_tns_Runtime.cpp index 452104588..84046fb85 100644 --- a/NativeScript/runtime/android/napi/com_tns_Runtime.cpp +++ b/NativeScript/runtime/android/napi/com_tns_Runtime.cpp @@ -253,8 +253,12 @@ extern "C" JNIEXPORT jint Java_com_tns_Runtime_generateNewObjectIdLegacy(JNIEnv* // @FastNative ABI: standard JNI signature (registered via RegisterNatives). static jboolean notifyGcFast_impl(JNIEnv* jEnv, jobject obj, jint runtimeId, jintArray object_ids) { - auto runtime = TryGetRuntime(runtimeId); - if (runtime == nullptr) { + // Not TryGetRuntime: on the GC listener's thread a Java exception goes + // uncaught, and a runtime that is gone has nothing left to release. + Runtime* runtime = nullptr; + try { + runtime = Runtime::GetRuntime(runtimeId); + } catch (NativeScriptException&) { return JNI_FALSE; } NapiScope scope(runtime->GetNapiEnv(), false); diff --git a/packages/react-native/android/src/main/cpp/NativeScriptRnInstall.cpp b/packages/react-native/android/src/main/cpp/NativeScriptRnInstall.cpp index fcf76578f..e0c665bd7 100644 --- a/packages/react-native/android/src/main/cpp/NativeScriptRnInstall.cpp +++ b/packages/react-native/android/src/main/cpp/NativeScriptRnInstall.cpp @@ -18,15 +18,6 @@ using namespace tns; -namespace { - -// Set once the first attach succeeds. React Native can reload the JS bundle, -// which builds a fresh jsi::Runtime; the module checks this before installing -// so a reload rebinds rather than stacking a second runtime on the first. -bool g_attached = false; - -} // namespace - extern "C" JNIEXPORT void JNICALL Java_com_tns_Runtime_attachNativeScript( JNIEnv* env, jobject obj, jint runtimeId, jstring metadataPath, jlong hostRuntimePtr, jboolean verboseLoggingEnabled, jint maxLogcatObjectSize, jboolean forceLog, @@ -58,8 +49,6 @@ extern "C" JNIEXPORT void JNICALL Java_com_tns_Runtime_attachNativeScript( runtime->GetEngineHost()->ExecuteScript(source, "internal/ts_helpers.js"); } } - - g_attached = true; } catch (NativeScriptException& e) { e.ReThrowToJava(nullptr); } catch (std::exception& e) { @@ -71,9 +60,19 @@ extern "C" JNIEXPORT void JNICALL Java_com_tns_Runtime_attachNativeScript( } } -extern "C" JNIEXPORT jboolean JNICALL -Java_org_nativescript_reactnative_NativeScriptNativeApiModule_nativeIsInstalled(JNIEnv*, jobject) { - return g_attached ? JNI_TRUE : JNI_FALSE; +extern "C" JNIEXPORT void JNICALL Java_com_tns_Runtime_detachNativeScript(JNIEnv*, jobject, + jint runtimeId) { + try { + Runtime::Detach(Runtime::GetRuntime(runtimeId)); + } catch (NativeScriptException& e) { + e.ReThrowToJava(nullptr); + } catch (std::exception& e) { + NativeScriptException nsEx(std::string("Error: c++ exception: ") + e.what()); + nsEx.ReThrowToJava(nullptr); + } catch (...) { + NativeScriptException nsEx(std::string("Error: c++ exception!")); + nsEx.ReThrowToJava(nullptr); + } } extern "C" JNIEXPORT jstring JNICALL diff --git a/packages/react-native/android/src/main/java/org/nativescript/reactnative/NativeScriptNativeApiModule.java b/packages/react-native/android/src/main/java/org/nativescript/reactnative/NativeScriptNativeApiModule.java index d07d099b8..3b5e5508e 100644 --- a/packages/react-native/android/src/main/java/org/nativescript/reactnative/NativeScriptNativeApiModule.java +++ b/packages/react-native/android/src/main/java/org/nativescript/reactnative/NativeScriptNativeApiModule.java @@ -3,6 +3,7 @@ import android.content.Context; import com.facebook.react.bridge.ReactApplicationContext; +import com.facebook.react.bridge.queue.MessageQueueThread; import com.facebook.react.module.annotations.ReactModule; import com.tns.Logger; @@ -23,6 +24,11 @@ * thread -- it is a synchronous TurboModule method, so React Native already * guarantees that. Everything it does happens against the runtime React Native * created; nothing here starts a JavaScript engine. + * + *

React Native creates one instance of this module per JavaScript runtime and + * invalidates it before destroying that runtime, which is where NativeScript + * lets go of it. A reloaded bundle installs into its new runtime through a new + * instance. */ @ReactModule(name = NativeScriptNativeApiModule.NAME) public class NativeScriptNativeApiModule extends NativeScriptNativeApiSpec { @@ -44,6 +50,8 @@ public class NativeScriptNativeApiModule extends NativeScriptNativeApiSpec { private final ReactApplicationContext reactContext; + private volatile Runtime attachedRuntime; + public NativeScriptNativeApiModule(ReactApplicationContext reactContext) { super(reactContext); this.reactContext = reactContext; @@ -54,13 +62,11 @@ public String getName() { return NAME; } - private native boolean nativeIsInstalled(); - private native String nativeRuntimeBackend(); @Override public boolean install(String metadataPath) { - if (nativeIsInstalled()) { + if (attachedRuntime != null) { return true; } @@ -88,10 +94,30 @@ public boolean install(String metadataPath) { + "JSI runtime and cannot run over the legacy bridge."); } - attachRuntime(context, runtimePtr, resolvedPath, readAsset(context, ASSET_TS_HELPERS)); + attachedRuntime = attachRuntime(context, runtimePtr, resolvedPath, + readAsset(context, ASSET_TS_HELPERS)); return true; } + @Override + public void invalidate() { + Runtime runtime = attachedRuntime; + attachedRuntime = null; + if (runtime != null) { + // Detaching releases handles into the jsi::Runtime, so it has to run + // before React Native destroys that runtime and never alongside JS. + // Bridgeless has already stopped the JS thread here, so the post is + // refused and this thread is safe. The bridge can invalidate from + // another thread while JS still runs, and queues the runtime's + // destruction on the JS thread behind this. + MessageQueueThread jsThread = reactContext.getJSMessageQueueThread(); + if (jsThread == null || jsThread.isOnThread() || !jsThread.runOnQueue(runtime::detach)) { + runtime.detach(); + } + } + super.invalidate(); + } + @Override public boolean installWorkletRuntime(com.facebook.react.bridge.ReadableMap runtimeHolder, String metadataPath) { @@ -104,7 +130,7 @@ public boolean installWorkletRuntime(com.facebook.react.bridge.ReadableMap runti @Override public boolean isInstalled() { - return nativeIsInstalled(); + return attachedRuntime != null; } /** @@ -138,8 +164,8 @@ public boolean __writeTestMarker(String content) { } } - private static void attachRuntime(Context context, long runtimePtr, String metadataPath, - String bootstrapScript) { + private static Runtime attachRuntime(Context context, long runtimePtr, String metadataPath, + String bootstrapScript) { File filesDir = context.getFilesDir(); File dexDir = new File(filesDir, "nativescript-dex"); if (!dexDir.exists() && !dexDir.mkdirs()) { @@ -148,7 +174,7 @@ private static void attachRuntime(Context context, long runtimePtr, String metad Logger logger = new NativeScriptLogger(isDebuggable(context)); - Runtime.attachToHostRuntime( + return Runtime.attachToHostRuntime( logger, context.getPackageName(), context.getApplicationInfo().nativeLibraryDir, diff --git a/platforms/android/test-app/runtime/src/main/java/com/tns/GcListener.java b/platforms/android/test-app/runtime/src/main/java/com/tns/GcListener.java index 0d5a69892..b7689ec99 100644 --- a/platforms/android/test-app/runtime/src/main/java/com/tns/GcListener.java +++ b/platforms/android/test-app/runtime/src/main/java/com/tns/GcListener.java @@ -60,7 +60,13 @@ public void run() { for (int i =0; i < objIds.length; i++) { objIds[i] = collectedObjectIds.get(i); } - runtime.notifyGc(objIds); + // unsubscribe() takes this lock too, so once it returns + // no batch is still on its way into that runtime. + synchronized (instance) { + if (subscribers.get(runtime) == subscriber) { + runtime.notifyGc(objIds); + } + } if (runtime.getLogger().isEnabled()) { runtime.getLogger().write("GC", "GC triggered for " + String.valueOf(objIds.length) + " objects"); runtime.getLogger().write("NS.GCListener","Objects referenced after GC: " + subscriber.referencesMap.size()); diff --git a/platforms/android/test-app/runtime/src/main/java/com/tns/Runtime.java b/platforms/android/test-app/runtime/src/main/java/com/tns/Runtime.java index 739c10824..16dc997ca 100644 --- a/platforms/android/test-app/runtime/src/main/java/com/tns/Runtime.java +++ b/platforms/android/test-app/runtime/src/main/java/com/tns/Runtime.java @@ -53,6 +53,9 @@ private native void attachNativeScript(int runtimeId, String metadataPath, long boolean verboseLoggingEnabled, int maxLogcatObjectSize, boolean forceLog, String bootstrapScript); + // Implemented alongside attachNativeScript; see detach(). + private native void detachNativeScript(int runtimeId); + private native void runModule(int runtimeId, String filePath) throws NativeScriptException; private native Object runScript(int runtimeId, String filePath) throws NativeScriptException; @@ -225,6 +228,10 @@ public int compare(Method lhs, Method rhs) { private static AtomicInteger nextRuntimeId = new AtomicInteger(0); private final static ThreadLocal currentRuntime = new ThreadLocal(); private final static Map runtimeCache = new ConcurrentHashMap<>(); + // Objects owned by a detached runtime, so a call into one can say why it + // fails. Outlives every Runtime by design; the weak keys pin nothing. + private final static Map orphanedObjects = + Collections.synchronizedMap(new NativeScriptWeakHashMap()); public static boolean nativeLibraryLoaded; private static final ClassStorageService classStorageService = new ClassStorageServiceImpl(ClassCacheImpl.INSTANCE, ClassLoadersCollectionImpl.INSTANCE); @@ -520,6 +527,42 @@ private void attach(long hostRuntimePtr, String metadataPath, String bootstrapSc initialized = true; } + /* + Unbinds from the embedder's runtime and frees everything NativeScript + holds for it. Must run while that runtime is still alive and not running + JS: React Native's module invalidation is such a point. Idempotent. The + instance cannot be attached again; a new host runtime gets a new Runtime. + */ + public void detach() { + if (!initialized) { + return; + } + initialized = false; + + // Returns only once no GC batch is being delivered to this runtime. + GcListener.unsubscribe(this); + + for (Object instance : strongJavaObjectToID.keySet()) { + orphanedObjects.put(instance, runtimeId); + } + for (Object instance : weakJavaObjectToID.keySet()) { + if (instance != null) { + orphanedObjects.put(instance, runtimeId); + } + } + runtimeCache.remove(runtimeId); + if (currentRuntime.get() == this) { + currentRuntime.remove(); + } + + detachNativeScript(runtimeId); + + strongInstances.clear(); + strongJavaObjectToID.clear(); + weakInstances.clear(); + weakJavaObjectToID.clear(); + } + /* Called via JNI from a native-spawned worker thread (WorkerWrapper), after the thread has attached to the JVM and before the worker script runs. @@ -1225,6 +1268,11 @@ public static Object callJSMethod(int runtimeId, Object javaObject, String metho javaObjectID = (runtime != null) ? runtime.getJavaObjectID(javaObject) : null; } + if (runtime == null && javaObject != null && orphanedObjects.containsKey(javaObject)) { + throw new NativeScriptException("Cannot call " + javaObject.getClass().getName() + "." + methodName + + "(): the JavaScript runtime it belonged to was reloaded or shut down."); + } + if (runtime == null) { runtime = Runtime.getCurrentRuntime(); javaObjectID = (runtime != null) ? runtime.getJavaObjectID(javaObject) : null; @@ -1405,17 +1453,18 @@ public void run() { } } - boolean success = threadScheduler.post(r); + if (!threadScheduler.post(r)) { + throw new NativeScriptException("Cannot call " + claz.getName() + "." + methodName + + "(): the JavaScript thread it runs on has stopped."); + } - if (success) { - synchronized (r) { - try { - if (arr[1] == null) { - r.wait(); - } - } catch (InterruptedException e) { - ret = e; + synchronized (r) { + try { + if (arr[1] == null) { + r.wait(); } + } catch (InterruptedException e) { + ret = e; } } From 900ee4ca42826177d2c266a19db409a8db146cfd Mon Sep 17 00:00:00 2001 From: Nathan Walker Date: Sun, 25 Oct 2026 13:42:11 -0700 Subject: [PATCH 2/3] refactor(android): detach through the worker teardown path Narrows the reload fix to what React Native needs: the module tracks its own install, invalidate() detaches, and detaching reuses the teardown a worker already goes through. - Runtime.detach() is detachRuntime() (formerly detachWorkerRuntime) plus Runtime::DisposeRuntime (formerly DisposeWorkerRuntime), now built in the host-runtime flavour too. The separate Runtime::Detach is gone. - ~Runtime still resets the main-runtime statics, and BuildMetadata still builds the tree once. Dropped, to land separately or not at all: throwing on a failed post to the JS thread, the GcListener delivery lock and the notifyGc change, the orphaned-object map, the LRU cache clear, the global-ref delete in ~Runtime, MethodCallbackData ownership, and their napi mirrors. --- NativeScript/ffi/jni/jsi/jni/LRUCache.h | 10 --- .../ffi/jni/jsi/metadata/MetadataNode.cpp | 15 +--- .../ffi/jni/jsi/metadata/MetadataNode.h | 3 - .../jni/jsi/objectmanager/ObjectManager.cpp | 4 -- NativeScript/ffi/jni/napi/jni/LRUCache.h | 10 --- .../ffi/jni/napi/metadata/MetadataNode.cpp | 11 --- .../ffi/jni/napi/metadata/MetadataNode.h | 3 - .../jni/napi/objectmanager/ObjectManager.cpp | 4 -- NativeScript/runtime/android/jsi/EngineHost.h | 4 +- NativeScript/runtime/android/jsi/Runtime.cpp | 38 ++++------ NativeScript/runtime/android/jsi/Runtime.h | 14 ++-- .../runtime/android/jsi/com_tns_Runtime.cpp | 8 +-- .../android/jsi/workers/WorkerWrapper.cpp | 6 +- NativeScript/runtime/android/napi/Runtime.cpp | 2 - .../runtime/android/napi/com_tns_Runtime.cpp | 8 +-- .../android/napi/workers/WorkerWrapper.cpp | 2 +- .../src/main/cpp/NativeScriptRnInstall.cpp | 2 +- .../src/main/java/com/tns/GcListener.java | 8 +-- .../src/main/java/com/tns/Runtime.java | 69 +++++-------------- 19 files changed, 47 insertions(+), 174 deletions(-) diff --git a/NativeScript/ffi/jni/jsi/jni/LRUCache.h b/NativeScript/ffi/jni/jsi/jni/LRUCache.h index afceeb132..7b829c3dd 100644 --- a/NativeScript/ffi/jni/jsi/jni/LRUCache.h +++ b/NativeScript/ffi/jni/jsi/jni/LRUCache.h @@ -100,16 +100,6 @@ class LRUCache { } } - void clear() { - if (m_evictCallback != nullptr) { - for (auto& entry : m_key_to_value) { - m_evictCallback(entry.second.first, m_state); - } - } - m_key_to_value.clear(); - m_key_tracker.clear(); - } - void update(const key_type& key, const value_type& value) { jweak ref = m_loadCallback(key, m_state); insert(key, ref); diff --git a/NativeScript/ffi/jni/jsi/metadata/MetadataNode.cpp b/NativeScript/ffi/jni/jsi/metadata/MetadataNode.cpp index e6189242e..12f85ad54 100644 --- a/NativeScript/ffi/jni/jsi/metadata/MetadataNode.cpp +++ b/NativeScript/ffi/jni/jsi/metadata/MetadataNode.cpp @@ -1307,12 +1307,6 @@ std::vector MetadataNode::SetClassMembersFro callbackData->candidates.push_back(std::move(entry)); callbackData->objectManager = objectManager; } - for (const auto &extension: collectedExtensionMethods) { - if (std::find(instanceMethodData.begin(), instanceMethodData.end(), extension.second) == - instanceMethodData.end()) { - cache->methodCallbackData.push_back(extension.second); - } - } auto instanceFieldCount = *reinterpret_cast(curPtr); curPtr += sizeof(uint16_t); for (auto i = 0; i < instanceFieldCount; i++) { @@ -1398,7 +1392,6 @@ std::vector MetadataNode::SetClassMembersFro auto &methodName = entry.getName(); if (methodName != lastMethodName) { callbackData = new MethodCallbackData(this); - cache->methodCallbackData.push_back(callbackData); auto method = JsFunction::createFromHostFunction( rt, engine::PropNameID::forAscii(rt, methodName), 0, [callbackData](JsRuntime &rt, const JsValue &thisVal, const JsValue *args, @@ -2563,7 +2556,6 @@ void MetadataNode::SetMissingBaseMethods( if (callbackData == nullptr) { callbackData = new MethodCallbackData(this); - GetMetadataNodeCache(rt)->methodCallbackData.push_back(callbackData); auto proto = constructor.getPropertyAsObject(rt, "prototype"); auto method = JsFunction::createFromHostFunction( rt, engine::PropNameID::forAscii(rt, methodName), 0, @@ -2590,8 +2582,8 @@ void MetadataNode::SetMissingBaseMethods( } void MetadataNode::BuildMetadata(const std::string &filesPath) { - // Process-wide: the static node caches point into this tree, so a runtime - // attached after another was disposed keeps the one already built. + // Once per process: the static node caches point into this tree, and it + // outlives any one main runtime. if (s_metadataReader.GetRoot() != nullptr) { return; } @@ -2626,9 +2618,6 @@ void MetadataNode::onDisposeRuntime(JsRuntime &rt) { for (const auto &entry: it->extendedClassCallbackData) { delete entry; } - for (const auto &entry: it->methodCallbackData) { - delete entry; - } } s_metadata_node_cache.Remove(key); delete it; diff --git a/NativeScript/ffi/jni/jsi/metadata/MetadataNode.h b/NativeScript/ffi/jni/jsi/metadata/MetadataNode.h index 265783b4a..2c06df48a 100644 --- a/NativeScript/ffi/jni/jsi/metadata/MetadataNode.h +++ b/NativeScript/ffi/jni/jsi/metadata/MetadataNode.h @@ -373,9 +373,6 @@ class MetadataNode { std::vector fieldCallbackData; std::vector propertyCallbackData; std::vector extendedClassCallbackData; - // Method callback data no CtorFuncCache entry owns: static methods, - // extension-only methods and methods copied from a skipped base class. - std::vector methodCallbackData; }; static bool s_profilerEnabled; diff --git a/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp b/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp index 08e000964..394a45446 100644 --- a/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp +++ b/NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp @@ -85,10 +85,6 @@ void ObjectManager::OnDisposeRuntime() { m_idToObject.clear(); m_jsObjectCtor = JsFunction(); - // Its destructor leaves the weak global refs alive, and the process-wide - // weak reference table is finite. - m_cache.clear(); - // The host-object proxies are owned by the *engine*, and each holds an owned // handle to the instance it wraps. Nothing above reaches them: they are only // destroyed when the engine tears its heap down, at which point diff --git a/NativeScript/ffi/jni/napi/jni/LRUCache.h b/NativeScript/ffi/jni/napi/jni/LRUCache.h index afceeb132..7b829c3dd 100644 --- a/NativeScript/ffi/jni/napi/jni/LRUCache.h +++ b/NativeScript/ffi/jni/napi/jni/LRUCache.h @@ -100,16 +100,6 @@ class LRUCache { } } - void clear() { - if (m_evictCallback != nullptr) { - for (auto& entry : m_key_to_value) { - m_evictCallback(entry.second.first, m_state); - } - } - m_key_to_value.clear(); - m_key_tracker.clear(); - } - void update(const key_type& key, const value_type& value) { jweak ref = m_loadCallback(key, m_state); insert(key, ref); diff --git a/NativeScript/ffi/jni/napi/metadata/MetadataNode.cpp b/NativeScript/ffi/jni/napi/metadata/MetadataNode.cpp index 582386eb5..6e9e48219 100644 --- a/NativeScript/ffi/jni/napi/metadata/MetadataNode.cpp +++ b/NativeScript/ffi/jni/napi/metadata/MetadataNode.cpp @@ -1584,12 +1584,6 @@ std::vector MetadataNode::SetClassMembersFro callbackData->candidates.push_back(std::move(entry)); callbackData->objectManager = objectManager; } - for (const auto &extension: collectedExtensionMethods) { - if (std::find(instanceMethodData.begin(), instanceMethodData.end(), extension.second) == - instanceMethodData.end()) { - GetMetadataNodeCache(env)->methodCallbackData.push_back(extension.second); - } - } auto instanceFieldCount = *reinterpret_cast(curPtr); curPtr += sizeof(uint16_t); for (auto i = 0; i < instanceFieldCount; i++) { @@ -1663,7 +1657,6 @@ std::vector MetadataNode::SetClassMembersFro auto &methodName = entry.getName(); if (methodName != lastMethodName) { callbackData = new MethodCallbackData(this); - GetMetadataNodeCache(env)->methodCallbackData.push_back(callbackData); napi_value method; NAPI_GUARD(napi_create_function(env, methodName.c_str(), methodName.size(), MethodCallback, callbackData, &method)) {} @@ -3061,7 +3054,6 @@ void MetadataNode::SetMissingBaseMethods( if (callbackData == nullptr) { callbackData = new MethodCallbackData(this); - GetMetadataNodeCache(env)->methodCallbackData.push_back(callbackData); napi_value proto = napi_util::get_prototype(env, constructor); napi_value method; NAPI_GUARD(napi_create_function(env, methodName.c_str(), NAPI_AUTO_LENGTH, MethodCallback, @@ -3113,9 +3105,6 @@ void MetadataNode::onDisposeEnv(napi_env env) { for (const auto &entry: it->fieldCallbackData) { delete entry; } - for (const auto &entry: it->methodCallbackData) { - delete entry; - } } s_metadata_node_cache.Remove(env); delete it; diff --git a/NativeScript/ffi/jni/napi/metadata/MetadataNode.h b/NativeScript/ffi/jni/napi/metadata/MetadataNode.h index fabe0f507..b6c699436 100644 --- a/NativeScript/ffi/jni/napi/metadata/MetadataNode.h +++ b/NativeScript/ffi/jni/napi/metadata/MetadataNode.h @@ -360,9 +360,6 @@ class MetadataNode { robin_hood::unordered_map CtorFuncCache; robin_hood::unordered_map ExtendedCtorFuncCache; std::vector fieldCallbackData; - // Method callback data no CtorFuncCache entry owns: static methods, - // extension-only methods and methods copied from a skipped base class. - std::vector methodCallbackData; }; static bool s_profilerEnabled; diff --git a/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp b/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp index 7bafbfe41..a5c67014a 100644 --- a/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp +++ b/NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp @@ -92,10 +92,6 @@ void ObjectManager::OnDisposeEnv() { NAPI_GUARD(napi_delete_reference(m_env, entry.second)) {} } m_idToObject.clear(); - - // Its destructor leaves the weak global refs alive, and the process-wide - // weak reference table is finite. - m_cache.clear(); } napi_value ObjectManager::GetOrCreateProxyWeak(jint javaObjectID, napi_value instance) { diff --git a/NativeScript/runtime/android/jsi/EngineHost.h b/NativeScript/runtime/android/jsi/EngineHost.h index 76c9e9e59..9a6016cdf 100644 --- a/NativeScript/runtime/android/jsi/EngineHost.h +++ b/NativeScript/runtime/android/jsi/EngineHost.h @@ -11,7 +11,7 @@ // EngineHost is held by shared_ptr and JSScope keeps a strong reference for its // lifetime. That is what makes teardown safe by construction: the VM, and the // recursive mutex the scope holds, cannot be freed while a scope is still -// unwinding over them. See Runtime::DisposeWorkerRuntime. +// unwinding over them. See Runtime::DisposeRuntime. #include #include @@ -58,7 +58,7 @@ class EngineHost { #if defined(NS_JSI_HOST_RUNTIME) // Adopts a runtime the embedder owns. The reference must outlive this host: // React Native tears its runtime down only after the TurboModule that holds - // us has been invalidated, which is what Runtime::Detach is wired to. + // us has been invalidated, which is where Runtime::DisposeRuntime runs. static std::shared_ptr Adopt(::facebook::jsi::Runtime &runtime); #else static std::shared_ptr Create(); diff --git a/NativeScript/runtime/android/jsi/Runtime.cpp b/NativeScript/runtime/android/jsi/Runtime.cpp index d68248ab7..dd4306f25 100644 --- a/NativeScript/runtime/android/jsi/Runtime.cpp +++ b/NativeScript/runtime/android/jsi/Runtime.cpp @@ -265,16 +265,6 @@ void Runtime::Attach(::facebook::jsi::Runtime &hostRuntime, const std::string &f InitCommon(filesRoot, /*callingDir*/ "", maxLogcatObjectSize, forceLog, /*guest*/ true); } -void Runtime::Detach(Runtime *runtime) { - // `engineHost` is referenced by the JSEnterScope macro below. - std::shared_ptr engineHost = runtime->GetEngineHost(); - { - JSEnterScope - runtime->DestroyRuntime(); - } - delete runtime; -} - #endif // NS_JSI_HOST_RUNTIME // Everything both flavours install. @@ -564,8 +554,6 @@ Runtime::~Runtime() { s_main_rt = nullptr; s_mainThreadInitialized = false; } - - JEnv().DeleteGlobalRef(m_runtime); } std::string Runtime::ReadFileText(const std::string &filePath) { @@ -624,6 +612,19 @@ void Runtime::DestroyRuntime() { // and ~EngineHost does the teardown once nothing is standing on it. } +void Runtime::DisposeRuntime(Runtime *runtime) { + // `engineHost` is referenced by the JSEnterScope macro below. + std::shared_ptr engineHost = runtime->GetEngineHost(); + { + JSEnterScope + runtime->DestroyRuntime(); + } + // Both this scope's reference and the runtime's are gone by the end of the + // next line, and whichever drops last runs ~EngineHost. Nothing frees the VM + // while the scope above is still unwinding over it. + delete runtime; +} + bool Runtime::NotifyGC(JNIEnv *jEnv, jobject obj, jintArray object_ids) { if (this->is_destroying) return true; m_objectManager->OnGarbageCollected(jEnv, object_ids); @@ -692,19 +693,6 @@ void Runtime::RunWorker(const std::string &filePath) { m_module.LoadWorker(engineHost->GetRuntime(), filePath); } -void Runtime::DisposeWorkerRuntime(Runtime *runtime) { - // `engineHost` is referenced by the JSEnterScope macro below. - std::shared_ptr engineHost = runtime->GetEngineHost(); - { - JSEnterScope - runtime->DestroyRuntime(); - } - // Both this scope's reference and the runtime's are gone by the end of the - // next line, and whichever drops last runs ~EngineHost. Nothing frees the VM - // while the scope above is still unwinding over it. - delete runtime; -} - jobject Runtime::RunScript(JNIEnv *_env, jobject obj, jstring scriptFile) { auto filename = ArgConverter::jstringToString(scriptFile); auto sourceUrl = ModuleInternal::EnsureFileProtocol(filename); diff --git a/NativeScript/runtime/android/jsi/Runtime.h b/NativeScript/runtime/android/jsi/Runtime.h index dde751437..5088e1dc7 100644 --- a/NativeScript/runtime/android/jsi/Runtime.h +++ b/NativeScript/runtime/android/jsi/Runtime.h @@ -90,11 +90,6 @@ namespace tns { void Attach(::facebook::jsi::Runtime &hostRuntime, const std::string &filesRoot, bool verboseLoggingEnabled, int maxLogcatObjectSize, bool forceLog); - - // Releases everything held in the host runtime and deletes `runtime`. The - // host runtime must still be alive with no JS running on it, and nothing - // may reach `runtime` through any thread once this begins. - static void Detach(Runtime *runtime); #endif jobject GetJavaRuntime() const; @@ -109,11 +104,10 @@ namespace tns { void RunWorker(const std::string &filePath); #endif - // Tears down a worker's Runtime (engine scope + engine state release) and - // deletes it. Called from the native worker thread during shutdown. -#if !defined(NS_JSI_HOST_RUNTIME) - static void DisposeWorkerRuntime(Runtime *runtime); -#endif + // Tears down a Runtime inside its engine scope and deletes it. The engine + // must still be alive with no JS running on it, and nothing may reach + // `runtime` from another thread once this begins. + static void DisposeRuntime(Runtime *runtime); jobject RunScript(JNIEnv *_env, jobject obj, jstring scriptFile); diff --git a/NativeScript/runtime/android/jsi/com_tns_Runtime.cpp b/NativeScript/runtime/android/jsi/com_tns_Runtime.cpp index 88e63f93c..20ab03d8f 100644 --- a/NativeScript/runtime/android/jsi/com_tns_Runtime.cpp +++ b/NativeScript/runtime/android/jsi/com_tns_Runtime.cpp @@ -273,12 +273,8 @@ extern "C" JNIEXPORT jint Java_com_tns_Runtime_generateNewObjectIdLegacy(JNIEnv* // @FastNative ABI: standard JNI signature (registered via RegisterNatives). static jboolean notifyGcFast_impl(JNIEnv* jEnv, jobject obj, jint runtimeId, jintArray object_ids) { - // Not TryGetRuntime: on the GC listener's thread a Java exception goes - // uncaught, and a runtime that is gone has nothing left to release. - Runtime* runtime = nullptr; - try { - runtime = Runtime::GetRuntime(runtimeId); - } catch (NativeScriptException&) { + auto runtime = TryGetRuntime(runtimeId); + if (runtime == nullptr) { return JNI_FALSE; } // The napi version passes false here to skip opening a napi handle scope. diff --git a/NativeScript/runtime/android/jsi/workers/WorkerWrapper.cpp b/NativeScript/runtime/android/jsi/workers/WorkerWrapper.cpp index 5bd5b3b7f..86c26c576 100644 --- a/NativeScript/runtime/android/jsi/workers/WorkerWrapper.cpp +++ b/NativeScript/runtime/android/jsi/workers/WorkerWrapper.cpp @@ -422,10 +422,10 @@ void WorkerWrapper::BackgroundLooper(std::shared_ptr self) { workerRt_.store(nullptr); // Enters the engine scope, tears the runtime down and deletes it. - Runtime::DisposeWorkerRuntime(runtime_); + Runtime::DisposeRuntime(runtime_); runtime_ = nullptr; // The last reference this thread holds to the worker VM, and it is - // dropped only after DisposeWorkerRuntime's scope has fully unwound. + // dropped only after DisposeRuntime's scope has fully unwound. workerHost_.reset(); } @@ -529,7 +529,7 @@ void WorkerWrapper::EnsureJniCached() { env.GetStaticMethodID(RUNTIME_CLASS, "initWorkerRuntime", "(ILjava/lang/String;)I"); RUN_WORKER_LOOP_METHOD_ID = env.GetStaticMethodID(RUNTIME_CLASS, "runWorkerLoop", "()V"); DETACH_WORKER_RUNTIME_METHOD_ID = - env.GetStaticMethodID(RUNTIME_CLASS, "detachWorkerRuntime", "(I)V"); + env.GetStaticMethodID(RUNTIME_CLASS, "detachRuntime", "(I)V"); LOOPER_CLASS = env.FindClass("android/os/Looper"); assert(LOOPER_CLASS != nullptr); diff --git a/NativeScript/runtime/android/napi/Runtime.cpp b/NativeScript/runtime/android/napi/Runtime.cpp index c6f0ce75e..49fc0455f 100644 --- a/NativeScript/runtime/android/napi/Runtime.cpp +++ b/NativeScript/runtime/android/napi/Runtime.cpp @@ -486,8 +486,6 @@ Runtime::~Runtime() { close(m_mainLooper_fd[1]); } } - - JEnv().DeleteGlobalRef(m_runtime); } std::string Runtime::ReadFileText(const std::string &filePath) { diff --git a/NativeScript/runtime/android/napi/com_tns_Runtime.cpp b/NativeScript/runtime/android/napi/com_tns_Runtime.cpp index 84046fb85..452104588 100644 --- a/NativeScript/runtime/android/napi/com_tns_Runtime.cpp +++ b/NativeScript/runtime/android/napi/com_tns_Runtime.cpp @@ -253,12 +253,8 @@ extern "C" JNIEXPORT jint Java_com_tns_Runtime_generateNewObjectIdLegacy(JNIEnv* // @FastNative ABI: standard JNI signature (registered via RegisterNatives). static jboolean notifyGcFast_impl(JNIEnv* jEnv, jobject obj, jint runtimeId, jintArray object_ids) { - // Not TryGetRuntime: on the GC listener's thread a Java exception goes - // uncaught, and a runtime that is gone has nothing left to release. - Runtime* runtime = nullptr; - try { - runtime = Runtime::GetRuntime(runtimeId); - } catch (NativeScriptException&) { + auto runtime = TryGetRuntime(runtimeId); + if (runtime == nullptr) { return JNI_FALSE; } NapiScope scope(runtime->GetNapiEnv(), false); diff --git a/NativeScript/runtime/android/napi/workers/WorkerWrapper.cpp b/NativeScript/runtime/android/napi/workers/WorkerWrapper.cpp index 21fa72e02..be943c0be 100644 --- a/NativeScript/runtime/android/napi/workers/WorkerWrapper.cpp +++ b/NativeScript/runtime/android/napi/workers/WorkerWrapper.cpp @@ -604,7 +604,7 @@ void WorkerWrapper::EnsureJniCached() { env.GetStaticMethodID(RUNTIME_CLASS, "initWorkerRuntime", "(ILjava/lang/String;)I"); RUN_WORKER_LOOP_METHOD_ID = env.GetStaticMethodID(RUNTIME_CLASS, "runWorkerLoop", "()V"); DETACH_WORKER_RUNTIME_METHOD_ID = - env.GetStaticMethodID(RUNTIME_CLASS, "detachWorkerRuntime", "(I)V"); + env.GetStaticMethodID(RUNTIME_CLASS, "detachRuntime", "(I)V"); LOOPER_CLASS = env.FindClass("android/os/Looper"); assert(LOOPER_CLASS != nullptr); diff --git a/packages/react-native/android/src/main/cpp/NativeScriptRnInstall.cpp b/packages/react-native/android/src/main/cpp/NativeScriptRnInstall.cpp index e0c665bd7..f08e3c5f2 100644 --- a/packages/react-native/android/src/main/cpp/NativeScriptRnInstall.cpp +++ b/packages/react-native/android/src/main/cpp/NativeScriptRnInstall.cpp @@ -63,7 +63,7 @@ extern "C" JNIEXPORT void JNICALL Java_com_tns_Runtime_attachNativeScript( extern "C" JNIEXPORT void JNICALL Java_com_tns_Runtime_detachNativeScript(JNIEnv*, jobject, jint runtimeId) { try { - Runtime::Detach(Runtime::GetRuntime(runtimeId)); + Runtime::DisposeRuntime(Runtime::GetRuntime(runtimeId)); } catch (NativeScriptException& e) { e.ReThrowToJava(nullptr); } catch (std::exception& e) { diff --git a/platforms/android/test-app/runtime/src/main/java/com/tns/GcListener.java b/platforms/android/test-app/runtime/src/main/java/com/tns/GcListener.java index b7689ec99..0d5a69892 100644 --- a/platforms/android/test-app/runtime/src/main/java/com/tns/GcListener.java +++ b/platforms/android/test-app/runtime/src/main/java/com/tns/GcListener.java @@ -60,13 +60,7 @@ public void run() { for (int i =0; i < objIds.length; i++) { objIds[i] = collectedObjectIds.get(i); } - // unsubscribe() takes this lock too, so once it returns - // no batch is still on its way into that runtime. - synchronized (instance) { - if (subscribers.get(runtime) == subscriber) { - runtime.notifyGc(objIds); - } - } + runtime.notifyGc(objIds); if (runtime.getLogger().isEnabled()) { runtime.getLogger().write("GC", "GC triggered for " + String.valueOf(objIds.length) + " objects"); runtime.getLogger().write("NS.GCListener","Objects referenced after GC: " + subscriber.referencesMap.size()); diff --git a/platforms/android/test-app/runtime/src/main/java/com/tns/Runtime.java b/platforms/android/test-app/runtime/src/main/java/com/tns/Runtime.java index 16dc997ca..f08f4d87d 100644 --- a/platforms/android/test-app/runtime/src/main/java/com/tns/Runtime.java +++ b/platforms/android/test-app/runtime/src/main/java/com/tns/Runtime.java @@ -53,7 +53,7 @@ private native void attachNativeScript(int runtimeId, String metadataPath, long boolean verboseLoggingEnabled, int maxLogcatObjectSize, boolean forceLog, String bootstrapScript); - // Implemented alongside attachNativeScript; see detach(). + // Implemented alongside attachNativeScript. private native void detachNativeScript(int runtimeId); private native void runModule(int runtimeId, String filePath) throws NativeScriptException; @@ -228,10 +228,6 @@ public int compare(Method lhs, Method rhs) { private static AtomicInteger nextRuntimeId = new AtomicInteger(0); private final static ThreadLocal currentRuntime = new ThreadLocal(); private final static Map runtimeCache = new ConcurrentHashMap<>(); - // Objects owned by a detached runtime, so a call into one can say why it - // fails. Outlives every Runtime by design; the weak keys pin nothing. - private final static Map orphanedObjects = - Collections.synchronizedMap(new NativeScriptWeakHashMap()); public static boolean nativeLibraryLoaded; private static final ClassStorageService classStorageService = new ClassStorageServiceImpl(ClassCacheImpl.INSTANCE, ClassLoadersCollectionImpl.INSTANCE); @@ -528,39 +524,12 @@ private void attach(long hostRuntimePtr, String metadataPath, String bootstrapSc } /* - Unbinds from the embedder's runtime and frees everything NativeScript - holds for it. Must run while that runtime is still alive and not running - JS: React Native's module invalidation is such a point. Idempotent. The - instance cannot be attached again; a new host runtime gets a new Runtime. + Unbinds from the embedder's runtime and frees what NativeScript holds in + it. Must run while that runtime is still alive and not running JS. */ public void detach() { - if (!initialized) { - return; - } - initialized = false; - - // Returns only once no GC batch is being delivered to this runtime. - GcListener.unsubscribe(this); - - for (Object instance : strongJavaObjectToID.keySet()) { - orphanedObjects.put(instance, runtimeId); - } - for (Object instance : weakJavaObjectToID.keySet()) { - if (instance != null) { - orphanedObjects.put(instance, runtimeId); - } - } - runtimeCache.remove(runtimeId); - if (currentRuntime.get() == this) { - currentRuntime.remove(); - } - + detachRuntime(runtimeId); detachNativeScript(runtimeId); - - strongInstances.clear(); - strongJavaObjectToID.clear(); - weakInstances.clear(); - weakJavaObjectToID.clear(); } /* @@ -605,11 +574,11 @@ public static void runWorkerLoop() { } /* - Called via JNI from the native worker thread during shutdown, before the - worker's runtime is disposed. + Called before a runtime is disposed: via JNI from the native worker + thread during shutdown, and by detach(). */ @RuntimeCallable - public static void detachWorkerRuntime(int runtimeId) { + public static void detachRuntime(int runtimeId) { Runtime runtime = runtimeCache.remove(runtimeId); if (runtime != null) { GcListener.unsubscribe(runtime); @@ -1268,11 +1237,6 @@ public static Object callJSMethod(int runtimeId, Object javaObject, String metho javaObjectID = (runtime != null) ? runtime.getJavaObjectID(javaObject) : null; } - if (runtime == null && javaObject != null && orphanedObjects.containsKey(javaObject)) { - throw new NativeScriptException("Cannot call " + javaObject.getClass().getName() + "." + methodName - + "(): the JavaScript runtime it belonged to was reloaded or shut down."); - } - if (runtime == null) { runtime = Runtime.getCurrentRuntime(); javaObjectID = (runtime != null) ? runtime.getJavaObjectID(javaObject) : null; @@ -1453,18 +1417,17 @@ public void run() { } } - if (!threadScheduler.post(r)) { - throw new NativeScriptException("Cannot call " + claz.getName() + "." + methodName - + "(): the JavaScript thread it runs on has stopped."); - } + boolean success = threadScheduler.post(r); - synchronized (r) { - try { - if (arr[1] == null) { - r.wait(); + if (success) { + synchronized (r) { + try { + if (arr[1] == null) { + r.wait(); + } + } catch (InterruptedException e) { + ret = e; } - } catch (InterruptedException e) { - ret = e; } } From 8d6e841f25d4636f6a3085a2e6bcafcc0a1a1389 Mon Sep 17 00:00:00 2001 From: Nathan Walker Date: Sun, 25 Oct 2026 13:43:46 -0700 Subject: [PATCH 3/3] refactor(android): name napi's runtime teardown DisposeRuntime too --- NativeScript/runtime/android/napi/Runtime.cpp | 2 +- NativeScript/runtime/android/napi/Runtime.h | 2 +- NativeScript/runtime/android/napi/workers/WorkerWrapper.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/NativeScript/runtime/android/napi/Runtime.cpp b/NativeScript/runtime/android/napi/Runtime.cpp index 49fc0455f..40d3dd124 100644 --- a/NativeScript/runtime/android/napi/Runtime.cpp +++ b/NativeScript/runtime/android/napi/Runtime.cpp @@ -596,7 +596,7 @@ void Runtime::RunWorker(const std::string &filePath) { m_module.LoadWorker(env, filePath); } -void Runtime::DisposeWorkerRuntime(Runtime *runtime) { +void Runtime::DisposeRuntime(Runtime *runtime) { // `env` is referenced by the (engine-specific) JSEnterScope macro below. napi_env env = runtime->env; { diff --git a/NativeScript/runtime/android/napi/Runtime.h b/NativeScript/runtime/android/napi/Runtime.h index e3c78be88..83901f80e 100644 --- a/NativeScript/runtime/android/napi/Runtime.h +++ b/NativeScript/runtime/android/napi/Runtime.h @@ -84,7 +84,7 @@ namespace tns { // Tears down a worker's Runtime (engine scope + env free) and deletes // it. Called from the native worker thread during shutdown. - static void DisposeWorkerRuntime(Runtime *runtime); + static void DisposeRuntime(Runtime *runtime); jobject RunScript(JNIEnv *_env, jobject obj, jstring scriptFile); diff --git a/NativeScript/runtime/android/napi/workers/WorkerWrapper.cpp b/NativeScript/runtime/android/napi/workers/WorkerWrapper.cpp index be943c0be..c16efaa1e 100644 --- a/NativeScript/runtime/android/napi/workers/WorkerWrapper.cpp +++ b/NativeScript/runtime/android/napi/workers/WorkerWrapper.cpp @@ -502,7 +502,7 @@ void WorkerWrapper::BackgroundLooper(std::shared_ptr self) { // Enters the engine scope, tears down the runtime and frees the env, // then deletes the Runtime (which frees the underlying engine runtime). - Runtime::DisposeWorkerRuntime(runtime_); + Runtime::DisposeRuntime(runtime_); runtime_ = nullptr; }