fix: bound JSICache slot growth, and give ReferenceState a real control block

[jslok]

Found while chasing steady native-heap growth on a hot path that converts HybridObjects to JS every frame. The visible symptom is one leak, but fixing it properly surfaced two lifetime bugs underneath it.

1. JSICache's weak-slot lists grow without bound

Every JSICacheReference::makeShared(..) does _xCache.push_back(owning.weak()), and those six std::vector<WeakReference<T>>s are only ever freed wholesale in ~JSICache. Nothing removes a slot when its value dies.

That is invisible for the usual case — a handful of long-lived callbacks per Runtime. It is not invisible for a caller that converts values to JS at a high rate: the slots accumulate for the entire lifetime of the Runtime, long after the values themselves have been collected. On a per-frame path (a Frame Processor boxing one HybridObject plus two results per frame) that was ~2700 permanently-retained slots per minute, visible in heapprofd as steady growth under JSICacheReference::makeShared.

The lists are now a small WeakCache<T> that compacts as it grows:

• compact() erases only slots whose value is definitively deleted, so it can never drop a slot ~JSICache still needs to force-destroy — it only removes bookkeeping for values that no longer exist.
• a _compactAt watermark (doubling, min 64) keeps this amortized O(1) per push. A cache made mostly of long-lived values, where compaction reclaims nothing, does not re-scan on every insert.

~JSICache walks .references() instead, which is otherwise unchanged.

2. WeakReference::isDeleted(), and why it is not lock()

The compaction probe must not materialize a strong reference. lock() checks isDeleted and then increments the strong count, but ~BorrowingReference decrements the count before calling forceDestroyValue(), and holds no mutex while doing so. A lock() landing in that window resurrects a value whose final release is already in flight, and the resurrected temporary's destructor then runs a second forceDestroyValue() concurrently with the releaser's.

Nitro allows HybridObject/callback releases on any thread, and compaction runs on the hot makeShared path, so that race is reachable. isDeleted() reads the atomic flag only. It is one-sided-safe: during such a race it may still report a dying value as alive, which merely keeps the slot until the next compaction — never the reverse.

3. ReferenceState was freed by a non-atomic two-counter check

Both BorrowingReference::maybeDestroyState() and WeakReference::maybeDestroy() freed the state when strongRefCount == 0 && weakRefCount == 0. Those are two independent atomics read as a unit, so a last-strong releaser and a last-weak releaser running concurrently can both observe (0, 0) — a double delete _state — or one can read a counter out of a state the other has already freed.

ReferenceState now uses shared_ptr's control-block scheme: weakRefCount starts at 1, representing one implicit weak reference collectively owned by the strong cohort, 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 — so exactly one thread ever sees the 1 -> 0 transition, and weakRefCount cannot reach zero before the final strong release has completed.

4. lock() now uses increment-if-not-zero

This one falls out of #3. With the implicit weak reference, the resurrection window described in #2 becomes actively harmful rather than merely odd: a resurrected strong reference performs a second final strong release, which releases the implicit weak reference twice and can free the state while a WeakReference still holds it.

So lock() now claims the strong count with a compare-exchange loop that refuses to go from zero, exactly like weak_ptr::lock(). A zero strong count means the final release is already under way, so returning null there is also just correct on its own — it is the pre-existing resurrection bug, fixed. The private WeakReference -> BorrowingReference lock-constructor no longer increments, since lock() has already claimed the count.

Scope note

I'd have preferred to send #1 alone, but the compaction probe is only safe given #3 and #4, so they belong in one change. Happy to split if you'd rather review them separately — #3 + #4 stand on their own as a correctness fix.

This is independent of #1451 (thread-safe static per-Runtime caches); the two touch JSICache.cpp in different places and do not conflict.

Testing

The JSICache growth fix and the ReferenceState control block have been running on-device (Android, release build) against a per-frame HybridObject conversion path, with all Nitro-consuming modules rebuilt: the retained-slot count stops ratcheting and the native-heap growth attributed to makeShared goes away.

The lock() hardening in #4 is newer and has been compile-checked (NDK clang, C++20) and reasoned through, but not yet soaked on-device — please review that one closely. I have not run the repo's own test suite. Formatted with config/.clang-format.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
nitro-docs	Skipped		Aug 1, 2026 2:11am

[mrousavy]

Hey - I'd definitely prefer separate atomic PRs for this. The change is quite big and a fundamental piece of Nitro architecture.

[jslok]

Makes sense - split into three atomic PRs, closing this one:

1. fix: prevent WeakReference::lock() from resurrecting a value mid final-release #1467 - WeakReference::lock() increment-if-not-zero (standalone, merge first)
2. fix: free ReferenceState via a shared_ptr-style control block #1468 - ReferenceState control block (stacked on fix: prevent WeakReference::lock() from resurrecting a value mid final-release #1467)
3. fix: bound JSICache growth by compacting dead weak slots #1469 - JSICache compaction, the leak fix itself (stacked on fix: free ReferenceState via a shared_ptr-style control block #1468)

Each is a single commit on top of the previous; the combined tip is code-identical to this PR. Dependency reasoning and per-PR testing status are in each description.