feat(runtime): interop.keepAliveWhileRetained for natively retained instances

NativeScript/runtime/Caches.h

@@ -169,6 +169,9 @@ class Caches {
 
   robin_hood::unordered_map<id, std::shared_ptr<v8::Persistent<v8::Value>>>
       Instances;
+  // Classes built by __extends, whose swizzled retain/release drive
+  // GcProtect. Only their instances can be kept alive while retained.
+  robin_hood::unordered_set<Class> RetainTrackedClasses;
   // Root struct objects by (backing buffer, struct type). Child views created
   // for nested-struct fields are never entered here.
   robin_hood::unordered_map<std::pair<void*, const StructInfo*>,

NativeScript/runtime/ClassBuilder.mm

@@ -332,7 +332,7 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) {
                 BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
                 if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) {
                   ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
-                  objcWrapper->GcProtect();
+                  ObjectManager::SetGcProtected(it->second, objcWrapper, true);
                 }
               }
             };
@@ -371,7 +371,7 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) {
                   BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
                   if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) {
                     ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
-                    objcWrapper->GcUnprotect();
+                    ObjectManager::SetGcProtected(it->second, objcWrapper, false);
                   }
                 }
               }
@@ -394,7 +394,7 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) {
                   BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
                   if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) {
                     ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
-                    objcWrapper->GcUnprotect();
+                    ObjectManager::SetGcProtected(it->second, objcWrapper, false);
                   }
                 }
               }
@@ -404,6 +404,7 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) {
           release(self, @selector(release));
         });
         class_addMethod(extendedClass, @selector(release), newRelease, "v@:");
+        cache->RetainTrackedClasses.insert(extendedClass);
 
         info.GetReturnValue().SetUndefined();
       }).ToLocalChecked();

NativeScript/runtime/DataWrapper.h

