fix: prevent WeakReference::lock() from resurrecting a value mid final-release - #1467
fix: prevent WeakReference::lock() from resurrecting a value mid final-release#1467jslok wants to merge 1 commit into
WeakReference::lock() from resurrecting a value mid final-release#1467Conversation
…l-release lock() checked isDeleted and then unconditionally incremented the strong count, but ~BorrowingReference decrements the count BEFORE calling forceDestroyValue() and holds no mutex while doing so. A lock() landing in that window handed out a strong reference to a value whose final release was already in flight, and the resurrected reference's destructor then ran a second forceDestroyValue() concurrently with the releaser's. lock() now claims the strong count with an increment-if-not-zero compare-exchange loop, exactly like weak_ptr::lock(), and returns null if the count already reached zero. The private WeakReference -> BorrowingReference lock-constructor no longer increments, since lock() has already claimed the count. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Stumbled onto this while investigating a memory leak I can confirm that this fixes a real, measurable leak for us — release builds on physical RN 0.86 (bridgeless, Hermes), react-native-vision-camera 5.2.1, a HybridObject
Thank you so much Looking forward for the release that includes these |
|
Thanks for validating the fix, I haven't merged it yet because I am not fully happy with code-style. I will get to this very soon to make the code better myself, then merge. |
Part 1 of 3 - #1464 split into atomic PRs as requested. This one is fully standalone and can merge on its own; the other two build on it (order: this one, then #1468, then #1469).
The race
lock()checksisDeletedand then unconditionally increments the strong count. But~BorrowingReferencedecrements the count before callingforceDestroyValue(), and holds no mutex while doing so. Alock()landing in that window hands out a strong reference to a value whose final release is already in flight - and the resurrected reference's destructor then runs a secondforceDestroyValue()concurrently with the releaser's.Nitro allows HybridObject/callback releases on any thread, so the window is reachable.
The fix
lock()now claims the strong count with an increment-if-not-zero compare-exchange loop, exactly likeweak_ptr::lock(). A zero strong count means the final release is already under way, so returning null there is simply correct. The privateWeakReference -> BorrowingReferencelock-constructor no longer increments, sincelock()has already claimed the count.Testing
Compile-checked (NDK clang, C++20) and reasoned through; this specific change has not been soaked on-device on its own, so please review closely. Formatted to match
config/.clang-formatoutput in the surrounding code.🤖 Generated with Claude Code