Skip to content

Commit a9a0000

Browse files
j-piaseckimeta-codesync[bot]
authored andcommitted
Prevent double calls to setMounted when merging a JS revision (#56424)
Summary: Pull Request resolved: #56424 Changelog: [GENERAL][FIXED] Fixed double `setMounted` flag calls with fabric branching enabled This change removes the commit source condition from the call site in `ShadowTree::tryCommit` and moves the logic into updateMountedFlag itself: - Mounted flags are skipped for ReactRevisionMerge commits because they were already set during the React branch commit. Setting them again would double increment the EventEmitter's additive enable counter. - Runtime shadow node references updates condition didn't change, only moved to a single variable. - When neither operation is needed, the function returns early to avoid an unnecessary tree traversal under the DispatchMutex. Reviewed By: lenaic, rubennorte Differential Revision: D100125886 fbshipit-source-id: aceb1ae3609957048b78f615233ed00ce2e04038
1 parent 179e0cd commit a9a0000

File tree

2 files changed

+39
-21
lines changed

2 files changed

+39
-21
lines changed

packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ CommitStatus ShadowTree::tryCommit(
422422

423423
auto newRevisionNumber = currentRevision_.number + 1;
424424

425-
if (!isReactBranch) {
425+
{
426426
std::scoped_lock dispatchLock(EventEmitter::DispatchMutex());
427427
updateMountedFlag(
428428
currentRevision_.rootShadowNode->getChildren(),

packages/react-native/ReactCommon/react/renderer/mounting/updateMountedFlag.cpp

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,25 @@ void updateMountedFlag(
2929
return;
3030
}
3131

32+
// Mounted flags shouldn't be updated during the React revision merge
33+
// because they were already set during the React branch commit. Setting them
34+
// again would double-increment the EventEmitter's additive enable counter.
35+
bool shouldUpdateMountedFlag =
36+
commitSource != ShadowTreeCommitSource::ReactRevisionMerge;
37+
38+
// Runtime shadow node references are updated during the React revision
39+
// commits so that JS can access layout data from the merged tree.
40+
bool shouldUpdateRuntimeReference =
41+
(commitSource == ShadowTreeCommitSource::React &&
42+
ReactNativeFeatureFlags::updateRuntimeShadowNodeReferencesOnCommit()) ||
43+
(ReactNativeFeatureFlags::
44+
updateRuntimeShadowNodeReferencesOnCommitThread() &&
45+
ShadowNode::getUseRuntimeShadowNodeReferenceUpdateOnThread());
46+
47+
if (!shouldUpdateMountedFlag && !shouldUpdateRuntimeReference) {
48+
return;
49+
}
50+
3251
size_t index = 0;
3352

3453
// Stage 1: Mount and unmount "updated" children.
@@ -47,15 +66,12 @@ void updateMountedFlag(
4766
break;
4867
}
4968

50-
newChild->setMounted(true);
51-
oldChild->setMounted(false);
69+
if (shouldUpdateMountedFlag) {
70+
newChild->setMounted(true);
71+
oldChild->setMounted(false);
72+
}
5273

53-
if ((commitSource == ShadowTreeCommitSource::React &&
54-
ReactNativeFeatureFlags::
55-
updateRuntimeShadowNodeReferencesOnCommit()) ||
56-
(ReactNativeFeatureFlags::
57-
updateRuntimeShadowNodeReferencesOnCommitThread() &&
58-
ShadowNode::getUseRuntimeShadowNodeReferenceUpdateOnThread())) {
74+
if (shouldUpdateRuntimeReference) {
5975
newChild->updateRuntimeShadowNodeReference(newChild);
6076
}
6177

@@ -68,25 +84,27 @@ void updateMountedFlag(
6884
// State 2: Mount new children.
6985
for (index = lastIndexAfterFirstStage; index < newChildren.size(); index++) {
7086
const auto& newChild = newChildren[index];
71-
newChild->setMounted(true);
72-
73-
if ((commitSource == ShadowTreeCommitSource::React &&
74-
ReactNativeFeatureFlags::
75-
updateRuntimeShadowNodeReferencesOnCommit()) ||
76-
(ReactNativeFeatureFlags::
77-
updateRuntimeShadowNodeReferencesOnCommitThread() &&
78-
ShadowNode::getUseRuntimeShadowNodeReferenceUpdateOnThread())) {
87+
88+
if (shouldUpdateMountedFlag) {
89+
newChild->setMounted(true);
90+
}
91+
92+
if (shouldUpdateRuntimeReference) {
7993
newChild->updateRuntimeShadowNodeReference(newChild);
8094
}
8195

8296
updateMountedFlag({}, newChild->getChildren(), commitSource);
8397
}
8498

8599
// State 3: Unmount old children.
86-
for (index = lastIndexAfterFirstStage; index < oldChildren.size(); index++) {
87-
const auto& oldChild = oldChildren[index];
88-
oldChild->setMounted(false);
89-
updateMountedFlag(oldChild->getChildren(), {}, commitSource);
100+
if (shouldUpdateMountedFlag) {
101+
for (index = lastIndexAfterFirstStage; index < oldChildren.size();
102+
index++) {
103+
const auto& oldChild = oldChildren[index];
104+
oldChild->setMounted(false);
105+
106+
updateMountedFlag(oldChild->getChildren(), {}, commitSource);
107+
}
90108
}
91109
}
92110
} // namespace facebook::react

0 commit comments

Comments
 (0)