@@ -400,8 +400,22 @@ class ObjCDataWrapper : public BaseDataWrapper {
   // anything keyed on the raw pointer needs this to detect a recycled slot.
   Class Klass() { return this->klass_; }
 
+  // Set by interop.keepAliveWhileRetained. While such an instance is
+  // GcProtected its registered handle is strong rather than a refused
+  // finalizer, so V8 traces what it references;
+  // ObjectManager::SyncKeepAliveRoot parks the weak callback's parameter here
+  // until the handle is weak again.
+  bool KeepsAliveWhileRetained() { return this->keepAliveWhileRetained_; }
+  void KeepAliveWhileRetained() { this->keepAliveWhileRetained_ = true; }
+  ObjectWeakCallbackState* RootedState() { return this->rootedState_; }
+  void SetRootedState(ObjectWeakCallbackState* state) {
+    this->rootedState_ = state;
+  }
+
  private:
   bool adapterClaim_ = false;
+  bool keepAliveWhileRetained_ = false;
+  ObjectWeakCallbackState* rootedState_ = nullptr;
   id data_;
   const tns::TypeEncoding* typeEncoding_;
   Class klass_;

NativeScript/runtime/Interop.h

@@ -143,6 +143,8 @@ class Interop {
                                         v8::Local<v8::Object> interop);
   static void RegisterHandleOfFunction(v8::Local<v8::Context> context,
                                        v8::Local<v8::Object> interop);
+  static void RegisterKeepAliveWhileRetainedFunction(
+      v8::Local<v8::Context> context, v8::Local<v8::Object> interop);
   static void RegisterAllocFunction(v8::Local<v8::Context> context,
                                     v8::Local<v8::Object> interop);
   static void RegisterFreeFunction(v8::Local<v8::Context> context,

NativeScript/runtime/InteropTypes.mm

@@ -28,6 +28,7 @@
   RegisterBufferFromDataFunction(context, interop);
   RegisterStringFromCString(context, interop);
   RegisterHandleOfFunction(context, interop);
+  RegisterKeepAliveWhileRetainedFunction(context, interop);
   RegisterAllocFunction(context, interop);
   RegisterFreeFunction(context, interop);
   RegisterAdoptFunction(context, interop);
@@ -303,6 +304,69 @@ new PrimitiveDataWrapper(sizeof(void*),
   tns::Assert(success, isolate);
 }
 
+// interop.keepAliveWhileRetained(obj): while native code retains `obj` (an
+// instance of a class built by __extends), its JS object is held strongly
+// instead of only being refused finalization, so everything it references
+// stays alive too. The cost is the one that refusal exists to avoid: a cycle
+// running back from that native retain to `obj` through JS is never collected.
+void Interop::RegisterKeepAliveWhileRetainedFunction(Local<Context> context,
+                                                     Local<Object> interop) {
+  Local<v8::Function> func;
+  bool success =
+      v8::Function::New(context, [](const FunctionCallbackInfo<Value>& info) {
+        Isolate* isolate = info.GetIsolate();
+        auto throwTypeError = [isolate](const char* message) {
+          isolate->ThrowException(Exception::TypeError(tns::ToV8String(isolate, message)));
+        };
+
+        if (info.Length() < 1 || !info[0]->IsObject()) {
+          throwTypeError("interop.keepAliveWhileRetained expects a native object.");
+          return;
+        }
+        Local<Object> arg = info[0].As<Object>();
+        BaseDataWrapper* wrapper = tns::GetValue(isolate, arg);
+        if (wrapper == nullptr || wrapper->Type() != WrapperType::ObjCObject) {
+          throwTypeError("interop.keepAliveWhileRetained expects a native object.");
+          return;
+        }
+        ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
+        id target = objcWrapper->Data();
+        if (target == nil) {
+          throwTypeError("interop.keepAliveWhileRetained expects a native object.");
+          return;
+        }
+
+        auto cache = Caches::Get(isolate);
+        bool tracked = false;
+        for (Class klass = object_getClass(target); klass != nil;
+             klass = class_getSuperclass(klass)) {
+          if (cache->RetainTrackedClasses.contains(klass)) {
+            tracked = true;
+            break;
+          }
+        }
+        if (!tracked) {
+          throwTypeError("interop.keepAliveWhileRetained expects an instance of a class built by "
+                         "__extends: only those observe native retains.");
+          return;
+        }
+
+        objcWrapper->KeepAliveWhileRetained();
+        auto it = cache->Instances.find(target);
+        if (it != cache->Instances.end() && it->second != nullptr && !it->second->IsEmpty() &&
+            it->second->Get(isolate) == arg) {
+          ObjectManager::SyncKeepAliveRoot(it->second, objcWrapper);
+        }
+      }).ToLocal(&func);
+
+  Isolate* isolate = v8::Isolate::GetCurrent();
+  tns::Assert(success, isolate);
+
+  success = interop->Set(context, tns::ToV8String(isolate, "keepAliveWhileRetained"), func)
+                .FromMaybe(false);
+  tns::Assert(success, isolate);
+}
+
 void Interop::RegisterAllocFunction(Local<Context> context, Local<Object> interop) {
   Local<v8::Function> func;
   bool success = v8::Function::New(context, [](const FunctionCallbackInfo<Value>& info) {

NativeScript/runtime/ObjectManager.h

@@ -6,6 +6,7 @@
 namespace tns {
 
 class ObjectManager;
+class ObjCDataWrapper;
 
 // Parameter of the kFinalizer weak callback armed on target_.
 //
@@ -50,6 +51,17 @@ class ObjectManager {
       const v8::WeakCallbackInfo<ObjectWeakCallbackState>& data);
   static bool DisposeValue(v8::Isolate* isolate, v8::Local<v8::Value> value,
                            bool isFinalDisposal = false);
+  // The only way a wrapper's GcProtect flag may change: it also keeps an
+  // interop.keepAliveWhileRetained handle strong exactly while protected.
+  static void SetGcProtected(
+      const std::shared_ptr<v8::Persistent<v8::Value>>& handle,
+      ObjCDataWrapper* wrapper, bool isProtected);
+  // Makes `handle` strong when `wrapper` keeps alive while retained and is
+  // protected, and weak again otherwise. `handle` must be the wrapper's entry
+  // in Caches::Instances.
+  static void SyncKeepAliveRoot(
+      const std::shared_ptr<v8::Persistent<v8::Value>>& handle,
+      ObjCDataWrapper* wrapper);
   // Disposes every handle from Register() that is still alive. Replaces the
   // old Isolate::VisitHandlesWithClassIds walk, which V8 removed.
   static void DisposeAllRegistered(v8::Isolate* isolate);

NativeScript/runtime/ObjectManager.mm

@@ -339,6 +339,49 @@ void DisposeHandle(v8::Isolate* isolate,
   return true;
 }
 
+void ObjectManager::SetGcProtected(const std::shared_ptr<Persistent<Value>>& handle,
+                                   ObjCDataWrapper* wrapper, bool isProtected) {
+  if (isProtected) {
+    wrapper->GcProtect();
+  } else {
+    wrapper->GcUnprotect();
+  }
+  SyncKeepAliveRoot(handle, wrapper);
+}
+
+void ObjectManager::SyncKeepAliveRoot(const std::shared_ptr<Persistent<Value>>& handle,
+                                      ObjCDataWrapper* wrapper) {
+  bool wantsRoot = wrapper->KeepsAliveWhileRetained() && wrapper->IsGcProtected();
+  bool isRooted = wrapper->RootedState() != nullptr;
+  if (wantsRoot == isRooted || handle == nullptr || handle->IsEmpty()) {
+    return;
+  }
+
+  if (wantsRoot) {
+    // A handle that is not weak was never registered with a finalizer, so
+    // there is no weakness to suspend.
+    if (!handle->IsWeak()) {
+      return;
+    }
+    ObjectWeakCallbackState* state = handle->ClearWeak<ObjectWeakCallbackState>();
+    if (state == nullptr) {
+      return;
+    }
+    if (state->disposing_) {
+      // Reached from inside this handle's own finalizer frame, which owns the
+      // state: leave the weakness in place for it.
+      handle->SetWeak(state, FinalizerCallback, WeakCallbackType::kFinalizer);
+      return;
+    }
+    wrapper->SetRootedState(state);
+    return;
+  }
+
+  ObjectWeakCallbackState* state = wrapper->RootedState();
+  wrapper->SetRootedState(nullptr);
+  handle->SetWeak(state, FinalizerCallback, WeakCallbackType::kFinalizer);
+}
+
 void ObjectManager::ReleaseNativeCounterpartCallback(const FunctionCallbackInfo<Value>& info) {
   Isolate* isolate = info.GetIsolate();
 
@@ -375,8 +418,17 @@ void DisposeHandle(v8::Isolate* isolate,
     auto it = cache->Instances.find(data);
     if (it != cache->Instances.end()) {
       std::shared_ptr<Persistent<Value>> handle = it->second;
-      ObjectWeakCallbackState* state =
-          handle->IsWeak() ? handle->ClearWeak<ObjectWeakCallbackState>() : nullptr;
+      // A handle kept alive while retained is strong, its state parked on the
+      // wrapper; retiring it without taking that state back would leave the
+      // handle rooted forever.
+      ObjectWeakCallbackState* state = nullptr;
+      ObjectWeakCallbackState* rooted = objcWrapper->RootedState();
+      if (rooted != nullptr && rooted->target_ == handle) {
+        objcWrapper->SetRootedState(nullptr);
+        state = rooted;
+      } else if (handle->IsWeak()) {
+        state = handle->ClearWeak<ObjectWeakCallbackState>();
+      }
       if (state != nullptr && state->disposing_) {
         // Reached from a -dealloc running inside this handle's own finalizer:
         // that frame already released the native counterpart and owns the

TestRunner/app/tests/KeepAliveWhileRetainedTests.js

@@ -0,0 +1,131 @@
+describe("interop.keepAliveWhileRetained", function () {
+    // Only the TypeScript __extends path installs the retain/release swizzles
+    // that observe native retains.
+    function makeExtendedClass() {
+        return (function (_super) {
+            __extends(KeepAliveParent, _super);
+            function KeepAliveParent() {
+                return (_super !== null && _super.apply(this, arguments)) || this;
+            }
+            return KeepAliveParent;
+        })(NSObject);
+    }
+
+    // Overwrites the stack region that held the creation locals so
+    // conservative stack scanning cannot keep dead wrappers alive.
+    function scrubStack() {
+        return (function scrub(n) {
+            return n > 0 ? scrub(n - 1) + n : 0;
+        })(300);
+    }
+
+    it("keeps what a natively retained instance references alive", function () {
+        var Parent = makeExtendedClass();
+        var N = 16;
+        var holder = NSMutableArray.alloc().init();
+
+        (function create() {
+            for (var i = 0; i < N; i++) {
+                var parent = Parent.alloc().init();
+                // Before and after the native retain: both orders must root.
+                if (i % 2 === 0) {
+                    interop.keepAliveWhileRetained(parent);
+                    holder.addObject(parent);
+                } else {
+                    holder.addObject(parent);
+                    interop.keepAliveWhileRetained(parent);
+                }
+                // Owned only by JS, reachable only through the parent.
+                parent.child = NSMutableArray.alloc().init();
+                parent.child.addObject(i);
+                parent.__mine = true;
+            }
+        })();
+
+        scrubStack();
+        __collect();
+        __collect();
+
+        for (var i = 0; i < N; i++) {
+            var parent = holder.objectAtIndex(i);
+            expect(parent.__mine).toBe(true);
+            expect(parent.child.count).toBe(1);
+            expect(parent.child.objectAtIndex(0)).toBe(i);
+        }
+    });
+
+    it("lets the instance be collected once native lets go", function (done) {
+        var Parent = makeExtendedClass();
+        var holder = NSMutableArray.alloc().init();
+        var ref;
+
+        (function create() {
+            var parent = Parent.alloc().init();
+            interop.keepAliveWhileRetained(parent);
+            holder.addObject(parent);
+            ref = new WeakRef(parent);
+        })();
+
+        scrubStack();
+        __collect();
+        setTimeout(function () {
+            __collect();
+            // A boolean, not the object: Jasmine must not hold a reference.
+            expect(ref.deref() !== undefined).toBe(true);
+
+            holder.removeAllObjects();
+            scrubStack();
+            setTimeout(function () {
+                __collect();
+                setTimeout(function () {
+                    __collect();
+                    expect(ref.deref()).toBeUndefined();
+                    done();
+                }, 0);
+            }, 0);
+        }, 0);
+    });
+
+    it("throws a TypeError for objects whose retains are not observed", function () {
+        expect(function () {
+            interop.keepAliveWhileRetained(NSObject.alloc().init());
+        }).toThrowError(TypeError);
+
+        var PlainExtended = NSObject.extend({}, { name: "KeepAlivePlainExtended" });
+        expect(function () {
+            interop.keepAliveWhileRetained(PlainExtended.alloc().init());
+        }).toThrowError(TypeError);
+
+        expect(function () {
+            interop.keepAliveWhileRetained({});
+        }).toThrowError(TypeError);
+        expect(function () {
+            interop.keepAliveWhileRetained();
+        }).toThrowError(TypeError);
+    });
+
+    // Retiring a registration whose handle is strong has to take its parked
+    // weak-callback state back; otherwise the handle stays a root forever.
+    it("frees the handle when an instance kept alive is retired by __releaseNativeCounterpart", function (done) {
+        var Parent = makeExtendedClass();
+        var holder = NSMutableArray.alloc().init();
+        var ref;
+
+        (function create() {
+            var parent = Parent.alloc().init();
+            interop.keepAliveWhileRetained(parent);
+            holder.addObject(parent);
+            ref = new WeakRef(parent);
+            __releaseNativeCounterpart(parent);
+        })();
+
+        scrubStack();
+        __collect();
+        setTimeout(function () {
+            __collect();
+            expect(ref.deref()).toBeUndefined();
+            holder.removeAllObjects();
+            done();
+        }, 0);
+    });
+});

TestRunner/app/tests/index.js

@@ -132,6 +132,7 @@ require("./NsRuntimeTests");
 require("./NsWorkerThreadsTests");
 require("./NsUrlTests");
 require("./GCFinalizerTests");
+require("./KeepAliveWhileRetainedTests");
 require("./BlockCacheRaceTests");
 require("./WorkerConcurrentStartupTests");
 require("./WorkerOptionsTests");