diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp index 06f60bbcaf74..f7393dbdd520 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp @@ -133,8 +133,4 @@ void RuntimeScheduler::setIntersectionObserverDelegate( intersectionObserverDelegate); } -void RuntimeScheduler::clear() noexcept { - return runtimeSchedulerImpl_->clear(); -} - } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.h b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.h index d81387d2d36a..813537ec7aec 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.h +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.h @@ -64,7 +64,6 @@ class RuntimeSchedulerBase { virtual void setIntersectionObserverDelegate( RuntimeSchedulerIntersectionObserverDelegate* intersectionObserverDelegate) = 0; - virtual void clear() noexcept = 0; }; // This is a proxy for RuntimeScheduler implementation, which will be selected @@ -181,8 +180,6 @@ class RuntimeScheduler final : RuntimeSchedulerBase { RuntimeSchedulerIntersectionObserverDelegate* intersectionObserverDelegate) override; - void clear() noexcept override; - private: // Actual implementation, stored as a unique pointer to simplify memory // management. diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp index 5a0469ee2d11..bd25d702aa73 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp @@ -246,12 +246,6 @@ void RuntimeScheduler_Legacy::setIntersectionObserverDelegate( // No-op in the legacy scheduler } -void RuntimeScheduler_Legacy::clear() noexcept { - // No-op: the legacy scheduler runs rendering updates synchronously in - // `scheduleRenderingUpdate`, so there is no queue to drain. See the header - // for details. -} - #pragma mark - Private void RuntimeScheduler_Legacy::scheduleWorkLoopIfNecessary() { diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h index f36ef6488f9c..397786bc1fef 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h @@ -138,8 +138,6 @@ class RuntimeScheduler_Legacy final : public RuntimeSchedulerBase { RuntimeSchedulerIntersectionObserverDelegate* intersectionObserverDelegate) override; - void clear() noexcept override; - private: std::priority_queue< std::shared_ptr, diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp index edb35147e5a7..989125ee0372 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp @@ -7,8 +7,6 @@ #include "RuntimeScheduler_Modern.h" -#include - #include #include #include @@ -191,7 +189,6 @@ void RuntimeScheduler_Modern::scheduleRenderingUpdate( RuntimeSchedulerRenderingUpdate&& renderingUpdate) { TraceSection s("RuntimeScheduler::scheduleRenderingUpdate"); - std::lock_guard lock(renderingUpdatesMutex_); surfaceIdsWithPendingRenderingUpdates_.insert(surfaceId); pendingRenderingUpdates_.push(renderingUpdate); } @@ -218,23 +215,6 @@ void RuntimeScheduler_Modern::setIntersectionObserverDelegate( intersectionObserverDelegate_ = intersectionObserverDelegate; } -void RuntimeScheduler_Modern::clear() noexcept { - TraceSection s("RuntimeScheduler::clear"); - - // Drop any pending rendering updates. The callbacks captured here may - // reference state owned by the caller (e.g. the `Scheduler`'s delegate); - // dropping them here guarantees they cannot be invoked on the JS thread - // after that state is destroyed. - std::lock_guard lock(renderingUpdatesMutex_); - auto droppedUpdates = pendingRenderingUpdates_.size(); - auto droppedSurfaces = surfaceIdsWithPendingRenderingUpdates_.size(); - pendingRenderingUpdates_ = {}; - surfaceIdsWithPendingRenderingUpdates_.clear(); - LOG(WARNING) << "RuntimeScheduler_Modern::clear() dropped " - << droppedUpdates << " pending rendering update(s) across " - << droppedSurfaces << " surface(s)."; -} - #pragma mark - Private void RuntimeScheduler_Modern::scheduleTask(std::shared_ptr task) { @@ -352,13 +332,6 @@ void RuntimeScheduler_Modern::runEventLoopTick( void RuntimeScheduler_Modern::updateRendering() { TraceSection s("RuntimeScheduler::updateRendering"); - // Hold the lock across the entire step so that `clear()` (called from - // another thread during `Scheduler` destruction) blocks until we are done - // running the callbacks. Otherwise `clear()` could return while we are - // still about to invoke lambdas that capture raw pointers into the - // now-destroyed `Scheduler`'s delegate. - std::lock_guard lock(renderingUpdatesMutex_); - // This is the integration of the Event Timing API in the Event Loop. // See https://w3c.github.io/event-timing/#sec-modifications-HTML const auto eventTimingDelegate = eventTimingDelegate_.load(); diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h index 2d0a85c71fa4..83d96517fd3e 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h @@ -13,7 +13,6 @@ #include #include #include -#include #include #include @@ -156,8 +155,6 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase { RuntimeSchedulerIntersectionObserverDelegate* intersectionObserverDelegate) override; - void clear() noexcept override; - private: std::atomic syncTaskRequests_{0}; @@ -223,14 +220,6 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase { */ bool isEventLoopScheduled_{false}; - /** - * Protects `pendingRenderingUpdates_` and - * `surfaceIdsWithPendingRenderingUpdates_` so that the queue can be safely - * cleared from another thread (e.g. when the owning `Scheduler` is - * destroyed) without racing with the JS thread that normally pushes and - * drains it. - */ - std::mutex renderingUpdatesMutex_; std::queue pendingRenderingUpdates_; std::unordered_set surfaceIdsWithPendingRenderingUpdates_; diff --git a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp index 3e8b94b0bba1..b43d082426d1 100644 --- a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp +++ b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp @@ -27,7 +27,8 @@ namespace facebook::react { Scheduler::Scheduler( const SchedulerToolbox& schedulerToolbox, UIManagerAnimationDelegate* animationDelegate, - SchedulerDelegate* delegate) { + SchedulerDelegate* delegate) + : delegateInvalidated_(std::make_shared>(false)) { runtimeExecutor_ = schedulerToolbox.runtimeExecutor; contextContainer_ = schedulerToolbox.contextContainer; @@ -142,6 +143,12 @@ Scheduler::~Scheduler() { LOG(WARNING) << "Scheduler::~Scheduler() was called (address: " << this << ")."; + // Invalidate any lambdas already queued via scheduleRenderingUpdate that + // captured a raw delegate_ pointer; without this they'd dereference a + // dangling SchedulerDelegate after Scheduler teardown. (No replacement + // token is allocated here - Scheduler is going away.) + *delegateInvalidated_ = true; + auto weakRuntimeScheduler = contextContainer_->find>( RuntimeSchedulerKey); @@ -165,23 +172,6 @@ Scheduler::~Scheduler() { uiManager_->setDelegate(nullptr); uiManager_->setAnimationDelegate(nullptr); - // After detaching from UIManager, no more calls can be made into this - // Scheduler, so nothing new will be pushed into the RuntimeScheduler's - // rendering-update queue. Drop whatever is still queued: those lambdas - // capture raw pointers to this Scheduler's delegate (via - // `uiManagerDidDispatchCommand` / `uiManagerDidFinishTransaction`), and - // would otherwise fire on the JS thread after the delegate is destroyed. - // `clear()` blocks until any in-flight `updateRendering` finishes, so it - // is safe to destroy the delegate after this point. -#if __APPLE__ - if (runtimeScheduler) { - LOG(WARNING) - << "Scheduler::~Scheduler() clearing RuntimeScheduler rendering-update queue (address: " - << this << ")."; - runtimeScheduler->clear(); - } -#endif - // Then, let's verify that the requirement was satisfied. auto surfaceIds = std::vector{}; uiManager_->getShadowTreeRegistry().enumerate( @@ -240,6 +230,19 @@ Scheduler::findComponentDescriptorByHandle_DO_NOT_USE_THIS_IS_BROKEN( #pragma mark - Delegate void Scheduler::setDelegate(SchedulerDelegate* delegate) { + if (delegate_ != delegate) { + // Mark the *current* token invalid: any rendering-update lambda already + // queued holds a shared_ptr to this atomic and will observe `true` on + // its next read, so it no-ops instead of calling into the previous + // delegate (which the caller is about to drop). + *delegateInvalidated_ = true; + // Then install a *fresh* token (a new atomic) so lambdas captured + // against the new delegate use their own non-invalidated flag. + // Reusing the previous atomic and flipping it back to `false` would + // re-arm the in-flight lambdas - exactly the use-after-free we're + // trying to prevent - because they share the same shared_ptr. + delegateInvalidated_ = std::make_shared>(false); + } delegate_ = delegate; } @@ -271,7 +274,13 @@ void Scheduler::uiManagerDidFinishTransaction( runtimeScheduler_->scheduleRenderingUpdate( surfaceId, [delegate = delegate_, + invalidated = delegateInvalidated_, mountingCoordinator = std::move(mountingCoordinator)]() { + if (*invalidated) { + LOG(WARNING) + << "[Scheduler] skipped deferred transaction because delegate token was invalidated"; + return; + } delegate->schedulerShouldRenderTransactions(mountingCoordinator); }); } else { @@ -297,9 +306,17 @@ void Scheduler::uiManagerDidDispatchCommand( runtimeScheduler_->scheduleRenderingUpdate( shadowNode->getSurfaceId(), [delegate = delegate_, + invalidated = delegateInvalidated_, shadowView = std::move(shadowView), commandName, args]() { + if (*invalidated) { + LOG(WARNING) + << "[Scheduler] skipped deferred command because delegate token was invalidated" + << " componentName=" << shadowView.componentName + << " command=" << commandName; + return; + } #if __APPLE__ LOG(WARNING) << "[Scheduler] uiManagerDidDispatchCommand componentName=" << shadowView.componentName << " command=" << commandName; #endif diff --git a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.h b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.h index 20fc3211bb2c..ec5adae09ce6 100644 --- a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.h +++ b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.h @@ -7,6 +7,7 @@ #pragma once +#include #include #include @@ -127,6 +128,11 @@ class Scheduler final : public UIManagerDelegate { friend class SurfaceHandler; SchedulerDelegate* delegate_; + // Invalidation token captured by-value into lambdas deferred via + // runtimeScheduler_->scheduleRenderingUpdate. Set to true on delegate + // change or Scheduler destruction so a lambda that outlives its captured + // raw delegate pointer can no-op instead of dereferencing dangling memory. + std::shared_ptr> delegateInvalidated_; SharedComponentDescriptorRegistry componentDescriptorRegistry_; RuntimeExecutor runtimeExecutor_; std::shared_ptr uiManager_; diff --git a/private/cxx-public-api/ReactNativeCPP.api b/private/cxx-public-api/ReactNativeCPP.api index 4771e880eb56..8c2f51b8e1b5 100644 --- a/private/cxx-public-api/ReactNativeCPP.api +++ b/private/cxx-public-api/ReactNativeCPP.api @@ -35928,7 +35928,6 @@ class RuntimeSchedulerBase { virtual void setIntersectionObserverDelegate( RuntimeSchedulerIntersectionObserverDelegate* intersectionObserverDelegate) = 0; - virtual void clear() noexcept = 0; }; class RuntimeScheduler final : RuntimeSchedulerBase { public: @@ -35973,7 +35972,6 @@ class RuntimeScheduler final : RuntimeSchedulerBase { void setIntersectionObserverDelegate( RuntimeSchedulerIntersectionObserverDelegate* intersectionObserverDelegate) override; - void clear() noexcept override; }; } // namespace facebook::react @@ -36074,7 +36072,6 @@ class RuntimeScheduler_Legacy final : public RuntimeSchedulerBase { void setIntersectionObserverDelegate( RuntimeSchedulerIntersectionObserverDelegate* intersectionObserverDelegate) override; - void clear() noexcept override; }; std::atomic runtimeAccessRequests_{0}; std::atomic_bool isSynchronous_{false}; @@ -36139,7 +36136,6 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase { void setIntersectionObserverDelegate( RuntimeSchedulerIntersectionObserverDelegate* intersectionObserverDelegate) override; - void clear() noexcept override; }; std::priority_queue< std::shared_ptr, @@ -36176,7 +36172,6 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase { HighResTimeStamp endTime); std::function now_; bool isEventLoopScheduled_{false}; - std::mutex renderingUpdatesMutex_; std::queue pendingRenderingUpdates_; std::unordered_set surfaceIdsWithPendingRenderingUpdates_; std::atomic