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
This commit is contained in:
Pritesh Nandgaonkar
2018-02-09 04:47:49 -08:00
committed by Facebook Github Bot
parent 6e38a26f32
commit 402ee11273
3 changed files with 48 additions and 12 deletions

View File

@@ -32,8 +32,14 @@ TEST(YogaTest, assert_layout_trees_are_same) {
YGNodeStyleSetFlexShrink(root1_child0_child0_child0, 1); YGNodeStyleSetFlexShrink(root1_child0_child0_child0, 1);
YGNodeInsertChild(root1_child0_child0, root1_child0_child0_child0, 0); 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); YGNodeCalculateLayout(root1, YGUndefined, YGUndefined, YGDirectionLTR);
ASSERT_EQ(YGConfigGetInstanceCount(), cal1_configInstanceCount);
ASSERT_EQ(YGNodeGetInstanceCount(), cal1_nodeInstanceCount);
const YGNodeRef root2 = YGNodeNewWithConfig(config); const YGNodeRef root2 = YGNodeNewWithConfig(config);
YGNodeStyleSetWidth(root2, 500); YGNodeStyleSetWidth(root2, 500);
YGNodeStyleSetHeight(root2, 500); YGNodeStyleSetHeight(root2, 500);
@@ -52,15 +58,28 @@ TEST(YogaTest, assert_layout_trees_are_same) {
YGNodeStyleSetFlexShrink(root2_child0_child0_child0, 1); YGNodeStyleSetFlexShrink(root2_child0_child0_child0, 1);
YGNodeInsertChild(root2_child0_child0, root2_child0_child0_child0, 0); 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); YGNodeCalculateLayout(root2, YGUndefined, YGUndefined, YGDirectionLTR);
ASSERT_EQ(YGConfigGetInstanceCount(), cal2_configInstanceCount);
ASSERT_EQ(YGNodeGetInstanceCount(), cal2_nodeInstanceCount);
ASSERT_TRUE(YGNodeLayoutGetDidUseLegacyFlag(root1)); ASSERT_TRUE(YGNodeLayoutGetDidUseLegacyFlag(root1));
ASSERT_TRUE(YGNodeLayoutGetDidUseLegacyFlag(root2)); ASSERT_TRUE(YGNodeLayoutGetDidUseLegacyFlag(root2));
ASSERT_TRUE(root1->isLayoutTreeEqualToNode(*root2)); ASSERT_TRUE(root1->isLayoutTreeEqualToNode(*root2));
YGNodeStyleSetAlignItems(root2, YGAlignFlexEnd); YGNodeStyleSetAlignItems(root2, YGAlignFlexEnd);
const int32_t cal3_configInstanceCount = YGConfigGetInstanceCount();
const int32_t cal3_nodeInstanceCount = YGNodeGetInstanceCount();
YGNodeCalculateLayout(root2, YGUndefined, YGUndefined, YGDirectionLTR); YGNodeCalculateLayout(root2, YGUndefined, YGUndefined, YGDirectionLTR);
ASSERT_EQ(YGConfigGetInstanceCount(), cal3_configInstanceCount);
ASSERT_EQ(YGNodeGetInstanceCount(), cal3_nodeInstanceCount);
ASSERT_FALSE(root1->isLayoutTreeEqualToNode(*root2)); ASSERT_FALSE(root1->isLayoutTreeEqualToNode(*root2));
YGNodeFreeRecursive(root1); YGNodeFreeRecursive(root1);

View File

@@ -68,17 +68,12 @@ struct YGCachedMeasurement {
if (!std::isnan(computedWidth) || !std::isnan(measurement.computedWidth)) { if (!std::isnan(computedWidth) || !std::isnan(measurement.computedWidth)) {
isEqual = isEqual && computedWidth == measurement.computedWidth; isEqual = isEqual && computedWidth == measurement.computedWidth;
} }
if (!std::isnan( if (!std::isnan(computedHeight) ||
computedHeight || !std::isnan(measurement.computedHeight))) { !std::isnan(measurement.computedHeight)) {
isEqual = isEqual && computedHeight == measurement.computedHeight; isEqual = isEqual && computedHeight == measurement.computedHeight;
} }
return availableWidth == measurement.availableWidth && return isEqual;
availableHeight == measurement.availableHeight &&
widthMeasureMode == measurement.widthMeasureMode &&
heightMeasureMode == measurement.heightMeasureMode &&
computedWidth == measurement.computedWidth &&
computedHeight == measurement.computedHeight;
} }
}; };

View File

@@ -251,6 +251,16 @@ YGNodeRef YGNodeClone(YGNodeRef oldNode) {
return node; 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) { static YGNodeRef YGNodeDeepClone(YGNodeRef oldNode) {
YGNodeRef node = YGNodeClone(oldNode); YGNodeRef node = YGNodeClone(oldNode);
YGVector vec = YGVector(); YGVector vec = YGVector();
@@ -263,12 +273,12 @@ static YGNodeRef YGNodeDeepClone(YGNodeRef oldNode) {
} }
node->setChildren(vec); node->setChildren(vec);
if (oldNode->getNextChild() != nullptr) { if (oldNode->getConfig() != nullptr) {
node->setNextChild(YGNodeDeepClone(oldNode->getNextChild())); node->setConfig(YGConfigClone(*(oldNode->getConfig())));
} }
if (node->getConfig() != nullptr) { if (oldNode->getNextChild() != nullptr) {
node->setConfig(new YGConfig(*node->getConfig())); node->setNextChild(YGNodeDeepClone(oldNode->getNextChild()));
} }
return node; return node;
@@ -291,6 +301,17 @@ void YGNodeFree(const YGNodeRef node) {
gNodeInstanceCount--; 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) { void YGNodeFreeRecursive(const YGNodeRef root) {
while (YGNodeGetChildCount(root) > 0) { while (YGNodeGetChildCount(root) > 0) {
const YGNodeRef child = YGNodeGetChild(root, 0); const YGNodeRef child = YGNodeGetChild(root, 0);
@@ -3642,6 +3663,7 @@ void YGNodeCalculateLayout(const YGNodeRef node,
YGPrintOptionsStyle)); YGPrintOptionsStyle));
} }
} }
YGConfigFreeRecursive(originalNode);
YGNodeFreeRecursive(originalNode); YGNodeFreeRecursive(originalNode);
} }
} }