From 84fbad6215692af7f00a0aebd9db542fcbc31c7a Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Wed, 26 Sep 2018 10:02:09 -0700 Subject: [PATCH] Fabric: Safer UIManager deallocation and uninstallation Summary: We have to uninstall UIManager synchronously to avoid a race condition when JS is capable to call already deallocated UIManager. Reviewed By: mdvacca Differential Revision: D10033406 fbshipit-source-id: 194d1ae2dd5ab09b036b1c165de289ada8e66014 --- ReactCommon/fabric/events/EventBeat.cpp | 9 ++++----- ReactCommon/fabric/uimanager/FabricUIManager.cpp | 13 ++++++++++--- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/ReactCommon/fabric/events/EventBeat.cpp b/ReactCommon/fabric/events/EventBeat.cpp index 327ff63d942..8dd62828b30 100644 --- a/ReactCommon/fabric/events/EventBeat.cpp +++ b/ReactCommon/fabric/events/EventBeat.cpp @@ -19,12 +19,11 @@ void EventBeat::beat() const { return; } - if (!beatCallback_) { - return; - } - - beatCallback_(); isRequested_ = false; + + if (beatCallback_) { + beatCallback_(); + } } void EventBeat::induce() const { diff --git a/ReactCommon/fabric/uimanager/FabricUIManager.cpp b/ReactCommon/fabric/uimanager/FabricUIManager.cpp index e3e71ac53a0..6bc96f956a6 100644 --- a/ReactCommon/fabric/uimanager/FabricUIManager.cpp +++ b/ReactCommon/fabric/uimanager/FabricUIManager.cpp @@ -104,9 +104,16 @@ static const std::string componentNameByReactViewName(std::string viewName) { } FabricUIManager::~FabricUIManager() { - (*executor_)([this] { - uninstaller_(); - }); + // We move `executor_` and `uninstaller_` inside a lambda to extend their + // life-time until the lambda finishes. + auto executor = std::shared_ptr {std::move(executor_)}; + auto uninstaller = std::move(uninstaller_); + + // We have to call this synchronously to postpose UIManager deallocation + // until it is fully uninstalled and JavaScript cannot access this anymore. + (*executor)([uninstaller, executor]() { + uninstaller(); + }, EventBeatBasedExecutor::Mode::Synchronous); } void FabricUIManager::setComponentDescriptorRegistry(const SharedComponentDescriptorRegistry &componentDescriptorRegistry) {