From 71a8944b3946beff7ba7d1e65c38aa56c30227ec Mon Sep 17 00:00:00 2001 From: Kevin Gozali Date: Tue, 2 Apr 2019 18:06:29 -0700 Subject: [PATCH] TM iOS: Use weak_ptr to pass around Instance to avoid unreleased refs Summary: There is a timing issue when reloading the bridge (in dev mode) and the tear down of the TurboModules. This causes `Instance` to never get freed, hence the "bridge" isn't cleaning up properly. The side effect can be bogus error saying that it's unable to find a module. To address this, JSCallInvoker should just take in weak_ptr. Reviewed By: RSNara Differential Revision: D14739181 fbshipit-source-id: f9f2a55486debaeb28d3d293df3cf1d3f6b9a031 --- React/CxxBridge/RCTCxxBridge.mm | 2 +- ReactCommon/turbomodule/core/JSCallInvoker.cpp | 7 ++++--- ReactCommon/turbomodule/core/JSCallInvoker.h | 4 ++-- ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm index cea1e0d840d7c6..163fe44fd1de9f 100644 --- a/React/CxxBridge/RCTCxxBridge.mm +++ b/React/CxxBridge/RCTCxxBridge.mm @@ -210,7 +210,7 @@ - (void) setRCTTurboModuleLookupDelegate:(id)turbo return _jsMessageThread; } -- (std::shared_ptr)reactInstance +- (std::weak_ptr)reactInstance { return _reactInstance; } diff --git a/ReactCommon/turbomodule/core/JSCallInvoker.cpp b/ReactCommon/turbomodule/core/JSCallInvoker.cpp index 848e9de9018e18..f58adb91f304f7 100644 --- a/ReactCommon/turbomodule/core/JSCallInvoker.cpp +++ b/ReactCommon/turbomodule/core/JSCallInvoker.cpp @@ -12,14 +12,15 @@ namespace facebook { namespace react { -JSCallInvoker::JSCallInvoker(std::shared_ptr reactInstance) +JSCallInvoker::JSCallInvoker(std::weak_ptr reactInstance) : reactInstance_(reactInstance) {} void JSCallInvoker::invokeAsync(std::function&& func) { - if (reactInstance_ == nullptr) { + auto instance = reactInstance_.lock(); + if (instance == nullptr) { return; } - reactInstance_->invokeAsync(std::move(func)); + instance->invokeAsync(std::move(func)); } } // namespace react diff --git a/ReactCommon/turbomodule/core/JSCallInvoker.h b/ReactCommon/turbomodule/core/JSCallInvoker.h index eff18fa878d89c..770cdf2545d39a 100644 --- a/ReactCommon/turbomodule/core/JSCallInvoker.h +++ b/ReactCommon/turbomodule/core/JSCallInvoker.h @@ -26,13 +26,13 @@ class Instance; */ class JSCallInvoker { public: - JSCallInvoker(std::shared_ptr reactInstance); + JSCallInvoker(std::weak_ptr reactInstance); void invokeAsync(std::function&& func); // TODO: add sync support private: - std::shared_ptr reactInstance_; + std::weak_ptr reactInstance_; }; } // namespace react diff --git a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h index d63d6922268fa4..dbd31c8f4dfef4 100644 --- a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h +++ b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h @@ -91,6 +91,6 @@ class JSI_EXPORT ObjCTurboModule : public TurboModule { @interface RCTBridge () - (std::shared_ptr)jsMessageThread; -- (std::shared_ptr)reactInstance; +- (std::weak_ptr)reactInstance; @end