From 1cef72af047bd2e56d774858093f1ee6269c2d27 Mon Sep 17 00:00:00 2001
From: Ramanpreet Nara <ramanpreet@fb.com>
Date: Tue, 28 Apr 2020 12:14:48 -0700
Subject: [PATCH] Part 1: Make CatalystInstanceImpl.getNativeModule Nullable

Summary:
## Description
When the TurboModule and the NativeModule systems are alive at the same time, after RN cleanup, if a TurboModule is required, we return `null` from TurboModuleManager. This causes `CatalystInstanceImpl.getNativeModule` to do a lookup on NativeModule registry, which throws an `AssertionError`, because the TurboModule isn't found. Instead of throwing an `AssertionError` from `CatalystInstanceImpl.getNativeModule`, this diff instead has that method return `null` in this particular case.

## Rationale
This should eliminate the crashes we're seeing in T46487253.

## Future action
In the future, we should guard `CatalystInstanceImpl.getNativeModule` with an `if (mDestroyed) return null;` statement. This'll ensure that if NativeModules are required after React Native has started cleanup, they'll be returned as `null`. Right now, we either return the destroyed NativeModule object, or create/initialize the NativeMoulde and return it, which is wrong.

Changelog:
[Android][Changed] - Make CatalystInstance.getNativeModule nullable

Reviewed By: JoshuaGross

Differential Revision: D21272029

fbshipit-source-id: 099ad9ab9fa2146299df4cf7f86ae7a8e526bb15
---
 .../java/com/facebook/react/bridge/CatalystInstance.java    | 2 ++
 .../com/facebook/react/bridge/CatalystInstanceImpl.java     | 6 +++++-
 .../main/java/com/facebook/react/bridge/ReactContext.java   | 1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java
index 09f0f6db3f1..f95b974e30d 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java
@@ -62,8 +62,10 @@ public interface CatalystInstance
 
   <T extends NativeModule> boolean hasNativeModule(Class<T> nativeModuleInterface);
 
+  @Nullable
   <T extends NativeModule> T getNativeModule(Class<T> nativeModuleInterface);
 
+  @Nullable
   NativeModule getNativeModule(String moduleName);
 
   JSIModule getJSIModule(JSIModuleType moduleType);
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 a6ac1405b05..6e481bbd2cd 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java
@@ -562,6 +562,7 @@ public class CatalystInstanceImpl implements CatalystInstance {
   }
 
   @Override
+  @Nullable
   public <T extends NativeModule> T getNativeModule(Class<T> nativeModuleInterface) {
     return (T) getNativeModule(getNameFromAnnotation(nativeModuleInterface));
   }
@@ -577,6 +578,7 @@ public class CatalystInstanceImpl implements CatalystInstance {
   }
 
   @Override
+  @Nullable
   public NativeModule getNativeModule(String moduleName) {
     if (getTurboModuleRegistry() != null) {
       TurboModule turboModule = getTurboModuleRegistry().getModule(moduleName);
@@ -593,7 +595,9 @@ public class CatalystInstanceImpl implements CatalystInstance {
       }
     }
 
-    return mNativeModuleRegistry.getModule(moduleName);
+    return mNativeModuleRegistry.hasModule(moduleName)
+        ? mNativeModuleRegistry.getModule(moduleName)
+        : null;
   }
 
   private <T extends NativeModule> String getNameFromAnnotation(Class<T> nativeModuleInterface) {
diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java
index fe13dd68981..126237e74e8 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java
@@ -158,6 +158,7 @@ public class ReactContext extends ContextWrapper {
   }
 
   /** @return the instance of the specified module interface associated with this ReactContext. */
+  @Nullable
   public <T extends NativeModule> T getNativeModule(Class<T> nativeModuleInterface) {
     if (mCatalystInstance == null) {
       raiseCatalystInstanceMissingException();