fix(runtime): charge the native memory of +0 objects once JS holds the last reference

NativeScript/runtime/ArgConverter.mm

@@ -909,6 +909,12 @@
       tns::SetValue(isolate, receiver, wrapper);
       cache->Instances.emplace(target, poValue);
       [target retain];
+      // A wrapper that skips registration is never collected, so charging it
+      // could not pay off.
+      if (!skipGCRegistration) {
+        ExternalMemory::RecordSoleOwnerCandidate(isolate, cache.get(),
+                                                 static_cast<ObjCDataWrapper*>(wrapper), poValue);
+      }
     }
   } else {
     tns::SetValue(isolate, receiver, wrapper);

NativeScript/runtime/Caches.h

@@ -8,6 +8,7 @@
 
 #include "Common.h"
 #include "ConcurrentMap.h"
+#include "ExternalMemory.h"
 #include "Metadata.h"
 #include "robin_hood.h"
 
@@ -169,6 +170,7 @@ class Caches {
       PointerInstances;
   // ExternalMemory's size estimator per class, resolved on first sight.
   robin_hood::unordered_map<Class, uint8_t> ExternalSizeEstimators;
+  ExternalMemory::SoleOwnerCandidates SoleOwnerCandidates;
 
   // Live IsolateTracked instances (URL, URLSearchParams, URLPattern). Their
   // weak-callback finalizers never fire at isolate disposal, so teardown

NativeScript/runtime/DataWrapper.h

@@ -416,8 +416,26 @@ class ObjCDataWrapper : public BaseDataWrapper {
   // anything keyed on the raw pointer needs this to detect a recycled slot.
   Class Klass() { return this->klass_; }
 
+  // Retains on Data() held by interop.bufferFromData buffers. Those buffers
+  // are JS's as much as the wrapper is, so ExternalMemory does not count them
+  // as native owners. Each buffer's deleter decrements the count from
+  // whichever thread frees it, possibly after this wrapper is gone, hence the
+  // shared counter.
+  std::shared_ptr<std::atomic<uint32_t>> BufferRetains() {
+    if (this->bufferRetains_ == nullptr) {
+      this->bufferRetains_ = std::make_shared<std::atomic<uint32_t>>(0);
+    }
+    return this->bufferRetains_;
+  }
+  uint32_t BufferRetainCount() const {
+    return this->bufferRetains_ != nullptr
+               ? this->bufferRetains_->load(std::memory_order_acquire)
+               : 0;
+  }
+
  private:
   bool adapterClaim_ = false;
+  std::shared_ptr<std::atomic<uint32_t>> bufferRetains_;
   id data_;
   const tns::TypeEncoding* typeEncoding_;
   Class klass_;

NativeScript/runtime/ExternalMemory.h

@@ -1,14 +1,22 @@
 #ifndef ExternalMemory_h
 #define ExternalMemory_h
 
+#include <objc/objc.h>
+#include <pthread.h>
+
+#include <atomic>
 #include <cstddef>
+#include <memory>
+#include <vector>
 
 #include "v8-external-memory-accounter.h"
 #include "v8.h"
 
 namespace tns {
 
 class BaseDataWrapper;
+class Caches;
+class ObjCDataWrapper;
 
 // Native bytes a wrapper keeps alive, reported to V8 so the collector weighs
 // them when scheduling collections. The wrapper owns it, so the bytes are
@@ -52,6 +60,70 @@ void SetSize(v8::Isolate* isolate, BaseDataWrapper* wrapper, size_t bytes);
 // nothing, so charging it only buys collections that cannot pay off.
 void ChargeEstimatedSize(v8::Isolate* isolate, v8::Local<v8::Value> value);
 
+// Objects that reached JS at +0 (a block or override argument, an instance
+// method's return) whose class has an estimator. Native code still holds such
+// an object while the call that handed it over runs, and lets go once that
+// call returns or its autorelease pool drains. Each candidate is checked
+// exactly once after that point, from the home thread's run loop, and charged
+// if no retain other than JS's own is left; one native still holds then is
+// never charged, even after native lets go. Worker threads drain no
+// autorelease pool per run loop pass, so on a worker only objects their native
+// caller releases outright can qualify.
+//
+// Guarded by the isolate's Locker, except NextDueMs, which the run loop
+// observer reads before deciding whether to take the Locker at all.
+class SoleOwnerCandidates {
+ public:
+  // The thread whose run loop runs the checks. Runtime::Init, on that thread.
+  void BindToCurrentThread() { homeThread_ = pthread_self(); }
+
+  // When the earliest candidate becomes checkable (NowMs units), or infinity.
+  double NextDueMs() const {
+    return nextDueMs_.load(std::memory_order_acquire);
+  }
+
+ private:
+  struct Candidate {
+    id object;
+    Class klass;
+    uint8_t estimator;
+    // The registry handle of the wrapper's JS object. Collection or an
+    // explicit release empties it, and holding it never keeps the object.
+    std::shared_ptr<v8::Persistent<v8::Value>> handle;
+    double dueMs;
+  };
+
+  friend void RecordSoleOwnerCandidate(
+      v8::Isolate*, Caches*, ObjCDataWrapper*,
+      const std::shared_ptr<v8::Persistent<v8::Value>>&);
+  friend void ChargeSoleOwners(v8::Isolate*, Caches*);
+  friend void ChargeEstimatedSize(v8::Isolate*, v8::Local<v8::Value>);
+
+  // The checks only run when the home run loop is about to sleep. A loop idling
+  // between foreign callbacks would never get there, so a no-op internal post
+  // makes it run one more pass no earlier than `dueMs`.
+  void EnsureWake(v8::Isolate* isolate, double dueMs, double now);
+
+  pthread_t homeThread_ = nullptr;
+  std::vector<Candidate> candidates_;
+  std::atomic<double> nextDueMs_{__builtin_inf()};
+  // Fire time of the last wake posted for candidates recorded off the home
+  // thread. Another is posted for a candidate this one fires before, or too
+  // long after.
+  double wakeAtMs_ = -1;
+};
+
+// Records the object of a wrapper just created for a +0 object as a candidate
+// for a deferred charge (see SoleOwnerCandidates). Does nothing for classes
+// without an estimator or once the bounded list is full. Isolate entered.
+void RecordSoleOwnerCandidate(
+    v8::Isolate* isolate, Caches* cache, ObjCDataWrapper* wrapper,
+    const std::shared_ptr<v8::Persistent<v8::Value>>& handle);
+
+// Checks the candidates that are due and charges those the wrapper alone
+// still retains. Home thread, isolate entered, before teardown starts.
+void ChargeSoleOwners(v8::Isolate* isolate, Caches* cache);
+
 // Forwards the system's memory pressure events to every live isolate.
 // Process-wide and idempotent.
 void StartMemoryPressureMonitoring();

NativeScript/runtime/ExternalMemory.mm

@@ -7,8 +7,11 @@
 #include <objc/message.h>
 #include <objc/runtime.h>
 
+#include <algorithm>
+
 #include "Caches.h"
 #include "DataWrapper.h"
+#include "EventLoop.h"
 #include "Helpers.h"
 #include "IsolateWrapper.h"
 #include "NSDataAdapter.h"
@@ -140,6 +143,17 @@ size_t Estimate(Estimator estimator, id obj) {
   return 0;
 }
 
+Estimator EstimatorFor(Caches* cache, Class klass) {
+  auto& estimators = cache->ExternalSizeEstimators;
+  auto it = estimators.find(klass);
+  if (it != estimators.end()) {
+    return static_cast<Estimator>(it->second);
+  }
+  Estimator estimator = ResolveEstimator(klass);
+  estimators.emplace(klass, estimator);
+  return estimator;
+}
+
 }  // namespace
 
 void SetSize(Isolate* isolate, BaseDataWrapper* wrapper, size_t bytes) {
@@ -170,23 +184,139 @@ void ChargeEstimatedSize(Isolate* isolate, Local<Value> value) {
     return;
   }
 
-  auto& estimators = Caches::Get(isolate)->ExternalSizeEstimators;
-  Class klass = objcWrapper->Klass();
-  auto it = estimators.find(klass);
-  Estimator estimator;
-  if (it != estimators.end()) {
-    estimator = static_cast<Estimator>(it->second);
-  } else {
-    estimator = ResolveEstimator(klass);
-    estimators.emplace(klass, estimator);
-  }
+  Caches* cache = Caches::Get(isolate).get();
+  Estimator estimator = EstimatorFor(cache, objcWrapper->Klass());
   if (estimator == kNone) {
     return;
   }
 
+  // Wrapping the object just recorded it as a +0 candidate; drop that entry
+  // so owned objects do not use up the bounded list.
+  auto& candidates = cache->SoleOwnerCandidates.candidates_;
+  if (!candidates.empty() && candidates.back().object == obj) {
+    candidates.pop_back();
+  }
+
   SetSize(isolate, wrapper, Estimate(estimator, obj));
 }
 
+namespace {
+
+// Bounds the list between two checks. A burst beyond it goes uncharged.
+constexpr size_t kMaxSoleOwnerCandidates = 4096;
+
+// How long a candidate recorded off the home thread waits before its check.
+// Its native caller runs on that other thread and lets go on its own
+// schedule, after the callback returns or once that thread's pool drains, so
+// there is no point the home thread could observe; a frame is long enough for
+// both in practice.
+constexpr double kForeignSettleMs = 16;
+
+// How late a wake may fire for a candidate, so that callbacks arriving within
+// this window of each other share one wake.
+constexpr double kWakeCoalesceMs = 4;
+}  // namespace
+
+void RecordSoleOwnerCandidate(Isolate* isolate, Caches* cache, ObjCDataWrapper* wrapper,
+                              const std::shared_ptr<Persistent<Value>>& handle) {
+  Estimator estimator = EstimatorFor(cache, wrapper->Klass());
+  if (estimator == kNone) {
+    return;
+  }
+  SoleOwnerCandidates& candidates = cache->SoleOwnerCandidates;
+  if (candidates.candidates_.size() >= kMaxSoleOwnerCandidates) {
+    return;
+  }
+
+  bool onHomeThread = pthread_equal(pthread_self(), candidates.homeThread_);
+  double now = onHomeThread ? 0 : EventLoop::NowMs();
+  double dueMs = onHomeThread ? 0 : now + kForeignSettleMs;
+  candidates.candidates_.push_back({wrapper->Data(), wrapper->Klass(), estimator, handle, dueMs});
+  if (dueMs < candidates.NextDueMs()) {
+    candidates.nextDueMs_.store(dueMs, std::memory_order_release);
+  }
+  if (!onHomeThread) {
+    candidates.EnsureWake(isolate, dueMs, now);
+  }
+}
+
+void ChargeSoleOwners(Isolate* isolate, Caches* cache) {
+  SoleOwnerCandidates& candidates = cache->SoleOwnerCandidates;
+  if (candidates.candidates_.empty()) {
+    return;
+  }
+
+  double now = EventLoop::NowMs();
+  if (candidates.NextDueMs() > now) {
+    return;
+  }
+  // Charging below can collect, and finalizers can run code that records new
+  // candidates, so the due ones leave the list before any is charged.
+  auto firstWaiting =
+      std::partition(candidates.candidates_.begin(), candidates.candidates_.end(),
+                     [now](const SoleOwnerCandidates::Candidate& c) { return c.dueMs <= now; });
+  std::vector<SoleOwnerCandidates::Candidate> due(
+      std::make_move_iterator(candidates.candidates_.begin()),
+      std::make_move_iterator(firstWaiting));
+  candidates.candidates_.erase(candidates.candidates_.begin(), firstWaiting);
+  double nextDueMs = __builtin_inf();
+  for (const auto& c : candidates.candidates_) {
+    nextDueMs = std::min(nextDueMs, c.dueMs);
+  }
+  candidates.nextDueMs_.store(nextDueMs, std::memory_order_release);
+  if (nextDueMs != __builtin_inf()) {
+    candidates.EnsureWake(isolate, nextDueMs, now);
+  }
+
+  for (SoleOwnerCandidates::Candidate& c : due) {
+    if (isolate->IsExecutionTerminating()) {
+      return;
+    }
+    if (c.handle->IsEmpty()) {
+      continue;
+    }
+    // Keeps the object alive while charging: an increase can collect.
+    HandleScope handleScope(isolate);
+    Local<Value> value = c.handle->Get(isolate);
+    BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
+    if (wrapper == nullptr || wrapper->Type() != WrapperType::ObjCObject ||
+        wrapper->ExternalCharge() != nullptr) {
+      continue;
+    }
+    ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
+    // A wrapper still attached to a live object owns its retain, so the object
+    // can be read from here on.
+    if (objcWrapper->Data() != c.object || objcWrapper->Klass() != c.klass) {
+      continue;
+    }
+    // JS's retains are the wrapper's own plus one per interop.bufferFromData
+    // buffer. Such a buffer's bytes are then weighed twice, by V8 and by this
+    // charge, until it dies; V8 counting the buffer alone never brings the
+    // collection that frees the wrapper. The buffer count is read before the
+    // retain count, and a deleter racing on another thread drops its count
+    // before its retain, so the race can only make the check miss. Immortal
+    // objects (constants, tagged pointers) report a count far above these.
+    uint32_t jsRetains = 1 + objcWrapper->BufferRetainCount();
+    if (static_cast<uint64_t>(CFGetRetainCount((CFTypeRef)c.object)) != jsRetains) {
+      continue;
+    }
+    SetSize(isolate, wrapper, Estimate(static_cast<Estimator>(c.estimator), c.object));
+  }
+}
+
+void SoleOwnerCandidates::EnsureWake(Isolate* isolate, double dueMs, double now) {
+  if (wakeAtMs_ >= dueMs && wakeAtMs_ <= dueMs + kWakeCoalesceMs) {
+    return;
+  }
+  Runtime* runtime = Runtime::GetRuntime(isolate);
+  std::shared_ptr<EventLoop> loop = runtime != nullptr ? runtime->GetEventLoop() : nullptr;
+  if (loop == nullptr) {
+    return;
+  }
+  wakeAtMs_ = dueMs + kWakeCoalesceMs;
+  loop->PostInternalDelayed([] {}, std::max(0.0, wakeAtMs_ - now));
+}
+
 void StartMemoryPressureMonitoring() {
   static dispatch_source_t source;
   static dispatch_once_t once;

NativeScript/runtime/InteropTypes.mm

@@ -201,16 +201,27 @@ new PrimitiveDataWrapper(sizeof(void*),
         // Take a +1 retain so the NSData outlives autorelease pool drains while
         // the ArrayBuffer is alive. The deleter below releases this retain when
         // V8 finalises the BackingStore (i.e. the ArrayBuffer is GC'd / detached).
+        // The wrapper counts it too, so the retain is not mistaken for a native
+        // owner (ExternalMemory::ChargeSoleOwners); the deleter drops the count
+        // before the retain, which can only make that check miss, never pass
+        // wrongly.
+        struct BufferRetain {
+          id obj;
+          std::shared_ptr<std::atomic<uint32_t>> count;
+        };
+        auto retain = new BufferRetain{obj, wrapper->BufferRetains()};
+        retain->count->fetch_add(1, std::memory_order_acq_rel);
         [obj retain];
 
         std::unique_ptr<v8::BackingStore> backingStore = ArrayBuffer::NewBackingStore(
             data, length,
             [](void* /*data*/, size_t /*length*/, void* deleter_data) {
-              if (deleter_data != nullptr) {
-                [(id)deleter_data release];
-              }
+              auto retain = static_cast<BufferRetain*>(deleter_data);
+              retain->count->fetch_sub(1, std::memory_order_acq_rel);
+              [retain->obj release];
+              delete retain;
             },
-            obj);
+            retain);
 
         Local<ArrayBuffer> result = ArrayBuffer::New(isolate, std::move(backingStore));
         info.GetReturnValue().Set(result);

NativeScript/runtime/Runtime.h

@@ -174,6 +174,8 @@ class Runtime {
 
   static void DrainRejectionsObserver(CFRunLoopObserverRef observer,
                                       CFRunLoopActivity activity, void* info);
+  static void ChargeSoleOwnersObserver(CFRunLoopObserverRef observer,
+                                       CFRunLoopActivity activity, void* info);
   v8::Isolate* isolate_ = nullptr;
   std::atomic<bool> terminationRequested_{false};
   napi_env napiEnv_ = nullptr;
@@ -184,6 +186,9 @@ class Runtime {
   // Drains unhandled promise rejections once per runloop turn
   // (kCFRunLoopBeforeWaiting). Torn down before isolate disposal in ~Runtime.
   CFRunLoopObserverRef rejectionObserver_ = nullptr;
+  // Charges +0 objects JS ended up owning (ExternalMemory::ChargeSoleOwners).
+  // Torn down with rejectionObserver_.
+  CFRunLoopObserverRef soleOwnerObserver_ = nullptr;
   double timeOriginMonotonic_;
   double timeOriginRealtimeMs_;
   // Keeps Caches::Workers alive for as long as this runtime exists: ~Runtime