From 151383d30286505cf4423278ca4321c012d47999 Mon Sep 17 00:00:00 2001 From: Ruslan Shestopalyuk Date: Fri, 21 Nov 2025 03:26:38 -0800 Subject: [PATCH] Explicit shutdown of TimerManager to ensure this is done on the main thread Summary: # Changelog [Internal] - It makes sense to explicitly shut down TimerManager instance to make sure that this is done from the place and thread it's intended to - otherwise, since lifetime of TimerManager instance is managed by a shared pointer, this can lead to situations, whereas some other thread retains the shared pointer and calls the shut down at some later stage, which can lead to all kinds of unintended consequences. Differential Revision: D87631467 --- .../react/runtime/PlatformTimerRegistry.h | 2 ++ .../ReactCommon/react/runtime/ReactInstance.cpp | 13 +++++++++---- .../ReactCommon/react/runtime/ReactInstance.h | 2 ++ .../ReactCommon/react/runtime/TimerManager.cpp | 12 ++++++++++++ .../ReactCommon/react/runtime/TimerManager.h | 3 +++ .../cxx/react/runtime/PlatformTimerRegistryImpl.cpp | 6 ++---- .../cxx/react/runtime/PlatformTimerRegistryImpl.h | 4 +++- 7 files changed, 33 insertions(+), 9 deletions(-) diff --git a/packages/react-native/ReactCommon/react/runtime/PlatformTimerRegistry.h b/packages/react-native/ReactCommon/react/runtime/PlatformTimerRegistry.h index 9b65875e5d55fb..54e5a247be11a5 100644 --- a/packages/react-native/ReactCommon/react/runtime/PlatformTimerRegistry.h +++ b/packages/react-native/ReactCommon/react/runtime/PlatformTimerRegistry.h @@ -25,6 +25,8 @@ class PlatformTimerRegistry { virtual void createRecurringTimer(uint32_t timerID, double delayMS) = 0; virtual ~PlatformTimerRegistry() noexcept = default; + + virtual void quit() {} }; using TimerManagerDelegate = PlatformTimerRegistry; diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index 46fa1c65f94799..8e39f07e7cb389 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -162,6 +162,11 @@ ReactInstance::ReactInstance( runtimeScheduler->scheduleWork(std::move(callback)); }); } +ReactInstance::~ReactInstance() noexcept { + if (timerManager_ != nullptr) { + timerManager_->quit(); + } +} void ReactInstance::unregisterFromInspector() { if (inspectorTarget_ != nullptr) { @@ -184,8 +189,8 @@ RuntimeExecutor ReactInstance::getUnbufferedRuntimeExecutor() noexcept { // This BufferedRuntimeExecutor ensures that the main JS bundle finished // execution before any JS queued into it from C++ are executed. Use -// getUnbufferedRuntimeExecutor() instead if you do not need the main JS bundle -// to have finished. e.g. setting global variables into JS runtime. +// getUnbufferedRuntimeExecutor() instead if you do not need the main JS +// bundle to have finished. e.g. setting global variables into JS runtime. RuntimeExecutor ReactInstance::getBufferedRuntimeExecutor() noexcept { return [weakBufferedRuntimeExecutor_ = std::weak_ptr(bufferedRuntimeExecutor_)]( @@ -219,8 +224,8 @@ std::string simpleBasename(const std::string& path) { * Load the JS bundle and flush buffered JS calls, future JS calls won't be * buffered after calling this. * Note that this method is asynchronous. However, a completion callback - * isn't needed because all calls into JS should be dispatched to the JSThread, - * preferably via the runtimeExecutor_. + * isn't needed because all calls into JS should be dispatched to the + * JSThread, preferably via the runtimeExecutor_. */ void ReactInstance::loadScript( std::unique_ptr script, diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.h b/packages/react-native/ReactCommon/react/runtime/ReactInstance.h index 84a8b4cc2cb2d7..b1280c4150e900 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.h +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.h @@ -31,6 +31,8 @@ class ReactInstance final : private jsinspector_modern::InstanceTargetDelegate { JsErrorHandler::OnJsError onJsError, jsinspector_modern::HostTarget *parentInspectorTarget = nullptr); + ~ReactInstance() noexcept; + RuntimeExecutor getUnbufferedRuntimeExecutor() noexcept; RuntimeExecutor getBufferedRuntimeExecutor() noexcept; diff --git a/packages/react-native/ReactCommon/react/runtime/TimerManager.cpp b/packages/react-native/ReactCommon/react/runtime/TimerManager.cpp index 1e31180bd36701..5a33246720a06d 100644 --- a/packages/react-native/ReactCommon/react/runtime/TimerManager.cpp +++ b/packages/react-native/ReactCommon/react/runtime/TimerManager.cpp @@ -56,6 +56,18 @@ TimerManager::TimerManager( std::unique_ptr platformTimerRegistry) noexcept : platformTimerRegistry_(std::move(platformTimerRegistry)) {} +TimerManager::~TimerManager() noexcept { + quit(); +} + +void TimerManager::quit() { + if (platformTimerRegistry_ == nullptr) { + return; + } + platformTimerRegistry_->quit(); + platformTimerRegistry_ = nullptr; +} + void TimerManager::setRuntimeExecutor( RuntimeExecutor runtimeExecutor) noexcept { runtimeExecutor_ = std::move(runtimeExecutor); diff --git a/packages/react-native/ReactCommon/react/runtime/TimerManager.h b/packages/react-native/ReactCommon/react/runtime/TimerManager.h index 7be8772505b69a..14c06c1338475f 100644 --- a/packages/react-native/ReactCommon/react/runtime/TimerManager.h +++ b/packages/react-native/ReactCommon/react/runtime/TimerManager.h @@ -46,6 +46,7 @@ struct TimerCallback { class TimerManager { public: explicit TimerManager(std::unique_ptr platformTimerRegistry) noexcept; + ~TimerManager() noexcept; void setRuntimeExecutor(RuntimeExecutor runtimeExecutor) noexcept; @@ -53,6 +54,8 @@ class TimerManager { void attachGlobals(jsi::Runtime &runtime); + void quit(); + private: TimerHandle createTimer( jsi::Function &&callback, diff --git a/packages/react-native/ReactCxxPlatform/react/runtime/platform/cxx/react/runtime/PlatformTimerRegistryImpl.cpp b/packages/react-native/ReactCxxPlatform/react/runtime/platform/cxx/react/runtime/PlatformTimerRegistryImpl.cpp index 6890e0c9f8a7fd..ad0fb40d56f6db 100644 --- a/packages/react-native/ReactCxxPlatform/react/runtime/platform/cxx/react/runtime/PlatformTimerRegistryImpl.cpp +++ b/packages/react-native/ReactCxxPlatform/react/runtime/platform/cxx/react/runtime/PlatformTimerRegistryImpl.cpp @@ -12,10 +12,8 @@ namespace facebook::react { -PlatformTimerRegistryImpl::~PlatformTimerRegistryImpl() noexcept { - LOG(INFO) - << "PlatformTimerRegistryImpl::~PlatformTimerRegistryImpl() was called (address: " - << this << ")."; +void PlatformTimerRegistryImpl::quit() { + LOG(INFO) << "Shutting down PlatformTimerRegistryImpl..."; taskDispatchThread_.quit(); std::lock_guard guard(timersMutex_); timers_.clear(); diff --git a/packages/react-native/ReactCxxPlatform/react/runtime/platform/cxx/react/runtime/PlatformTimerRegistryImpl.h b/packages/react-native/ReactCxxPlatform/react/runtime/platform/cxx/react/runtime/PlatformTimerRegistryImpl.h index d1ae13a518f9e7..c5a7c8f1c45828 100644 --- a/packages/react-native/ReactCxxPlatform/react/runtime/platform/cxx/react/runtime/PlatformTimerRegistryImpl.h +++ b/packages/react-native/ReactCxxPlatform/react/runtime/platform/cxx/react/runtime/PlatformTimerRegistryImpl.h @@ -23,7 +23,7 @@ class PlatformTimerRegistryImpl : public PlatformTimerRegistry { PlatformTimerRegistryImpl &operator=(const PlatformTimerRegistryImpl &) = delete; PlatformTimerRegistryImpl(PlatformTimerRegistryImpl &&) noexcept = delete; PlatformTimerRegistryImpl &operator=(PlatformTimerRegistryImpl &&) noexcept = delete; - ~PlatformTimerRegistryImpl() noexcept override; + ~PlatformTimerRegistryImpl() noexcept = default; void createTimer(uint32_t timerId, double delayMs) override; @@ -33,6 +33,8 @@ class PlatformTimerRegistryImpl : public PlatformTimerRegistry { void setTimerManager(std::weak_ptr timerManager); + void quit() override; + private: struct Timer { uint32_t timerId{0};