From e6931caca4ae48c1cabe2de7fa74016279e60b76 Mon Sep 17 00:00:00 2001
From: Samuel Susla <samuelsusla@fb.com>
Date: Thu, 4 Mar 2021 13:11:49 -0800
Subject: [PATCH] Don't retain State in StateWrapperImpl

Summary:
Changelog: [internal]

StateWrapperImpl shouldn't retain State strongly because cleanup of `StateWrapperImpl` is triggered from Java and isn't guaranteed to take happen before runtime is destroyed.

This should resolve crash where `StateWrapperImpl`'s destruction causes a `~Pointer` to be called after runtime is destroyed.

Chain of ownership that will be broken by storing State weakly inside `StateWrapperImpl`.
`StateWrapperImpl -> ParagraphState -> TextLayourManager's cache -> AttributedString -> ShadowView -> EventEmitter -> EventTarget -> Pointer`

{F451105831}

Reviewed By: JoshuaGross

Differential Revision: D26815275

fbshipit-source-id: 0703c6dccc62c1d152923b786a83273fa8a03694
---
 .../react/fabric/jni/StateWrapperImpl.cpp       | 17 ++++++++++++-----
 .../react/fabric/jni/StateWrapperImpl.h         |  2 +-
 ReactCommon/react/renderer/core/State.h         |  1 +
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.cpp
index 9a5b929e9af..c95ac15bc2e 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.cpp
+++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.cpp
@@ -23,17 +23,24 @@ jni::local_ref<StateWrapperImpl::jhybriddata> StateWrapperImpl::initHybrid(
 }
 
 jni::local_ref<ReadableNativeMap::jhybridobject> StateWrapperImpl::getState() {
-  folly::dynamic map = state_->getDynamic();
+  auto state = state_.lock();
+  if (!state) {
+    return nullptr;
+  }
+  folly::dynamic map = state->getDynamic();
   local_ref<ReadableNativeMap::jhybridobject> readableNativeMap =
       ReadableNativeMap::newObjectCxxArgs(map);
   return readableNativeMap;
 }
 
 void StateWrapperImpl::updateStateImpl(NativeMap *map) {
-  // Get folly::dynamic from map
-  auto dynamicMap = map->consume();
-  // Set state
-  state_->updateState(dynamicMap);
+  auto state = state_.lock();
+  if (state) {
+    // Get folly::dynamic from map
+    auto dynamicMap = map->consume();
+    // Set state
+    state->updateState(dynamicMap);
+  }
 }
 
 void StateWrapperImpl::registerNatives() {
diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.h
index 9a88eb0271b..dccd52155b3 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.h
+++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.h
@@ -32,7 +32,7 @@ class StateWrapperImpl : public jni::HybridClass<StateWrapperImpl> {
       jni::alias_ref<jobject> self,
       int callbackRefId);
 
-  State::Shared state_;
+  State::Weak state_;
 
  private:
   jni::alias_ref<StateWrapperImpl::jhybriddata> jhybridobject_;
diff --git a/ReactCommon/react/renderer/core/State.h b/ReactCommon/react/renderer/core/State.h
index c0472fec057..285691fe94a 100644
--- a/ReactCommon/react/renderer/core/State.h
+++ b/ReactCommon/react/renderer/core/State.h
@@ -24,6 +24,7 @@ namespace react {
 class State {
  public:
   using Shared = std::shared_ptr<const State>;
+  using Weak = std::weak_ptr<const State>;
 
   static size_t constexpr initialRevisionValue = 1;