Java objects should own C++ State

Summary: When passing StateWrapper objects across the JNI, we were not ensuring that the Java objects would own the C++ state. This was initially done because I assumed that in Java, State would either be used immediately or discarded, so this wouldn't be unsafe. As it turns out, it makes sense in some cases to store the StateWrapper in Java and use it later, potentially even in other threads, so we need to make sure we maintain ownership of the C++ object from the Java object.

Reviewed By: shergin

Differential Revision: D15206194

fbshipit-source-id: a437d921ba00b194cf08bad80666bd99baf11d52

ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp

@@ -287,14 +287,12 @@ local_ref<JMountItem::javaobject> createUpdateStateMountItem(
 
   auto state = mutation.newChildShadowView.state;
 
-  // We use state.get() to pass a raw pointer through the JNI
-  // We don't need to access the state ptr in Java, but we need to be able to
-  // pass a state object back through the JNI for state updates
-
   // Do not hold onto Java object from C
+  // We DO want to hold onto C object from Java, since we don't know the
+  // lifetime of the Java object
   auto javaStateWrapper = StateWrapperImpl::newObjectJavaArgs();
   StateWrapperImpl* cStateWrapper = cthis(javaStateWrapper);
-  cStateWrapper->state_ = state.get();
+  cStateWrapper->state_ = state;
 
   return updateStateInstruction(
       javaUIManager,

ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/StateWrapperImpl.h

@@ -23,7 +23,7 @@ class StateWrapperImpl : public jni::HybridClass<StateWrapperImpl> {
   jni::local_ref<ReadableNativeMap::jhybridobject> getState();
   void updateStateImpl(NativeMap *map);
 
-  const State* state_;
+  State::Shared state_;
  private:
   jni::alias_ref<StateWrapperImpl::jhybriddata> jhybridobject_;