fix: release ObjectManager's JS handles and JNI weak refs at teardown

test-app/runtime/src/main/cpp/LRUCache.h

@@ -105,6 +105,23 @@ class LRUCache {
             insert(key, ref);
         }
 
+
+        /*
+         * Evicts every entry, running the evict callback for each. Needed at
+         * teardown: the callback owns the resource behind each value (JNI weak
+         * global refs, here), and nothing else releases them -- eviction
+         * otherwise only happens on capacity pressure or invalidation.
+         */
+        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();
+        }
+
     private:
 
         void evictKey(const key_type& key) {

test-app/runtime/src/main/cpp/ObjectManager.cpp

@@ -220,7 +220,7 @@ Local<Object> ObjectManager::GetJsObjectByJavaObject(int javaObjectID) {
     return handleScope.Escape(Local<Object>());
   }
 
-  Persistent<Object>* jsObject = it->second;
+  Persistent<Object>* jsObject = it->second->target;
 
   auto localObject = Local<Object>::New(isolate, *jsObject);
   return handleScope.Escape(localObject);
@@ -296,7 +296,7 @@ void ObjectManager::Link(const Local<Object>& object, uint32_t javaObjectID,
   // link
   object->SetInternalField(jsInfoIdx, jsInfo);
 
-  m_idToObject.emplace(javaObjectID, objectHandle);
+  m_idToObject.emplace(javaObjectID, state);
 }
 
 bool ObjectManager::CloneLink(const Local<Object>& src,
@@ -364,6 +364,9 @@ void ObjectManager::JSObjectFinalizer(Isolate* isolate,
   auto jsInstanceInfo = GetJSInstanceInfoFromRuntimeObject(po->Get(m_isolate));
 
   if (jsInstanceInfo == nullptr) {
+    // The link was already torn down (ReleaseNativeCounterpart); nothing but
+    // the registration is left to drop.
+    m_idToObject.erase(callbackState->javaObjectID);
     po->Reset();
     delete po;
     delete callbackState;
@@ -460,6 +463,50 @@ int ObjectManager::GenerateNewObjectID() {
   return oldValue;
 }
 
+void ObjectManager::ReleaseAllRegistered() {
+  HandleScope handleScope(m_isolate);
+
+  auto jsInfoIdx = static_cast<int>(MetadataNodeKeys::JsInfo);
+
+  // Detached first: nothing below should observe a half-emptied map, and the
+  // finalizers these handles were armed with will never run now anyway.
+  auto survivors = std::move(m_idToObject);
+  m_idToObject.clear();
+
+  for (auto& entry : survivors) {
+    ObjectWeakCallbackState* state = entry.second;
+    Persistent<Object>* po = state->target;
+
+    if (!po->IsEmpty()) {
+      auto local = po->Get(m_isolate);
+      if (!local.IsEmpty()) {
+        // Drop the back-pointer before the JSInstanceInfo goes away.
+        local->SetInternalField(jsInfoIdx, Undefined(m_isolate));
+      }
+      po->Reset();
+    }
+
+    delete po;
+    delete state->jsInfo;
+    delete state;
+  }
+
+  if (m_poJsWrapperFunc != nullptr) {
+    m_poJsWrapperFunc->Reset();
+    delete m_poJsWrapperFunc;
+    m_poJsWrapperFunc = nullptr;
+  }
+}
+
+ObjectManager::~ObjectManager() {
+  // JNI only -- the isolate is already disposed by the time this runs. The LRU
+  // cache holds a JNI weak global ref per entry (up to its capacity) and only
+  // ever evicted them under capacity pressure, so a worker that touched Java
+  // objects abandoned the whole cache when it died. ART's weak-global table is
+  // bounded, so enough worker cycles turned that leak into an abort.
+  m_cache.clear();
+}
+
 void ObjectManager::ReleaseJSInstance(Persistent<Object>* po,
                                       JSInstanceInfo* jsInstanceInfo) {
   int javaObjectID = jsInstanceInfo->JavaObjectID;
@@ -473,9 +520,11 @@ void ObjectManager::ReleaseJSInstance(Persistent<Object>* po,
     throw NativeScriptException(ss.str());
   }
 
-  assert(po == it->second);
+  assert(po == it->second->target);
 
+  ObjectWeakCallbackState* callbackState = it->second;
   m_idToObject.erase(it);
+  delete callbackState;
   m_released.insert(po, javaObjectID);
   po->Reset();
 
@@ -604,6 +653,14 @@ void ObjectManager::ReleaseNativeCounterpart(v8::Local<v8::Object>& object) {
   env.CallVoidMethod(m_javaRuntimeObject, RELEASE_NATIVE_INSTANCE_METHOD_ID,
                      jsInstanceInfo->JavaObjectID);
 
+  // The registration outlives the link: it is dropped by the finalizer once
+  // the wrapper is collected. Until then the entry must not point at the
+  // JSInstanceInfo being freed here.
+  auto it = m_idToObject.find(jsInstanceInfo->JavaObjectID);
+  if (it != m_idToObject.end()) {
+    it->second->jsInfo = nullptr;
+  }
+
   delete jsInstanceInfo;
   auto jsInfoIdx = static_cast<int>(MetadataNodeKeys::JsInfo);
   object->SetInternalField(jsInfoIdx, Undefined(m_isolate));

test-app/runtime/src/main/cpp/ObjectManager.h

@@ -19,6 +19,21 @@ class ObjectManager {
  public:
   ObjectManager(jobject javaRuntimeObject);
 
+  /*
+   * Frees the JS side of every still-linked object. Must run on the runtime's
+   * own thread while the isolate is alive -- it resets v8::Persistents and
+   * clears internal fields. V8 does not run weak callbacks at isolate
+   * disposal, so without this every linked wrapper's handle, JSInstanceInfo
+   * and callback state is abandoned.
+   */
+  void ReleaseAllRegistered();
+
+  /*
+   * JNI-only teardown; runs from ~Runtime, after the isolate is disposed and
+   * while the thread is still attached. Must not touch any v8 handle.
+   */
+  ~ObjectManager();
+
   void Init(v8::Isolate* isolate);
 
   JniLocalRef GetJavaObjectByJsObject(const v8::Local<v8::Object>& object);
@@ -98,11 +113,17 @@ class ObjectManager {
   struct ObjectWeakCallbackState {
     ObjectWeakCallbackState(ObjectManager* _thisPtr, JSInstanceInfo* _jsInfo,
                             v8::Persistent<v8::Object>* _target)
-        : thisPtr(_thisPtr), jsInfo(_jsInfo), target(_target) {}
+        : thisPtr(_thisPtr),
+          jsInfo(_jsInfo),
+          target(_target),
+          javaObjectID(_jsInfo->JavaObjectID) {}
 
     ObjectManager* thisPtr;
     JSInstanceInfo* jsInfo;
     v8::Persistent<v8::Object>* target;
+    // Duplicated from jsInfo: the finalizer has to unregister itself even when
+    // the JsInfo field was already cleared and jsInfo freed.
+    uint32_t javaObjectID;
   };
 
   struct GarbageCollectionInfo {
@@ -201,7 +222,7 @@ class ObjectManager {
 
   std::stack<GarbageCollectionInfo> m_markedForGC;
 
-  std::unordered_map<int, v8::Persistent<v8::Object>*> m_idToObject;
+  std::unordered_map<int, ObjectWeakCallbackState*> m_idToObject;
 
   PersistentObjectIdSet m_released;
 

test-app/runtime/src/main/cpp/Runtime.cpp

@@ -983,6 +983,14 @@ void Runtime::DestroyRuntime() {
   // is alive and its destructors can still touch v8::Global handles.
   IsolateTracked::SweepAll(m_isolate);
 
+  // Same reason: every still-linked Java<->JS wrapper holds a Persistent, a
+  // JSInstanceInfo and its weak-callback state, and those finalizers will
+  // never run now. The JNI half of ObjectManager is released later, in its
+  // destructor, once the isolate is gone.
+  if (m_objectManager != nullptr) {
+    m_objectManager->ReleaseAllRegistered();
+  }
+
   // Everything below still needs the isolate alive -- the caller disposes it
   // only after this returns -- but runs after the hooks above so nothing they
   // touch is pulled out from under them.