Fix use-after-free if JNI Yoga nodes are garbage collected using mult…

…iple threads (#37243)

Summary:
Pull Request resolved: #37243

X-link: facebook/litho#944

X-link: facebook/yoga#1279

Java bindings for Yoga rely solely on garbage collection for memory management. Each Java `YogaNode` has references to its children and parent Java Nodes. This means, for a node to be garbage collected, it cannot be reachable from any user accessible node. Each node then has single ownership of a `YGNodeRef`. When the `YogaNode` is garbage collected, a finalizer is run to call `YGNodeFree` and free the underlying native Yoga Node.

This may cause a use-after-free if finalizers are run from multiple threads. This is because `YGNodeFree` does more than just freeing, but instead also interacts with its parent and children nodes to detach itself, and remove any dangling pointers. If multiple threads run finalizers at once, one may traverse and try to mutate a node which another is freeing.

Because we know the entire connected tree is dead, there is no need to remove dangling pointers, so I want to expose a way to just free a Yoga Node, without it mutating the tree as a side effect.

This adds a currently private `YGNodeDeallocate` that frees without traversal. Ideally from naming this is what `YGNodeFree` would do, but we think changing the behavior of that might be too disruptive to OSS. At the same time there may be other memory safety related API changes we would like to eventually make, so this isn't made public beyond the JNI bindings to prevent needing to transition more APIs.

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D45556206

fbshipit-source-id: 62a1394c6f6bdc2b437b388098ea362a0fbcd0f7

packages/react-native/ReactAndroid/src/main/java/com/facebook/yoga/YogaNative.java

@@ -32,7 +32,7 @@ public class YogaNative {
   // YGNode related
   static native long jni_YGNodeNewJNI();
   static native long jni_YGNodeNewWithConfigJNI(long configPointer);
-  static native void jni_YGNodeFreeJNI(long nativePointer);
+  static native void jni_YGNodeDeallocateJNI(long nativePointer);
   static native void jni_YGNodeResetJNI(long nativePointer);
   static native void jni_YGNodeInsertChildJNI(long nativePointer, long childPointer, int index);
   static native void jni_YGNodeSwapChildJNI(long nativePointer, long childPointer, int index);

packages/react-native/ReactAndroid/src/main/java/com/facebook/yoga/YogaNodeJNIFinalizer.java

@@ -29,7 +29,7 @@ public void freeNatives() {
     if (mNativePointer != 0) {
       long nativePointer = mNativePointer;
       mNativePointer = 0;
-      YogaNative.jni_YGNodeFreeJNI(nativePointer);
+      YogaNative.jni_YGNodeDeallocateJNI(nativePointer);
     }
   }
 }

packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNIVanilla.cpp

@@ -199,7 +199,10 @@ static void jni_YGConfigSetLoggerJNI(
   }
 }
 
-static void jni_YGNodeFreeJNI(JNIEnv* env, jobject obj, jlong nativePointer) {
+static void jni_YGNodeDeallocateJNI(
+    JNIEnv* env,
+    jobject obj,
+    jlong nativePointer) {
   if (nativePointer == 0) {
     return;
   }
@@ -769,7 +772,7 @@ static JNINativeMethod methods[] = {
      (void*) jni_YGConfigSetLoggerJNI},
     {"jni_YGNodeNewJNI", "()J", (void*) jni_YGNodeNewJNI},
     {"jni_YGNodeNewWithConfigJNI", "(J)J", (void*) jni_YGNodeNewWithConfigJNI},
-    {"jni_YGNodeFreeJNI", "(J)V", (void*) jni_YGNodeFreeJNI},
+    {"jni_YGNodeDeallocateJNI", "(J)V", (void*) jni_YGNodeDeallocateJNI},
     {"jni_YGNodeResetJNI", "(J)V", (void*) jni_YGNodeResetJNI},
     {"jni_YGNodeInsertChildJNI", "(JJI)V", (void*) jni_YGNodeInsertChildJNI},
     {"jni_YGNodeSwapChildJNI", "(JJI)V", (void*) jni_YGNodeSwapChildJNI},

packages/react-native/ReactCommon/yoga/yoga/Yoga-internal.h

@@ -27,6 +27,10 @@ void YGNodeCalculateLayoutWithContext(
     YGDirection ownerDirection,
     void* layoutContext);
 
+// Deallocates a Yoga Node. Unlike YGNodeFree, does not remove the node from
+// its parent or children.
+void YGNodeDeallocate(YGNodeRef node);
+
 YG_EXTERN_C_END
 
 namespace facebook {

packages/react-native/ReactCommon/yoga/yoga/Yoga.cpp

@@ -230,6 +230,10 @@ YOGA_EXPORT void YGNodeFree(const YGNodeRef node) {
   }
 
   node->clearChildren();
+  YGNodeDeallocate(node);
+}
+
+YOGA_EXPORT void YGNodeDeallocate(const YGNodeRef node) {
   Event::publish<Event::NodeDeallocation>(node, {node->getConfig()});
   delete node;
 }