From bf83600996298f21698fc1fd0b90ab597d411d92 Mon Sep 17 00:00:00 2001 From: Eric Samelson Date: Tue, 1 May 2018 13:57:58 -0700 Subject: [PATCH] Remove race condition in initial layout of ReactRootView Summary: There is a race condition stemming from `ReactRootView.onCreate` that occasionally causes the initial root layout calculation to never occur. In this method currently, `updateRootLayoutSpecs(...)` is called before `enableLayoutCalculation()`, meaning that it's possible for the native modules thread to reach `UIImplementation.updateViewHierarchy` before layout calculation has been enabled (i.e. before the rootViewTag is added to `UIImplementation.mMeasuredRootNodes` on the UI thread). When this occurs, `UIImplementation.applyUpdatesRecursive` is never called. This causes the app to hang on the splash screen indefinitely, the JS never gets past the first call to `render`, and no layout events are ever sent to the JS. This PR reverses the order of those two method calls to ensure that the rootViewTag is always added to `mMeasuredRootNodes` before `UIImplementation.updateViewHierarchy` expects it. We inspected all of the code paths of both `enableLayoutCalculation()` and `updateRootLayoutSpecs()` to ensure that in this new order, the rootViewTag will *always* be added to `mMeasuredRootNodes` before the call to `updateViewHierarchy` is dispatched, and that no other side effects would be introduced. Tested with an app that reliably had this issue (hanging splash screen) on 1 out of every ~10 launches. Logged values to ensure that empty `mMeasuredRootNodes` was the difference between failed and successful launches. After this change, I launched the same app 50+ times without any issues. [ANDROID][BUGFIX][ReactRootView] - remove race condition in initial layout of ReactRootView Closes https://github.com/facebook/react-native/pull/19038 Differential Revision: D7834270 Pulled By: hramos fbshipit-source-id: 6040da900f807dcacbc86ae2c36b4ca004f80178 --- .../src/main/java/com/facebook/react/ReactRootView.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java b/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java index baa2a6e30..4c4a79529 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java @@ -147,12 +147,12 @@ public class ReactRootView extends SizeMonitoringFrameLayout // Check if we were waiting for onMeasure to attach the root view. if (mReactInstanceManager != null && !mIsAttachedToInstance) { attachToReactInstanceManager(); + enableLayoutCalculation(); } else { + enableLayoutCalculation(); updateRootLayoutSpecs(mWidthMeasureSpec, mHeightMeasureSpec); } - enableLayoutCalculation(); - } finally { Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE); }