From fb07dcff40a288658e24e3190e6c1929f25ec183 Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Mon, 28 Oct 2019 12:38:05 -0700 Subject: [PATCH] Fix data race with gCurrentGenerationCount Summary: Yoga layout can be invoked on multiple threads, and gCurrentGenerationCount is a shared global without synchronization. Changelog: [General] [Fixed] - Fix an internal thread safety issue in Yoga Reviewed By: SidharthGuglani Differential Revision: D18092734 fbshipit-source-id: 85753d139549b4e5507f97a56d589fb6854557fa --- yoga/Yoga.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/yoga/Yoga.cpp b/yoga/Yoga.cpp index d3ddf7ad..e0a06b43 100644 --- a/yoga/Yoga.cpp +++ b/yoga/Yoga.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include "Utils.h" #include "YGNode.h" @@ -927,7 +928,7 @@ bool YGNodeLayoutGetDidLegacyStretchFlagAffectLayout(const YGNodeRef node) { return node->getLayout().doesLegacyStretchFlagAffectsLayout(); } -uint32_t gCurrentGenerationCount = 0; +std::atomic gCurrentGenerationCount(0); bool YGLayoutNodeInternal( const YGNodeRef node, @@ -1846,7 +1847,7 @@ static float YGNodeComputeFlexBasisForChildren( const uint32_t generationCount) { float totalOuterFlexBasis = 0.0f; YGNodeRef singleFlexChild = nullptr; - const YGVector &children = node->getChildren(); + const YGVector& children = node->getChildren(); YGMeasureMode measureModeMainDim = YGFlexDirectionIsRow(mainAxis) ? widthMeasureMode : heightMeasureMode; // If there is only one child with flexGrow + flexShrink it means we can set @@ -4079,7 +4080,7 @@ void YGNodeCalculateLayoutWithContext( // Increment the generation count. This will force the recursive routine to // visit all dirty nodes at least once. Subsequent visits will be skipped if // the input parameters don't change. - gCurrentGenerationCount++; + gCurrentGenerationCount.fetch_add(1, std::memory_order_relaxed); node->resolveDimension(); float width = YGUndefined; YGMeasureMode widthMeasureMode = YGMeasureModeUndefined; @@ -4136,7 +4137,7 @@ void YGNodeCalculateLayoutWithContext( markerData, layoutContext, 0, // tree root - gCurrentGenerationCount)) { + gCurrentGenerationCount.load(std::memory_order_relaxed))) { node->setPosition( node->getLayout().direction(), ownerWidth, ownerHeight, ownerWidth); YGRoundToPixelGrid(node, node->getConfig()->pointScaleFactor, 0.0f, 0.0f); @@ -4167,7 +4168,7 @@ void YGNodeCalculateLayoutWithContext( nodeWithoutLegacyFlag->resolveDimension(); // Recursively mark nodes as dirty nodeWithoutLegacyFlag->markDirtyAndPropogateDownwards(); - gCurrentGenerationCount++; + gCurrentGenerationCount.fetch_add(1, std::memory_order_relaxed); // Rerun the layout, and calculate the diff unsetUseLegacyFlagRecursively(nodeWithoutLegacyFlag); LayoutData layoutMarkerData = {}; @@ -4186,7 +4187,7 @@ void YGNodeCalculateLayoutWithContext( layoutMarkerData, layoutContext, 0, // tree root - gCurrentGenerationCount)) { + gCurrentGenerationCount.load(std::memory_order_relaxed))) { nodeWithoutLegacyFlag->setPosition( nodeWithoutLegacyFlag->getLayout().direction(), ownerWidth,