fix: bound JSICache growth by compacting dead weak slots

packages/react-native-nitro-modules/cpp/jsi/JSICache.cpp

@@ -27,12 +27,12 @@ JSICache::~JSICache() {
   Logger::log(LogLevel::Info, TAG, "Destroying JSICache...");
   std::unique_lock lock(_mutex);
 
-  destroyReferences(_valueCache);
-  destroyReferences(_objectCache);
-  destroyReferences(_functionCache);
-  destroyReferences(_weakObjectCache);
-  destroyReferences(_propNameIDCache);
-  destroyReferences(_arrayBufferCache);
+  destroyReferences(_valueCache.references());
+  destroyReferences(_objectCache.references());
+  destroyReferences(_functionCache.references());
+  destroyReferences(_weakObjectCache.references());
+  destroyReferences(_propNameIDCache.references());
+  destroyReferences(_arrayBufferCache.references());
 }
 
 JSICacheReference JSICache::getOrCreateCache(jsi::Runtime& runtime) {

packages/react-native-nitro-modules/cpp/jsi/JSICache.hpp

@@ -10,6 +10,8 @@
 #include "BorrowingReference.hpp"
 #include "NitroLogger.hpp"
 #include "WeakReference.hpp"
+#include <algorithm>
+#include <cstddef>
 #include <jsi/jsi.h>
 #include <memory>
 #include <mutex>
@@ -57,14 +59,57 @@ class JSICache final : public jsi::NativeState {
 private:
   friend class JSICacheReference;
 
+public:
+  /**
+   * A list of weakly-held cache slots that drops dead slots as it grows.
+   *
+   * Slots were previously only ever appended and freed as a whole in `~JSICache`, so a caller that converts
+   * values to JS at a high rate grew these lists without bound for the lifetime of the Runtime, long after the
+   * values themselves had been collected.
+   *
+   * `compact()` only erases slots whose value is definitively deleted (`isDeleted()` - deliberately NOT `lock()`,
+   * which materializes a strong reference), so it can never drop a slot `~JSICache` still needs to destroy.
+   * The doubling `_compactAt` watermark keeps this amortized O(1) per push, so a cache made mostly of long-lived
+   * values does not re-scan on every insert.
+   */
+  template <typename T>
+  class WeakCache final {
+  public:
+    void push(WeakReference<T>&& reference) {
+      if (_references.size() >= _compactAt) [[unlikely]] {
+        compact();
+      }
+      _references.push_back(std::move(reference));
+    }
+
+    [[nodiscard]]
+    const std::vector<WeakReference<T>>& references() const {
+      return _references;
+    }
+
+  private:
+    void compact() {
+      _references.erase(
+          std::remove_if(_references.begin(), _references.end(), [](const WeakReference<T>& reference) { return reference.isDeleted(); }),
+          _references.end());
+      _compactAt = std::max(kMinCompactSize, _references.size() * 2);
+    }
+
+  private:
+    static inline constexpr size_t kMinCompactSize = 64;
+
+    std::vector<WeakReference<T>> _references;
+    size_t _compactAt{kMinCompactSize};
+  };
+
 private:
   std::mutex _mutex;
-  std::vector<WeakReference<jsi::Value>> _valueCache;
-  std::vector<WeakReference<jsi::Object>> _objectCache;
-  std::vector<WeakReference<jsi::Function>> _functionCache;
-  std::vector<WeakReference<jsi::WeakObject>> _weakObjectCache;
-  std::vector<WeakReference<jsi::PropNameID>> _propNameIDCache;
-  std::vector<WeakReference<jsi::ArrayBuffer>> _arrayBufferCache;
+  WeakCache<jsi::Value> _valueCache;
+  WeakCache<jsi::Object> _objectCache;
+  WeakCache<jsi::Function> _functionCache;
+  WeakCache<jsi::WeakObject> _weakObjectCache;
+  WeakCache<jsi::PropNameID> _propNameIDCache;
+  WeakCache<jsi::ArrayBuffer> _arrayBufferCache;
 
 private:
   static inline std::unordered_map<jsi::Runtime*, std::weak_ptr<JSICache>> _globalCache;
@@ -86,32 +131,32 @@ class JSICacheReference final {
 public:
   BorrowingReference<jsi::Value> makeShared(jsi::Value&& value) {
     BorrowingReference<jsi::Value> owning(new jsi::Value(std::move(value)));
-    _strongCache->_valueCache.push_back(owning.weak());
+    _strongCache->_valueCache.push(owning.weak());
     return owning;
   }
   BorrowingReference<jsi::Object> makeShared(jsi::Object&& value) {
     BorrowingReference<jsi::Object> owning(new jsi::Object(std::move(value)));
-    _strongCache->_objectCache.push_back(owning.weak());
+    _strongCache->_objectCache.push(owning.weak());
     return owning;
   }
   BorrowingReference<jsi::Function> makeShared(jsi::Function&& value) {
     BorrowingReference<jsi::Function> owning(new jsi::Function(std::move(value)));
-    _strongCache->_functionCache.push_back(owning.weak());
+    _strongCache->_functionCache.push(owning.weak());
     return owning;
   }
   BorrowingReference<jsi::WeakObject> makeShared(jsi::WeakObject&& value) {
     BorrowingReference<jsi::WeakObject> owning(new jsi::WeakObject(std::move(value)));
-    _strongCache->_weakObjectCache.push_back(owning.weak());
+    _strongCache->_weakObjectCache.push(owning.weak());
     return owning;
   }
   BorrowingReference<jsi::PropNameID> makeShared(jsi::PropNameID&& value) {
     BorrowingReference<jsi::PropNameID> owning(new jsi::PropNameID(std::move(value)));
-    _strongCache->_propNameIDCache.push_back(owning.weak());
+    _strongCache->_propNameIDCache.push(owning.weak());
     return owning;
   }
   BorrowingReference<jsi::ArrayBuffer> makeShared(jsi::ArrayBuffer&& value) {
     BorrowingReference<jsi::ArrayBuffer> owning(new jsi::ArrayBuffer(std::move(value)));
-    _strongCache->_arrayBufferCache.push_back(owning.weak());
+    _strongCache->_arrayBufferCache.push(owning.weak());
     return owning;
   }
 

packages/react-native-nitro-modules/cpp/utils/BorrowingReference.hpp

@@ -56,8 +56,8 @@ class BorrowingReference final {
       bool shouldDestroy = _state->decrementStrongRefCount();
       if (shouldDestroy) {
         forceDestroyValue();
+        releaseImplicitWeakRef();
       }
-      maybeDestroyState();
     }
 
     _value = ref._value;
@@ -71,10 +71,10 @@ class BorrowingReference final {
   }
 
 private:
-  // WeakReference<T> -> BorrowingReference<T> Lock-constructor
-  explicit BorrowingReference(const WeakReference<T>& ref) : _value(ref._value), _state(ref._state) {
-    _state->strongRefCount++;
-  }
+  // WeakReference<T> -> BorrowingReference<T> Lock-constructor.
+  // The caller (`WeakReference<T>::lock()`) has already claimed the strong ref count via
+  // `tryIncrementStrongRefCount()`, so this must NOT increment it again.
+  explicit BorrowingReference(const WeakReference<T>& ref) : _value(ref._value), _state(ref._state) {}
 
 private:
   // BorrowingReference<C> -> BorrowingReference<T> Cast-constructor
@@ -97,8 +97,8 @@ class BorrowingReference final {
     bool shouldDestroy = _state->decrementStrongRefCount();
     if (shouldDestroy) {
       forceDestroyValue();
+      releaseImplicitWeakRef();
     }
-    maybeDestroyState();
   }
 
 public:
@@ -192,12 +192,14 @@ class BorrowingReference final {
   }
 
 private:
-  void maybeDestroyState() {
-    if (_state->strongRefCount == 0 && _state->weakRefCount == 0) {
-      // free the full memory if there are no more references at all
+  // Releases the strong cohort's implicit weak reference (see `ReferenceState`). Called exactly once per state,
+  // by whichever strong reference performed the final strong release - the value is already destroyed at this
+  // point. Frees the state if no `WeakReference` is left holding it either.
+  void releaseImplicitWeakRef() {
+    if (_state->weakRefCount.fetch_sub(1) == 1) {
       delete _state;
-      _state = nullptr;
     }
+    _state = nullptr;
   }
 
   void forceDestroyValue() {

packages/react-native-nitro-modules/cpp/utils/ReferenceState.hpp

@@ -34,7 +34,28 @@ struct ReferenceState {
     return oldRefCount <= 1;
   }
 
-  explicit ReferenceState() : strongRefCount(1), weakRefCount(0), isDeleted(false) {}
+  /**
+   * Increments the strong ref count by one - unless it is already zero, and returns whether it did.
+   *
+   * A zero strong count means the final strong release is already under way (`~BorrowingReference` decrements
+   * the count BEFORE calling `forceDestroyValue()`), so handing out a strong reference in that window would
+   * resurrect a dying value. This is `weak_ptr::lock()`'s increment-if-not-zero.
+   */
+  inline bool tryIncrementStrongRefCount() {
+    size_t count = strongRefCount.load();
+    while (count != 0) {
+      if (strongRefCount.compare_exchange_weak(count, count + 1)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  // `weakRefCount` starts at 1: the strong cohort collectively owns one implicit weak reference, released by
+  // whichever strong reference performs the final strong release (after it destroyed the value). The state is
+  // freed by whoever brings `weakRefCount` to zero, decided by `fetch_sub`'s return value alone - the same
+  // shape as `shared_ptr`'s control block.
+  explicit ReferenceState() : strongRefCount(1), weakRefCount(1), isDeleted(false) {}
 };
 
 } // namespace margelo::nitro

packages/react-native-nitro-modules/cpp/utils/WeakReference+Borrowing.hpp

@@ -26,6 +26,11 @@ BorrowingReference<T> WeakReference<T>::lock() const {
     // return nullptr
     return BorrowingReference<T>();
   }
+  if (!_state->tryIncrementStrongRefCount()) {
+    // the last strong reference is mid-release - the value is about to be destroyed, it just hasn't
+    // flagged `isDeleted` yet.
+    return BorrowingReference<T>();
+  }
 
   return BorrowingReference(*this);
 }

packages/react-native-nitro-modules/cpp/utils/WeakReference.hpp

@@ -49,8 +49,7 @@ class WeakReference final {
       return *this;
 
     if (_state != nullptr) {
-      _state->weakRefCount--;
-      maybeDestroy();
+      releaseWeakRef();
     }
 
     _value = ref._value;
@@ -67,8 +66,7 @@ class WeakReference final {
 
     if (_state != nullptr) {
       // destroy previous pointer
-      _state->weakRefCount--;
-      maybeDestroy();
+      releaseWeakRef();
     }
 
     _value = ref._value;
@@ -83,8 +81,7 @@ class WeakReference final {
 
   ~WeakReference() {
     if (_state != nullptr) {
-      _state->weakRefCount--;
-      maybeDestroy();
+      releaseWeakRef();
     }
   }
 
@@ -94,21 +91,31 @@ class WeakReference final {
   [[nodiscard]]
   BorrowingReference<T> lock() const;
 
+  /**
+   * Returns whether the referenced value has already been deleted.
+   *
+   * Unlike `lock()`, this never materializes a strong reference, so it is safe to call while the value's final
+   * release may be running concurrently on another Thread. During such a race it may still report a dying value
+   * as alive ("maybe alive"), never the reverse.
+   */
+  [[nodiscard]]
+  bool isDeleted() const {
+    return _state == nullptr || _state->isDeleted.load();
+  }
+
 public:
   friend class BorrowingReference<T>;
 
 private:
-  void maybeDestroy() {
-    if (_state->strongRefCount == 0 && _state->weakRefCount == 0) {
-      // free the full memory if there are no more references at all
-      if (!_state->isDeleted) [[unlikely]] {
-        std::string typeName = TypeInfo::getFriendlyTypename<T>(true);
-        throw std::runtime_error("WeakReference<" + typeName + "> encountered a stale `_value` - BorrowingReference<" + typeName +
-                                 "> should've already deleted this!");
-      }
+  // Releases this weak reference's count on the state, freeing the state if it was the last reference overall.
+  // The strong cohort owns one implicit weak reference (see `ReferenceState`), so the count can only reach zero
+  // after the final strong release already destroyed the value, and `fetch_sub`'s return value alone decides
+  // who frees the state.
+  void releaseWeakRef() {
+    if (_state->weakRefCount.fetch_sub(1) == 1) {
       delete _state;
-      _state = nullptr;
     }
+    _state = nullptr;
   }
 
 private: