From a17e3cf893dc85173712d9026f635f8bbdbcd07e Mon Sep 17 00:00:00 2001 From: Andrei Shikov Date: Mon, 12 Apr 2021 13:16:01 -0700 Subject: [PATCH] Avoid mounting tree on mode change if it wasn't commited Summary: Changelog: [Internal] Calls to `surfaceHandler.start()` and `setDisplayMode(DisplayMode::Visible)` in quick succession on different threads can cause a race condition between mount and commit operations. The mountingCoordinator will try to mount an empty revision without any commits causing it to fail with: ``` TransactionTelemetry.cpp:108: function getCommitStartTime: assertion failed (commitStartTime_ != kTelemetryUndefinedTimePoint) ``` which is called from [Binding.cpp](https://www.internalfb.com/intern/diffusion/FBS/browse/master/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp?lines=791-791&blame=1). This change avoids this initial commit by verifying we had at least 1 revision commited before mounting it. Reviewed By: sammy-SC Differential Revision: D27430174 fbshipit-source-id: d208d55f02cd218a316d6fea62d0106c2bcb4be8 --- ReactCommon/react/renderer/mounting/ShadowTree.cpp | 6 ++++-- ReactCommon/react/renderer/mounting/ShadowTree.h | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ReactCommon/react/renderer/mounting/ShadowTree.cpp b/ReactCommon/react/renderer/mounting/ShadowTree.cpp index 6006539eb0a..7350dbc9d6b 100644 --- a/ReactCommon/react/renderer/mounting/ShadowTree.cpp +++ b/ReactCommon/react/renderer/mounting/ShadowTree.cpp @@ -248,7 +248,7 @@ ShadowTree::ShadowTree( family)); currentRevision_ = ShadowTreeRevision{ - rootShadowNode, ShadowTreeRevision::Number{0}, TransactionTelemetry{}}; + rootShadowNode, INITIAL_REVISION, TransactionTelemetry{}}; mountingCoordinator_ = std::make_shared(currentRevision_); @@ -275,7 +275,9 @@ void ShadowTree::setCommitMode(CommitMode commitMode) const { revision = currentRevision_; } - if (commitMode == CommitMode::Normal) { + // initial revision never contains any commits so mounting it here is + // incorrect + if (commitMode == CommitMode::Normal && revision.number != INITIAL_REVISION) { mount(revision); } } diff --git a/ReactCommon/react/renderer/mounting/ShadowTree.h b/ReactCommon/react/renderer/mounting/ShadowTree.h index fd90b735497..2a8f1d1fadd 100644 --- a/ReactCommon/react/renderer/mounting/ShadowTree.h +++ b/ReactCommon/react/renderer/mounting/ShadowTree.h @@ -124,6 +124,8 @@ class ShadowTree final { MountingCoordinator::Shared getMountingCoordinator() const; private: + constexpr static ShadowTreeRevision::Number INITIAL_REVISION{0}; + void mount(ShadowTreeRevision const &revision) const; void emitLayoutEvents(