Skip to content

Commit

Permalink
TM iOS: Use weak_ptr to pass around Instance to avoid unreleased refs
Browse files Browse the repository at this point in the history
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<Instance>.

Reviewed By: RSNara

Differential Revision: D14739181

fbshipit-source-id: f9f2a55486debaeb28d3d293df3cf1d3f6b9a031
  • Loading branch information
fkgozali authored and facebook-github-bot committed Apr 3, 2019
1 parent 2387d7d commit 71a8944
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 7 deletions.
2 changes: 1 addition & 1 deletion React/CxxBridge/RCTCxxBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ - (void) setRCTTurboModuleLookupDelegate:(id<RCTTurboModuleLookupDelegate>)turbo
return _jsMessageThread;
}

- (std::shared_ptr<Instance>)reactInstance
- (std::weak_ptr<Instance>)reactInstance
{
return _reactInstance;
}
Expand Down
7 changes: 4 additions & 3 deletions ReactCommon/turbomodule/core/JSCallInvoker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@
namespace facebook {
namespace react {

JSCallInvoker::JSCallInvoker(std::shared_ptr<Instance> reactInstance)
JSCallInvoker::JSCallInvoker(std::weak_ptr<Instance> reactInstance)
: reactInstance_(reactInstance) {}

void JSCallInvoker::invokeAsync(std::function<void()>&& func) {
if (reactInstance_ == nullptr) {
auto instance = reactInstance_.lock();
if (instance == nullptr) {
return;
}
reactInstance_->invokeAsync(std::move(func));
instance->invokeAsync(std::move(func));
}

} // namespace react
Expand Down
4 changes: 2 additions & 2 deletions ReactCommon/turbomodule/core/JSCallInvoker.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ class Instance;
*/
class JSCallInvoker {
public:
JSCallInvoker(std::shared_ptr<Instance> reactInstance);
JSCallInvoker(std::weak_ptr<Instance> reactInstance);

void invokeAsync(std::function<void()>&& func);
// TODO: add sync support

private:
std::shared_ptr<Instance> reactInstance_;
std::weak_ptr<Instance> reactInstance_;
};

} // namespace react
Expand Down
2 changes: 1 addition & 1 deletion ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,6 @@ class JSI_EXPORT ObjCTurboModule : public TurboModule {
@interface RCTBridge ()

- (std::shared_ptr<facebook::react::MessageQueueThread>)jsMessageThread;
- (std::shared_ptr<facebook::react::Instance>)reactInstance;
- (std::weak_ptr<facebook::react::Instance>)reactInstance;

@end

0 comments on commit 71a8944

Please sign in to comment.