Fix Use After Free With Concurrent Java GC

Summary:
Fixes facebook#1271

Java bindings for Yoga rely solely on garbage collection for memory management. Each Java `YogaNode` has references to its children and parent, along with an `YGNodeRef` which it owns. 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.

This bit of behavior is counterintuitive from the API name anyway, so this diff makes a breaking change to `YGNodeFree` to instead mostly just be a `free()` (with a tracing step whose publishing should be thread safe). The existing behavior is exposed in an added function `YGNodeDetachAndFree()`.

The majority of existing `YGNodeFree` usages will probably still work under the new behavior, but we don't have that many in fbsource (many instead call `YGNodeFreeRecursive` to free a whole tree at once). So for safety, I replaced existing usages of `YGNodeFree` outside of Yoga with `YGNodeDetachAndFree` to preserve existing behavior.

JavaScript and C# bindings also use `YGNodeFree()`, and both allow explicit freeing. For now I switched both of their behavior to detachment, to avoid exposing the ability to managed languages to corrupt memory.

Differential Revision: D45556206

fbshipit-source-id: 27a7c94ffea566e51b23cf3d7c48b94761624936

csharp/Facebook.Yoga/Native.cs

@@ -31,6 +31,9 @@ public static extern void YGInteropSetLogger(
         [DllImport(DllName, ExactSpelling = true, CallingConvention = CallingConvention.Cdecl)]
         public static extern void YGNodeFree(IntPtr node);
 
+        [DllImport(DllName, ExactSpelling = true, CallingConvention = CallingConvention.Cdecl)]
+        public static extern void YGNodeDetachAndFree(IntPtr node);
+
         [DllImport(DllName, ExactSpelling = true, CallingConvention = CallingConvention.Cdecl)]
         public static extern void YGNodeReset(YGNodeHandle node);
 

csharp/Facebook.Yoga/YGNodeHandle.cs

@@ -31,7 +31,7 @@ protected override bool ReleaseHandle()
             ReleaseManaged();
             if (!IsInvalid)
             {
-                Native.YGNodeFree(this.handle);
+                Native.YGNodeDetachAndFree(this.handle);
                 GC.KeepAlive(this);
             }
             return true;

javascript/src_native/Node.cc

@@ -61,7 +61,7 @@ Node::Node(Config* config)
 }
 
 Node::~Node(void) {
-  YGNodeFree(m_node);
+  YGNodeDetachAndFree(m_node);
 }
 
 void Node::reset(void) {

yoga/Yoga.cpp

@@ -226,6 +226,11 @@ YOGA_EXPORT YGNodeRef YGNodeClone(YGNodeRef oldNode) {
 }
 
 YOGA_EXPORT void YGNodeFree(const YGNodeRef node) {
+  Event::publish<Event::NodeDeallocation>(node, {node->getConfig()});
+  delete node;
+}
+
+YOGA_EXPORT void YGNodeDetachAndFree(const YGNodeRef node) {
   if (YGNodeRef owner = node->getOwner()) {
     owner->removeChild(node);
     node->setOwner(nullptr);

yoga/Yoga.h

@@ -58,6 +58,7 @@ WIN_EXPORT YGNodeRef YGNodeNew(void);
 WIN_EXPORT YGNodeRef YGNodeNewWithConfig(YGConfigRef config);
 WIN_EXPORT YGNodeRef YGNodeClone(YGNodeRef node);
 WIN_EXPORT void YGNodeFree(YGNodeRef node);
+WIN_EXPORT void YGNodeDeatchAndFree(YGNodeRef node);
 WIN_EXPORT void YGNodeFreeRecursiveWithCleanupFunc(
     YGNodeRef node,
     YGNodeCleanupFunc cleanup);