From 56ad1bd38ac772f2a2335de5ac798eb8c3526f1e Mon Sep 17 00:00:00 2001
From: Ramanpreet Nara <ramanpreet@fb.com>
Date: Fri, 1 Nov 2019 19:22:22 -0700
Subject: [PATCH] Assert TurboModuleRegistry is not null

Summary:
Looking at the crash reports from T46487253:
1. This crash happens only with TurboModule-compatible NativeModules.
2. Users who experience this crash are in the TurboModules test group.

Therefore, the crash happens while trying to load TurboModules.

The stack trace of the crash includes [this lookup via the NativeModule system](https://fburl.com/diffusion/vxj9goz5). When TurboModules are enabled, we can only start executing this line if one of two things are true:
1. The TurboModuleRegistry is null in CatalystInstanceImpl.
2. The TurboModuleRegistry isn't null but the NativeModule returned by the TurboModuleRegistry is null.

We can protect against 1 by asserting that when `ReactFeatureFlags.useTurboModules` is `true`, `mTurboModuleRegistry` is not null. Once this check lands, unless there's a race with setting `ReactFeatureFlags.useTurboModules`, we should be able to rule out 1.

Changelog:
[Added][Android] - Assert TurboModuleRegistry isn't null before using it in CatalystInstanceImpl

Reviewed By: PeteTheHeat

Differential Revision: D18211935

fbshipit-source-id: de88c033425c474ef80b73386b7182b1d3bb382f
---
 .../react/bridge/CatalystInstanceImpl.java    | 20 ++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java
index 46ad79944ca..7155c45f9d5 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java
@@ -557,7 +557,7 @@ public class CatalystInstanceImpl implements CatalystInstance {
   @Override
   public <T extends NativeModule> boolean hasNativeModule(Class<T> nativeModuleInterface) {
     String moduleName = getNameFromAnnotation(nativeModuleInterface);
-    return mTurboModuleRegistry != null && mTurboModuleRegistry.hasModule(moduleName)
+    return getTurboModuleRegistry() != null && getTurboModuleRegistry().hasModule(moduleName)
         ? true
         : mNativeModuleRegistry.hasModule(moduleName);
   }
@@ -567,10 +567,20 @@ public class CatalystInstanceImpl implements CatalystInstance {
     return (T) getNativeModule(getNameFromAnnotation(nativeModuleInterface));
   }
 
+  private TurboModuleRegistry getTurboModuleRegistry() {
+    if (ReactFeatureFlags.useTurboModules) {
+      return Assertions.assertNotNull(
+          mTurboModuleRegistry,
+          "TurboModules are enabled, but mTurboModuleRegistry hasn't been set.");
+    }
+
+    return null;
+  }
+
   @Override
   public NativeModule getNativeModule(String moduleName) {
-    if (mTurboModuleRegistry != null) {
-      TurboModule turboModule = mTurboModuleRegistry.getModule(moduleName);
+    if (getTurboModuleRegistry() != null) {
+      TurboModule turboModule = getTurboModuleRegistry().getModule(moduleName);
 
       if (turboModule != null) {
         return (NativeModule) turboModule;
@@ -595,8 +605,8 @@ public class CatalystInstanceImpl implements CatalystInstance {
     Collection<NativeModule> nativeModules = new ArrayList<>();
     nativeModules.addAll(mNativeModuleRegistry.getAllModules());
 
-    if (mTurboModuleRegistry != null) {
-      for (TurboModule turboModule : mTurboModuleRegistry.getModules()) {
+    if (getTurboModuleRegistry() != null) {
+      for (TurboModule turboModule : getTurboModuleRegistry().getModules()) {
         nativeModules.add((NativeModule) turboModule);
       }
     }