From 71bb19827b0568ead70dc5c591d07ec976445449 Mon Sep 17 00:00:00 2001
From: Joshua Gross <joshuagross@fb.com>
Date: Mon, 12 Oct 2020 22:50:24 -0700
Subject: [PATCH] NativeAnimatedModule: fix crash that can occur when animated
 node setup races with NativeAnimatedModule setup

Summary:
When switching between non-Fabric and Fabric screens, I believe that `initializeEventListenerForUIManagerType` is not always being called on the NativeAnimatedNodesManager if
`NativeAnimatedModule.initializeLifecycleEventListenersForViewTag` is being called before the NativeAnimatedNodesManager ivar has been set. This should occur very infrequently, but logs
indicate that it /does/ happen in some marginal cases. Protecting against these cases should be trivial, by using the `getNodesManager` method which is responsible for returning a nodes manager or creating a new one.

The existing uses of the `NativeAnimatedNodesManager` ivar also occur on different threads and we were not protecting against this, so I'm changing it to an atomic. It's very likely that
the inconsistency issues in the past were caused not by ordering errors, but thread races.

Changelog: [Internal]

Reviewed By: fkgozali

Differential Revision: D24267118

fbshipit-source-id: 68abbff7ef3d0b2ecc9aa9977165663ad9447ab8
---
 .../react/animated/NativeAnimatedModule.java  | 23 +++++++++++++------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java
index 8a05e0627ed..2f2880080e6 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java
@@ -17,6 +17,7 @@ import com.facebook.react.bridge.Arguments;
 import com.facebook.react.bridge.Callback;
 import com.facebook.react.bridge.LifecycleEventListener;
 import com.facebook.react.bridge.ReactApplicationContext;
+import com.facebook.react.bridge.ReactSoftException;
 import com.facebook.react.bridge.ReadableMap;
 import com.facebook.react.bridge.UIManager;
 import com.facebook.react.bridge.UIManagerListener;
@@ -34,6 +35,7 @@ import com.facebook.react.uimanager.common.UIManagerType;
 import com.facebook.react.uimanager.common.ViewUtil;
 import java.util.Queue;
 import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * Module that exposes interface for creating and managing animated nodes on the "native" side.
@@ -111,7 +113,7 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
   private final ConcurrentLinkedQueue<UIThreadOperation> mPreOperations =
       new ConcurrentLinkedQueue<>();
 
-  private @Nullable NativeAnimatedNodesManager mNodesManager;
+  private final AtomicReference<NativeAnimatedNodesManager> mNodesManager = new AtomicReference<>();
 
   private boolean mBatchingControlledByJS = false; // TODO T71377544: delete
   private volatile long mCurrentFrameNumber; // TODO T71377544: delete
@@ -223,6 +225,7 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
     executeAllOperations(mOperations, batchNumber);
   }
 
+  @UiThread
   private void executeAllOperations(Queue<UIThreadOperation> operationQueue, long maxBatchNumber) {
     NativeAnimatedNodesManager nodesManager = getNodesManager();
     while (true) {
@@ -315,15 +318,15 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
    */
   @Nullable
   private NativeAnimatedNodesManager getNodesManager() {
-    if (mNodesManager == null) {
+    if (mNodesManager.get() == null) {
       ReactApplicationContext reactApplicationContext = getReactApplicationContextIfActiveOrWarn();
 
       if (reactApplicationContext != null) {
-        mNodesManager = new NativeAnimatedNodesManager(reactApplicationContext);
+        mNodesManager.compareAndSet(null, new NativeAnimatedNodesManager(reactApplicationContext));
       }
     }
 
-    return mNodesManager;
+    return mNodesManager.get();
   }
 
   private void clearFrameCallback() {
@@ -340,7 +343,7 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
 
   @VisibleForTesting
   public void setNodesManager(NativeAnimatedNodesManager nodesManager) {
-    mNodesManager = nodesManager;
+    mNodesManager.set(nodesManager);
   }
 
   /**
@@ -358,8 +361,14 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
       mNumNonFabricAnimations++;
     }
 
-    if (mNodesManager != null) {
-      mNodesManager.initializeEventListenerForUIManagerType(mUIManagerType);
+    NativeAnimatedNodesManager nodesManager = getNodesManager();
+    if (nodesManager != null) {
+      nodesManager.initializeEventListenerForUIManagerType(mUIManagerType);
+    } else {
+      ReactSoftException.logSoftException(
+          NAME,
+          new RuntimeException(
+              "initializeLifecycleEventListenersForViewTag could not get NativeAnimatedNodesManager"));
     }
 
     // Subscribe to UIManager (Fabric or non-Fabric) lifecycle events if we haven't yet