fix(runtime): release native objects on the runtime's thread

NativeScript/runtime/ArgConverter.mm

@@ -638,9 +638,20 @@
     tns::SetValue(isolate, thiz, wrapper);
     std::shared_ptr<Persistent<Value>> poThiz = ObjectManager::Register(context, thiz);
     cache->Instances.emplace(result, poThiz);
-    if (!resultIsOwned) {
+    // An initializer can answer with an existing object whose release is still
+    // waiting (see CreateJsWrapper).
+    bool tookOver = ObjectManager::TakeDeferredRelease(isolate, result);
+    if (tookOver && resultIsOwned) {
+      [result release];
+    } else if (!tookOver && !resultIsOwned) {
       [result retain];
     }
+    if (tookOver) {
+      // Only releases of objects that first reached JS on the runtime's
+      // thread are ever queued.
+      wrapper->SetBornOnRuntimeThread();
+      ObjectManager::ProtectIfNativelyRetained(result, wrapper);
+    }
     ExternalMemory::ChargeEstimatedSize(isolate, thiz);
   }
 }
@@ -908,7 +919,15 @@
       receiver = poValue->Get(isolate).As<Object>();
       tns::SetValue(isolate, receiver, wrapper);
       cache->Instances.emplace(target, poValue);
-      [target retain];
+      // An extended class reads its retain count to decide whether native
+      // code owns the object, so a pending release is taken over, not stacked.
+      if (!ObjectManager::TakeDeferredRelease(isolate, target)) {
+        [target retain];
+      } else if (wrapper->Type() == WrapperType::ObjCObject) {
+        ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
+        objcWrapper->SetBornOnRuntimeThread();
+        ObjectManager::ProtectIfNativelyRetained(target, objcWrapper);
+      }
     }
   } else {
     tns::SetValue(isolate, receiver, wrapper);

NativeScript/runtime/Caches.h

@@ -110,6 +110,22 @@ class Caches {
     return cache->GetOrCreateState<T>();
   }
 
+  // Teardown only: the existing state, even after InvalidateIsolate, for a
+  // subsystem that must finish work its state still holds. Never creates.
+  template <typename T>
+  static T* ExistingStateFor(v8::Isolate* isolate) {
+    auto raw = isolate->GetData(0);
+    if (raw == nullptr) {
+      return nullptr;
+    }
+    Caches* cache = reinterpret_cast<std::shared_ptr<Caches>*>(raw)->get();
+    size_t index = StateSlotIndexFor<T>();
+    if (index >= cache->stateSlots_.size()) {
+      return nullptr;
+    }
+    return static_cast<T*>(cache->stateSlots_[index].get());
+  }
+
   inline int getIsolateId() { return isolateId_; }
 
   // The id assigned at Init; unlike getIsolateId() it survives

NativeScript/runtime/DataWrapper.h

@@ -20,6 +20,11 @@ class WorkerInspectorClient;
 
 namespace tns {
 
+// Whether the calling thread is the one `isolate`'s runtime runs on (its event
+// loop's thread). JS can also run on other threads, any that take the
+// isolate's Locker. Several runtimes may share a thread. Defined in Runtime.mm.
+bool OnRuntimeThreadOf(v8::Isolate* isolate);
+
 class PrimitiveDataWrapper;
 struct ObjectWeakCallbackState;
 class EventLoop;
@@ -396,7 +401,8 @@ class ObjCDataWrapper : public BaseDataWrapper {
   ObjCDataWrapper(id data, const TypeEncoding* typeEncoding = nullptr)
       : data_(data),
         typeEncoding_(typeEncoding),
-        klass_(object_getClass(data)) {}
+        klass_(object_getClass(data)),
+        bornOnRuntimeThread_(OnRuntimeThreadOf(v8::Isolate::TryGetCurrent())) {}
 
   const WrapperType Type() { return WrapperType::ObjCObject; }
 
@@ -409,6 +415,11 @@ class ObjCDataWrapper : public BaseDataWrapper {
   bool IsAdapterClaim() { return this->adapterClaim_; }
   void MarkAdapterClaim() { this->adapterClaim_ = true; }
 
+  // Whether the object first reached JS on its runtime's thread, the stand-in
+  // for "belongs to that thread": its native release must happen there too.
+  bool BornOnRuntimeThread() { return this->bornOnRuntimeThread_; }
+  void SetBornOnRuntimeThread() { this->bornOnRuntimeThread_ = true; }
+
   const TypeEncoding* TypeEncoding() { return this->typeEncoding_; }
 
   // The class Data() had when this wrapper was built. Data() alone cannot tell
@@ -421,6 +432,7 @@ class ObjCDataWrapper : public BaseDataWrapper {
   id data_;
   const tns::TypeEncoding* typeEncoding_;
   Class klass_;
+  bool bornOnRuntimeThread_;
 };
 
 class ObjCClassWrapper : public BaseDataWrapper {
@@ -488,7 +500,10 @@ class AnonymousFunctionWrapper : public BaseDataWrapper {
 class BlockWrapper : public BaseDataWrapper {
  public:
   BlockWrapper(void* block, const TypeEncoding* typeEncoding, bool ownsBlock)
-      : block_(block), typeEncoding_(typeEncoding), ownsBlock_(ownsBlock) {}
+      : block_(block),
+        typeEncoding_(typeEncoding),
+        ownsBlock_(ownsBlock),
+        bornOnRuntimeThread_(OnRuntimeThreadOf(v8::Isolate::TryGetCurrent())) {}
 
   const WrapperType Type() { return WrapperType::Block; }
 
@@ -498,10 +513,14 @@ class BlockWrapper : public BaseDataWrapper {
 
   bool OwnsBlock() { return this->ownsBlock_; }
 
+  // See ObjCDataWrapper::BornOnRuntimeThread.
+  bool BornOnRuntimeThread() { return this->bornOnRuntimeThread_; }
+
  private:
   void* block_;
   const TypeEncoding* typeEncoding_;
   bool ownsBlock_;
+  bool bornOnRuntimeThread_;
 };
 
 class FunctionReferenceTypeWrapper : public BaseDataWrapper {

NativeScript/runtime/ObjectManager.h

@@ -6,6 +6,7 @@
 namespace tns {
 
 class ObjectManager;
+class ObjCDataWrapper;
 
 // Parameter of the kFinalizer weak callback armed on target_.
 //
@@ -54,6 +55,17 @@ class ObjectManager {
   // old Isolate::VisitHandlesWithClassIds walk, which V8 removed.
   static void DisposeAllRegistered(v8::Isolate* isolate);
 
+  // A collection can run on any thread that holds the isolate's Locker, but
+  // the native releases it causes are handed to the runtime's thread. A new
+  // Instances entry for an object whose release is still waiting takes that
+  // release over as its own retain instead of retaining again; returns whether
+  // it did. Once the entry's retain is settled, call ProtectIfNativelyRetained.
+  static bool TakeDeferredRelease(v8::Isolate* isolate, id obj);
+  // An extended class protects its wrapper when native code first retains the
+  // object past the runtime's own retain. A native retain made while a release
+  // waited found no wrapper to protect; this catches up.
+  static void ProtectIfNativelyRetained(id obj, ObjCDataWrapper* wrapper);
+
  private:
   static void ReleaseNativeCounterpartCallback(
       const v8::FunctionCallbackInfo<v8::Value>& info);

NativeScript/runtime/ObjectManager.mm

@@ -2,11 +2,16 @@
 #include <Block.h>
 #include <CoreFoundation/CoreFoundation.h>
 #include <sstream>
+#include <unordered_map>
+#include <vector>
 #include "Caches.h"
 #include "Constants.h"
 #include "DataWrapper.h"
 #include "FFICall.h"
+#include "EventLoop.h"
 #include "Helpers.h"
+#include "Runtime.h"
+#include "TNSDerivedClass.h"
 
 using namespace v8;
 using namespace std;
@@ -15,11 +20,6 @@
 
 static Class NSTimerClass = objc_getClass("NSTimer");
 
-void ObjectManager::Init(Isolate* isolate, Local<ObjectTemplate> globalTemplate) {
-  globalTemplate->Set(tns::ToV8String(isolate, "__releaseNativeCounterpart"),
-                      FunctionTemplate::New(isolate, ReleaseNativeCounterpartCallback));
-}
-
 namespace {
 
 void LinkRegistered(v8::Isolate* isolate, ObjectWeakCallbackState* state) {
@@ -52,8 +52,96 @@ void UnlinkRegistered(ObjectWeakCallbackState* state) {
   state->next_ = nullptr;
 }
 
+// The native release of an object that first reached JS on its runtime's
+// thread belongs on that thread. JS, and with it a collection and its
+// finalizers, can run on any thread that holds the isolate's Locker (a JS
+// callback dispatched to a background queue), and a -dealloc that tears down a
+// view, a CA transaction or a block capturing one crashes off the main thread.
+// Objects first wrapped on another thread are still released where the
+// collection runs.
+//
+// Every access holds the isolate's Locker: finalizers and
+// __releaseNativeCounterpart on any thread, and on the runtime's thread the
+// drain task, wrapper creation and teardown. So no other synchronization.
+struct DeferredReleases {
+  // Object -> releases it is owed. An object stays alive while it is owed one,
+  // so its address cannot be reused by another object meanwhile.
+  std::unordered_map<void*, unsigned> objects;
+  std::vector<const void*> blocks;
+  bool drainPosted = false;
+};
+
+void DrainDeferredReleases(DeferredReleases* pending) {
+  std::unordered_map<void*, unsigned> objects;
+  std::vector<const void*> blocks;
+  objects.swap(pending->objects);
+  blocks.swap(pending->blocks);
+  pending->drainPosted = false;
+  for (const auto& entry : objects) {
+    id obj = (__bridge id)entry.first;
+    for (unsigned i = 0; i < entry.second; i++) {
+      [obj release];
+    }
+  }
+  for (const void* block : blocks) {
+    Block_release(block);
+  }
+}
+
+// The queue to hand a release to, or null when the caller releases inline.
+// Created by Init, so a collection on another thread never creates it; read
+// without the validity check because a collection can still run on another
+// thread between InvalidateIsolate and teardown, which drains it.
+DeferredReleases* DeferralQueue(Isolate* isolate, bool bornOnRuntimeThread) {
+  if (!bornOnRuntimeThread || OnRuntimeThreadOf(isolate)) {
+    return nullptr;
+  }
+  return Caches::ExistingStateFor<DeferredReleases>(isolate);
+}
+
+// One event loop task per batch. If the loop no longer takes work, teardown is
+// under way and drains the queue itself.
+void PostDrain(Isolate* isolate, DeferredReleases* pending) {
+  if (pending->drainPosted) {
+    return;
+  }
+  Runtime* runtime = Runtime::GetRuntime(isolate);
+  std::shared_ptr<EventLoop> loop = runtime != nullptr ? runtime->GetEventLoop() : nullptr;
+  pending->drainPosted = loop != nullptr && loop->PostInternal([isolate]() {
+    if (DeferredReleases* queued = Caches::ExistingStateFor<DeferredReleases>(isolate)) {
+      DrainDeferredReleases(queued);
+    }
+  });
+}
+
+void ReleaseOnRuntimeThread(Isolate* isolate, id obj, bool bornOnRuntimeThread) {
+  DeferredReleases* pending = DeferralQueue(isolate, bornOnRuntimeThread);
+  if (pending == nullptr) {
+    [obj release];
+    return;
+  }
+  pending->objects[(__bridge void*)obj]++;
+  PostDrain(isolate, pending);
+}
+
+void BlockReleaseOnRuntimeThread(Isolate* isolate, const void* block, bool bornOnRuntimeThread) {
+  DeferredReleases* pending = DeferralQueue(isolate, bornOnRuntimeThread);
+  if (pending == nullptr) {
+    Block_release(block);
+    return;
+  }
+  pending->blocks.push_back(block);
+  PostDrain(isolate, pending);
+}
+
 }  // namespace
 
+void ObjectManager::Init(Isolate* isolate, Local<ObjectTemplate> globalTemplate) {
+  globalTemplate->Set(tns::ToV8String(isolate, "__releaseNativeCounterpart"),
+                      FunctionTemplate::New(isolate, ReleaseNativeCounterpartCallback));
+  Caches::StateFor<DeferredReleases>(isolate);
+}
+
 std::shared_ptr<Persistent<Value>> ObjectManager::Register(Local<Context> context,
                                                            const Local<Value> obj) {
   Isolate* isolate = v8::Isolate::GetCurrent();
@@ -104,6 +192,12 @@ void DisposeHandle(v8::Isolate* isolate,
   Isolate::Scope isolateScope(isolate);
   HandleScope scope(isolate);
 
+  // The event loop has stopped taking work, so nothing else would drain.
+  DeferredReleases* deferred = Caches::ExistingStateFor<DeferredReleases>(isolate);
+  if (deferred != nullptr) {
+    DrainDeferredReleases(deferred);
+  }
+
   // Detach the whole list first so disposal can't walk into freed entries, and
   // claim every state up front: disposing one entry can re-enter JS from
   // -dealloc and reach __releaseNativeCounterpart for any other entry this
@@ -130,6 +224,12 @@ void DisposeHandle(v8::Isolate* isolate,
     state = next;
   }
 
+  // The walk's own releases are inline on the runtime's thread; this catches
+  // any made from another.
+  if (deferred != nullptr) {
+    DrainDeferredReleases(deferred);
+  }
+
   DisposeHandleMap(isolate, cache->CtorFuncs);
   DisposeHandleMap(isolate, cache->ProtocolCtorFuncs);
   DisposeHandleMap(isolate, cache->CFunctions);
@@ -175,6 +275,28 @@ void DisposeHandle(v8::Isolate* isolate,
   state->target_->SetWeak(state, FinalizerCallback, WeakCallbackType::kFinalizer);
 }
 
+bool ObjectManager::TakeDeferredRelease(Isolate* isolate, id obj) {
+  DeferredReleases* pending = Caches::ExistingStateFor<DeferredReleases>(isolate);
+  if (pending == nullptr || pending->objects.empty()) {
+    return false;
+  }
+  auto it = pending->objects.find((__bridge void*)obj);
+  if (it == pending->objects.end()) {
+    return false;
+  }
+  if (--it->second == 0) {
+    pending->objects.erase(it);
+  }
+  return true;
+}
+
+void ObjectManager::ProtectIfNativelyRetained(id obj, ObjCDataWrapper* wrapper) {
+  if (class_conformsToProtocol(object_getClass(obj), @protocol(TNSDerivedClass)) &&
+      [obj retainCount] > 1) {
+    wrapper->GcProtect();
+  }
+}
+
 bool ObjectManager::DisposeValue(Isolate* isolate, Local<Value> value, bool isFinalDisposal) {
   if (value.IsEmpty() || value->IsNullOrUndefined() || !value->IsObject()) {
     return true;
@@ -219,7 +341,12 @@ void DisposeHandle(v8::Isolate* isolate,
             cache->Instances.erase(it);
           }
         }
-        [target release];
+        if (objCObjectWrapper->IsAdapterClaim()) {
+          // The adapter's -dealloc retires this wrapper; it must run first.
+          [target release];
+        } else {
+          ReleaseOnRuntimeThread(isolate, target, objCObjectWrapper->BornOnRuntimeThread());
+        }
       }
       break;
     }
@@ -237,7 +364,8 @@ void DisposeHandle(v8::Isolate* isolate,
       // Block_copy and runs the block's dispose helper once we drop the last
       // reference. (Using CFRelease here over-released stack blocks that were
       // never promoted to the heap, crashing in objc_release during GC.)
-      Block_release(blockWrapper->Block());
+      BlockReleaseOnRuntimeThread(isolate, blockWrapper->Block(),
+                                  blockWrapper->BornOnRuntimeThread());
       break;
     }
     case WrapperType::Reference: {
@@ -386,7 +514,11 @@ void DisposeHandle(v8::Isolate* isolate,
     // wrapper, so it must not be touched afterwards.
     bool adapterClaim = objcWrapper->IsAdapterClaim();
 
-    [data release];
+    if (adapterClaim) {
+      [data release];
+    } else {
+      ReleaseOnRuntimeThread(isolate, data, objcWrapper->BornOnRuntimeThread());
+    }
 
     // The release above can run a -dealloc that detaches this wrapper; the
     // internal field owns it, so free only what is still attached. An

NativeScript/runtime/Runtime.h

@@ -1,6 +1,7 @@
 #ifndef Runtime_h
 #define Runtime_h
 
+#include <pthread.h>
 #include <atomic>
 #include <optional>
 
@@ -51,6 +52,8 @@ class Runtime {
 
   inline CFRunLoopRef RuntimeLoop() { return runtimeLoop_; }
 
+  inline bool IsOnRuntimeThread() const { return pthread_equal(runtimeThread_, pthread_self()) != 0; }
+
   /*
    * Scheduler bound to this runtime's runloop. Producers on any thread post;
    * entries run only on the runtime's home thread.
@@ -192,6 +195,7 @@ class Runtime {
   std::unique_ptr<ModuleInternal> moduleInternal_;
   int workerId_;
   CFRunLoopRef runtimeLoop_;
+  pthread_t runtimeThread_;
   std::shared_ptr<EventLoop> eventLoop_;
   // Drains unhandled promise rejections once per runloop turn
   // (kCFRunLoopBeforeWaiting). Torn down before isolate disposal in ~Runtime.