Skip non-owned subtrees in roundLayoutResultsToPixelGrid - #2015
Closed
zeyap wants to merge 1 commit into
Closed
Conversation
Summary: > [!NOTE] > **This PR description is AI-generated.** Fabric runs layout on candidate trees before taking the commit mutex, and concurrent commits share every unchanged subtree, so Yoga's pixel-grid rounding pass could write rounded positions and dimensions into the same shared `yoga::Node` from two threads at once. The layout pass itself never mutates nodes it does not own (`Node::cloneChildrenIfNeeded`), but `roundLayoutResultsToPixelGrid` recursed across the ownership frontier. I made the rounding recursion skip children whose owner is not the current node, mirroring the existing owner checks in `Node::cloneChildrenIfNeeded` and `YGNodeFreeRecursive`. The skipped writes had no reader: `YogaLayoutableShadowNode::layout` copies metrics only from children with `hasNewLayout` (asserting they are owned), `hasNewLayout` is set only on nodes the pass laid out, and the shadow nodes past the frontier are sealed, so their `LayoutMetrics` could not change in that commit anyway. ## Changelog: [GENERAL] [FIXED] - Fix data race between concurrent Fabric commits in Yoga's pixel-grid rounding pass X-link: react/react-native#58144 Reviewed By: christophpurrer Differential Revision: D117688567 Pulled By: zeyap
|
@zeyap has exported this pull request. If you are a Meta employee, you can view the originating Diff in D117688567. |
meta-codesync Bot
pushed a commit
to react/react-native
that referenced
this pull request
Aug 28, 2026
Summary: X-link: react/yoga#2015 > [!NOTE] > **This PR description is AI-generated.** Fabric runs layout on candidate trees before taking the commit mutex, and concurrent commits share every unchanged subtree, so Yoga's pixel-grid rounding pass could write rounded positions and dimensions into the same shared `yoga::Node` from two threads at once. The layout pass itself never mutates nodes it does not own (`Node::cloneChildrenIfNeeded`), but `roundLayoutResultsToPixelGrid` recursed across the ownership frontier. I made the rounding recursion skip children whose owner is not the current node, mirroring the existing owner checks in `Node::cloneChildrenIfNeeded` and `YGNodeFreeRecursive`. The skipped writes had no reader: `YogaLayoutableShadowNode::layout` copies metrics only from children with `hasNewLayout` (asserting they are owned), `hasNewLayout` is set only on nodes the pass laid out, and the shadow nodes past the frontier are sealed, so their `LayoutMetrics` could not change in that commit anyway. ## Changelog: [GENERAL] [FIXED] - Fix data race between concurrent Fabric commits in Yoga's pixel-grid rounding pass Pull Request resolved: #58144 Test Plan: ThreadSanitizer reports of the race, from react-native-reanimated's sanitizer nightly (React commit on the JS thread racing a Reanimated commit on the main thread over a shared `ParagraphShadowNode`): https://github.com/software-mansion/react-native-reanimated/actions/runs/32811464205/job/97691453444 Reviewed By: christophpurrer Differential Revision: D117688567 Pulled By: zeyap fbshipit-source-id: 3f0662aee2ce865b4a13edffd46217f349a4285d
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Note
This PR description is AI-generated.
Fabric runs layout on candidate trees before taking the commit mutex, and concurrent commits share every unchanged subtree, so Yoga's pixel-grid rounding pass could write rounded positions and dimensions into the same shared
yoga::Nodefrom two threads at once. The layout pass itself never mutates nodes it does not own (Node::cloneChildrenIfNeeded), butroundLayoutResultsToPixelGridrecursed across the ownership frontier. I made the rounding recursion skip children whose owner is not the current node, mirroring the existing owner checks inNode::cloneChildrenIfNeededandYGNodeFreeRecursive. The skipped writes had no reader:YogaLayoutableShadowNode::layoutcopies metrics only from children withhasNewLayout(asserting they are owned),hasNewLayoutis set only on nodes the pass laid out, and the shadow nodes past the frontier are sealed, so theirLayoutMetricscould not change in that commit anyway.Changelog:
[GENERAL] [FIXED] - Fix data race between concurrent Fabric commits in Yoga's pixel-grid rounding pass
X-link: react/react-native#58144
Reviewed By: christophpurrer
Differential Revision: D117688567
Pulled By: zeyap