From b55146f7763e4e9d6cc2fd54dde62d4e4289721f Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Fri, 25 Oct 2019 16:14:21 -0700 Subject: [PATCH] Check that CatalystInstance is active for non-NativeModule callsites of ReactContext.getJSModule Summary: In previous diffs I migrated many (all?) NativeModules in FB and open-source to check for `hasActiveCatalystInstance` before calling `getJSModule`. We log SoftExceptions in those cases to find more potential race condition and lifecycle bugs without crashing. In this diff, I migrate all the non-NativeModule callsites that I could find. Previous diffs: see D18032458, D18035359, D18032788, D18092136, D18092137, D18112989, D18134400 Changelog: [Internal] Reviewed By: mdvacca, mmmulani Differential Revision: D18134694 fbshipit-source-id: 4729abfb84280b634463b1cd9b4dd808f310b6e7 --- .../com/facebook/react/ReactInstanceManager.java | 9 ++++++++- .../react/jstasks/HeadlessJsTaskContext.java | 13 ++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index b40b80d00d1..24d6fcc7c32 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -67,6 +67,8 @@ import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.ReactMarker; import com.facebook.react.bridge.ReactMarkerConstants; +import com.facebook.react.bridge.ReactNoCrashSoftException; +import com.facebook.react.bridge.ReactSoftException; import com.facebook.react.bridge.UIManager; import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.bridge.WritableNativeMap; @@ -477,10 +479,15 @@ public class ReactInstanceManager { private void toggleElementInspector() { ReactContext currentContext = getCurrentReactContext(); - if (currentContext != null) { + if (currentContext != null && currentContext.hasActiveCatalystInstance()) { currentContext .getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class) .emit("toggleElementInspector", null); + } else { + ReactSoftException.logSoftException( + TAG, + new ReactNoCrashSoftException( + "Cannot toggleElementInspector, CatalystInstance not available")); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/jstasks/HeadlessJsTaskContext.java b/ReactAndroid/src/main/java/com/facebook/react/jstasks/HeadlessJsTaskContext.java index 2bdee667834..b6f60da4e17 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/jstasks/HeadlessJsTaskContext.java +++ b/ReactAndroid/src/main/java/com/facebook/react/jstasks/HeadlessJsTaskContext.java @@ -11,6 +11,7 @@ import android.os.Handler; import android.util.SparseArray; import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.ReactContext; +import com.facebook.react.bridge.ReactSoftException; import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.common.LifecycleState; import com.facebook.react.modules.appregistry.AppRegistry; @@ -106,9 +107,15 @@ public class HeadlessJsTaskContext { } mActiveTasks.add(taskId); mActiveTaskConfigs.put(taskId, new HeadlessJsTaskConfig(taskConfig)); - reactContext - .getJSModule(AppRegistry.class) - .startHeadlessTask(taskId, taskConfig.getTaskKey(), taskConfig.getData()); + if (reactContext.hasActiveCatalystInstance()) { + reactContext + .getJSModule(AppRegistry.class) + .startHeadlessTask(taskId, taskConfig.getTaskKey(), taskConfig.getData()); + } else { + ReactSoftException.logSoftException( + "HeadlessJsTaskContext", + new RuntimeException("Cannot start headless task, CatalystInstance not available")); + } if (taskConfig.getTimeout() > 0) { scheduleTaskTimeout(taskId, taskConfig.getTimeout()); }