assert that ShadowNodeRegistry is only accessed from one thread

Reviewed By: astreet

Differential Revision: D3461859

fbshipit-source-id: 790e831d2ca239110e00a5723be40e870ceab020

Author: cjhopman

ReactAndroid/src/main/java/com/facebook/react/common/SingleThreadAsserter.java

@@ -0,0 +1,29 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+package com.facebook.react.common;
+
+import javax.annotation.Nullable;
+
+import com.facebook.infer.annotation.Assertions;
+
+/**
+ * Simple class for asserting that operations only run on a single thread.
+ */
+public class SingleThreadAsserter {
+  private @Nullable Thread mThread = null;
+
+  public void assertNow() {
+    Thread current = Thread.currentThread();
+    if (mThread == null) {
+      mThread = current;
+    }
+    Assertions.assertCondition(mThread == current);
+  }
+}

ReactAndroid/src/main/java/com/facebook/react/uimanager/ShadowNodeRegistry.java

@@ -12,6 +12,8 @@
 import android.util.SparseArray;
 import android.util.SparseBooleanArray;
 
+import com.facebook.react.common.SingleThreadAsserter;
+
 /**
  * Simple container class to keep track of {@link ReactShadowNode}s associated with a particular
  * UIManagerModule instance.
@@ -20,19 +22,25 @@
 
   private final SparseArray<ReactShadowNode> mTagsToCSSNodes;
   private final SparseBooleanArray mRootTags;
+  private final SingleThreadAsserter mThreadAsserter;
 
   public ShadowNodeRegistry() {
     mTagsToCSSNodes = new SparseArray<>();
     mRootTags = new SparseBooleanArray();
+    mThreadAsserter = new SingleThreadAsserter();
   }
 
   public void addRootNode(ReactShadowNode node) {
+    // TODO(6242243): This should be asserted... but UIManagerModule is
+    // thread-unsafe and calls this on the wrong thread.
+    //mThreadAsserter.assertNow();
     int tag = node.getReactTag();
     mTagsToCSSNodes.put(tag, node);
     mRootTags.put(tag, true);
   }
 
   public void removeRootNode(int tag) {
+    mThreadAsserter.assertNow();
     if (!mRootTags.get(tag)) {
       throw new IllegalViewOperationException(
           "View with tag " + tag + " is not registered as a root view");
@@ -43,10 +51,12 @@ public void removeRootNode(int tag) {
   }
 
   public void addNode(ReactShadowNode node) {
+    mThreadAsserter.assertNow();
     mTagsToCSSNodes.put(node.getReactTag(), node);
   }
 
   public void removeNode(int tag) {
+    mThreadAsserter.assertNow();
     if (mRootTags.get(tag)) {
       throw new IllegalViewOperationException(
           "Trying to remove root node " + tag + " without using removeRootNode!");
@@ -55,18 +65,22 @@ public void removeNode(int tag) {
   }
 
   public ReactShadowNode getNode(int tag) {
+    mThreadAsserter.assertNow();
     return mTagsToCSSNodes.get(tag);
   }
 
   public boolean isRootNode(int tag) {
+    mThreadAsserter.assertNow();
     return mRootTags.get(tag);
   }
 
   public int getRootNodeCount() {
+    mThreadAsserter.assertNow();
     return mRootTags.size();
   }
 
   public int getRootTag(int index) {
+    mThreadAsserter.assertNow();
     return mRootTags.keyAt(index);
   }
 }

ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java

@@ -99,6 +99,7 @@ public void registerRootView(
     rootCSSNode.setThemedContext(context);
     rootCSSNode.setStyleWidth(width);
     rootCSSNode.setStyleHeight(height);
+
     mShadowNodeRegistry.addRootNode(rootCSSNode);
 
     // register it within NativeViewHierarchyManager

ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java

@@ -141,9 +141,7 @@ private static Map<String, Object> createConstants(List<ViewManager> viewManager
    * CatalystApplicationFragment as an example.
    *
    * TODO(6242243): Make addMeasuredRootView thread safe
-   * NB: this method is horribly not-thread-safe, the only reason it works right now is because
-   * it's called exactly once and is called before any JS calls are made. As soon as that fact no
-   * longer holds, this method will need to be fixed.
+   * NB: this method is horribly not-thread-safe.
    */
   public int addMeasuredRootView(final SizeMonitoringFrameLayout rootView) {
     final int tag = mNextRootViewTag;