From 440d9b954cb4d9115b10cef72f2ef4ef199d9ff2 Mon Sep 17 00:00:00 2001 From: HyunWoo Lee Date: Thu, 29 Oct 2026 01:10:44 +0900 Subject: [PATCH] Fix use-after-free in ~SyncCallback when a callback is destroyed off the JS thread --- .../ReactCommon/react/bridging/Function.h | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/react-native/ReactCommon/react/bridging/Function.h b/packages/react-native/ReactCommon/react/bridging/Function.h index 19aaec53c006..07658e60bd96 100644 --- a/packages/react-native/ReactCommon/react/bridging/Function.h +++ b/packages/react-native/ReactCommon/react/bridging/Function.h @@ -106,7 +106,9 @@ template class SyncCallback { public: SyncCallback(jsi::Runtime &rt, jsi::Function function, std::shared_ptr jsInvoker) - : wrapper_(CallbackWrapper::createWeak(std::move(function), rt, std::move(jsInvoker))) + : wrapper_(CallbackWrapper::createWeak(std::move(function), rt, std::move(jsInvoker))), + collection_(&LongLivedObjectCollection::get(rt)), + wrapperPtr_(wrapper_.lock().get()) { } @@ -116,18 +118,30 @@ class SyncCallback { SyncCallback &operator=(const SyncCallback &) = delete; // Allow move - SyncCallback(SyncCallback &&other) noexcept : wrapper_(std::move(other.wrapper_)) {} + SyncCallback(SyncCallback &&other) noexcept + : wrapper_(std::move(other.wrapper_)), + collection_(std::exchange(other.collection_, nullptr)), + wrapperPtr_(std::exchange(other.wrapperPtr_, nullptr)) + { + } SyncCallback &operator=(SyncCallback &&other) noexcept { wrapper_ = std::move(other.wrapper_); + collection_ = std::exchange(other.collection_, nullptr); + wrapperPtr_ = std::exchange(other.wrapperPtr_, nullptr); return *this; } ~SyncCallback() { - if (auto wrapper = wrapper_.lock()) { - wrapper->destroy(); + // May run on any thread (e.g. the fbjni destructor thread). Locking wrapper_ + // here could leave this thread with the last reference outside the + // collection's lock, racing with clear() during runtime teardown. Remove by + // address instead, so any release happens under that lock. expired() skips + // wrappers that clear() already released, whose address may be reused. + if (collection_ != nullptr && !wrapper_.expired()) { + collection_->remove(wrapperPtr_); } } @@ -179,6 +193,11 @@ class SyncCallback { // Held weakly so lifetime is managed by LongLivedObjectCollection. std::weak_ptr wrapper_; + // Used by the destructor to release the wrapper without locking wrapper_. + // The collection outlives the runtime: LongLivedObjectCollection::get() never + // erases its entries. + LongLivedObjectCollection *collection_; + const LongLivedObject *wrapperPtr_; }; template