From 402ee112732dd918568458c5f7adee5940e63629 Mon Sep 17 00:00:00 2001
From: Pritesh Nandgaonkar <prit91@fb.com>
Date: Fri, 9 Feb 2018 04:47:49 -0800
Subject: [PATCH] Fix memory leak by not duplicating the YGConfig

Summary:
YGConfig isn't owned by the YGNode, and thus isn't freed when freeing the node (`YGNodeFreeRecursive`)

This fixes a memory leak caused by D6856812

Reviewed By: emilsjolander

Differential Revision: D6945022

fbshipit-source-id: 5fd3c3e2ac1cd94d459d5aa06e0daa8f107779ac
---
 tests/YGLayoutDiffingTest.cpp | 19 +++++++++++++++++++
 yoga/Yoga-internal.h          | 11 +++--------
 yoga/Yoga.cpp                 | 30 ++++++++++++++++++++++++++----
 3 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/tests/YGLayoutDiffingTest.cpp b/tests/YGLayoutDiffingTest.cpp
index 349c130f..ab3624bd 100644
--- a/tests/YGLayoutDiffingTest.cpp
+++ b/tests/YGLayoutDiffingTest.cpp
@@ -32,8 +32,14 @@ TEST(YogaTest, assert_layout_trees_are_same) {
   YGNodeStyleSetFlexShrink(root1_child0_child0_child0, 1);
   YGNodeInsertChild(root1_child0_child0, root1_child0_child0_child0, 0);
 
+  const int32_t cal1_configInstanceCount = YGConfigGetInstanceCount();
+  const int32_t cal1_nodeInstanceCount = YGNodeGetInstanceCount();
+
   YGNodeCalculateLayout(root1, YGUndefined, YGUndefined, YGDirectionLTR);
 
+  ASSERT_EQ(YGConfigGetInstanceCount(), cal1_configInstanceCount);
+  ASSERT_EQ(YGNodeGetInstanceCount(), cal1_nodeInstanceCount);
+
   const YGNodeRef root2 = YGNodeNewWithConfig(config);
   YGNodeStyleSetWidth(root2, 500);
   YGNodeStyleSetHeight(root2, 500);
@@ -52,15 +58,28 @@ TEST(YogaTest, assert_layout_trees_are_same) {
   YGNodeStyleSetFlexShrink(root2_child0_child0_child0, 1);
   YGNodeInsertChild(root2_child0_child0, root2_child0_child0_child0, 0);
 
+  const int32_t cal2_configInstanceCount = YGConfigGetInstanceCount();
+  const int32_t cal2_nodeInstanceCount = YGNodeGetInstanceCount();
+
   YGNodeCalculateLayout(root2, YGUndefined, YGUndefined, YGDirectionLTR);
 
+  ASSERT_EQ(YGConfigGetInstanceCount(), cal2_configInstanceCount);
+  ASSERT_EQ(YGNodeGetInstanceCount(), cal2_nodeInstanceCount);
+
   ASSERT_TRUE(YGNodeLayoutGetDidUseLegacyFlag(root1));
   ASSERT_TRUE(YGNodeLayoutGetDidUseLegacyFlag(root2));
   ASSERT_TRUE(root1->isLayoutTreeEqualToNode(*root2));
 
   YGNodeStyleSetAlignItems(root2, YGAlignFlexEnd);
 
+  const int32_t cal3_configInstanceCount = YGConfigGetInstanceCount();
+  const int32_t cal3_nodeInstanceCount = YGNodeGetInstanceCount();
+
   YGNodeCalculateLayout(root2, YGUndefined, YGUndefined, YGDirectionLTR);
+
+  ASSERT_EQ(YGConfigGetInstanceCount(), cal3_configInstanceCount);
+  ASSERT_EQ(YGNodeGetInstanceCount(), cal3_nodeInstanceCount);
+
   ASSERT_FALSE(root1->isLayoutTreeEqualToNode(*root2));
 
   YGNodeFreeRecursive(root1);
diff --git a/yoga/Yoga-internal.h b/yoga/Yoga-internal.h
index 5106d971..291b3ba2 100644
--- a/yoga/Yoga-internal.h
+++ b/yoga/Yoga-internal.h
@@ -68,17 +68,12 @@ struct YGCachedMeasurement {
     if (!std::isnan(computedWidth) || !std::isnan(measurement.computedWidth)) {
       isEqual = isEqual && computedWidth == measurement.computedWidth;
     }
-    if (!std::isnan(
-            computedHeight || !std::isnan(measurement.computedHeight))) {
+    if (!std::isnan(computedHeight) ||
+        !std::isnan(measurement.computedHeight)) {
       isEqual = isEqual && computedHeight == measurement.computedHeight;
     }
 
-    return availableWidth == measurement.availableWidth &&
-        availableHeight == measurement.availableHeight &&
-        widthMeasureMode == measurement.widthMeasureMode &&
-        heightMeasureMode == measurement.heightMeasureMode &&
-        computedWidth == measurement.computedWidth &&
-        computedHeight == measurement.computedHeight;
+    return isEqual;
   }
 };
 
diff --git a/yoga/Yoga.cpp b/yoga/Yoga.cpp
index 6e6806f2..19e8b4cf 100644
--- a/yoga/Yoga.cpp
+++ b/yoga/Yoga.cpp
@@ -251,6 +251,16 @@ YGNodeRef YGNodeClone(YGNodeRef oldNode) {
   return node;
 }
 
+static YGConfigRef YGConfigClone(const YGConfig& oldConfig) {
+  const YGConfigRef config = new YGConfig(oldConfig);
+  YGAssert(config != nullptr, "Could not allocate memory for config");
+  if (config == nullptr) {
+    abort();
+  }
+  gConfigInstanceCount++;
+  return config;
+}
+
 static YGNodeRef YGNodeDeepClone(YGNodeRef oldNode) {
   YGNodeRef node = YGNodeClone(oldNode);
   YGVector vec = YGVector();
@@ -263,12 +273,12 @@ static YGNodeRef YGNodeDeepClone(YGNodeRef oldNode) {
   }
   node->setChildren(vec);
 
-  if (oldNode->getNextChild() != nullptr) {
-    node->setNextChild(YGNodeDeepClone(oldNode->getNextChild()));
+  if (oldNode->getConfig() != nullptr) {
+    node->setConfig(YGConfigClone(*(oldNode->getConfig())));
   }
 
-  if (node->getConfig() != nullptr) {
-    node->setConfig(new YGConfig(*node->getConfig()));
+  if (oldNode->getNextChild() != nullptr) {
+    node->setNextChild(YGNodeDeepClone(oldNode->getNextChild()));
   }
 
   return node;
@@ -291,6 +301,17 @@ void YGNodeFree(const YGNodeRef node) {
   gNodeInstanceCount--;
 }
 
+static void YGConfigFreeRecursive(const YGNodeRef root) {
+  if (root->getConfig() != nullptr) {
+    gConfigInstanceCount--;
+    delete root->getConfig();
+  }
+  // Delete configs recursively for childrens
+  for (uint32_t i = 0; i < root->getChildrenCount(); ++i) {
+    YGConfigFreeRecursive(root->getChild(i));
+  }
+}
+
 void YGNodeFreeRecursive(const YGNodeRef root) {
   while (YGNodeGetChildCount(root) > 0) {
     const YGNodeRef child = YGNodeGetChild(root, 0);
@@ -3642,6 +3663,7 @@ void YGNodeCalculateLayout(const YGNodeRef node,
                 YGPrintOptionsStyle));
       }
     }
+    YGConfigFreeRecursive(originalNode);
     YGNodeFreeRecursive(originalNode);
   }
 }