From aa27645cf0ea1d71a5f1cd73a7186ae7eedf1e72 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Sun, 27 Oct 2019 23:11:31 -0700 Subject: [PATCH] Guard against `UIManagerHelper.getUIManager` returning null Summary: Because the `mCatalystInstance` of the ReactContext can be null during teardown, there are technicaly cases where `UIManagerHelper.getUIManager` can return null. In those cases we check for a CatalystInstance and raise a SoftException, and return null. We must then guard in every case where we call `getUIManager` to prevent NullPointerExceptions. See T56103679. Currently crashes are coming from `PropsAnimatedNode.restoreDefaultValues` calling `UIManagerModule.synchronouslyUpdateViewOnUIThread` on teardown/at the end of an animation as RN is being torn down. This can happen in both Paper and Fabric. In dev this will still crash because the SoftException will trigger a crash. It will be a noop with logged warnings in production builds. Changelog: [Internal] Reviewed By: yungsters Differential Revision: D18165576 fbshipit-source-id: 7059e04ca339208dd64a0a08a375b565cb8cda02 --- .../react/uimanager/UIManagerHelper.java | 10 +++++++ .../react/uimanager/UIManagerModule.java | 30 +++++++++++++------ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java index d73fc801b2bfc7..3632fe840d7f74 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java @@ -10,9 +10,11 @@ import static com.facebook.react.uimanager.common.UIManagerType.FABRIC; import static com.facebook.react.uimanager.common.ViewUtil.getUIManagerType; +import androidx.annotation.Nullable; import com.facebook.react.bridge.CatalystInstance; import com.facebook.react.bridge.JSIModuleType; import com.facebook.react.bridge.ReactContext; +import com.facebook.react.bridge.ReactSoftException; import com.facebook.react.bridge.UIManager; import com.facebook.react.uimanager.common.UIManagerType; @@ -20,12 +22,20 @@ public class UIManagerHelper { /** @return a {@link UIManager} that can handle the react tag received by parameter. */ + @Nullable public static UIManager getUIManagerForReactTag(ReactContext context, int reactTag) { return getUIManager(context, getUIManagerType(reactTag)); } /** @return a {@link UIManager} that can handle the react tag received by parameter. */ + @Nullable public static UIManager getUIManager(ReactContext context, @UIManagerType int uiManagerType) { + if (!context.hasActiveCatalystInstance()) { + ReactSoftException.logSoftException( + "UIManagerHelper", + new RuntimeException("Cannot get UIManager: no active Catalyst instance")); + return null; + } CatalystInstance catalystInstance = context.getCatalystInstance(); return uiManagerType == FABRIC ? (UIManager) catalystInstance.getJSIModule(JSIModuleType.UIManager) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java index c09545cc116dc2..cdd8f98e9fe24a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java @@ -393,7 +393,9 @@ public void synchronouslyUpdateViewOnUIThread(int tag, ReadableMap props) { if (uiManagerType == FABRIC) { UIManager fabricUIManager = UIManagerHelper.getUIManager(getReactApplicationContext(), uiManagerType); - fabricUIManager.synchronouslyUpdateViewOnUIThread(tag, props); + if (fabricUIManager != null) { + fabricUIManager.synchronouslyUpdateViewOnUIThread(tag, props); + } } else { mUIImplementation.synchronouslyUpdateViewOnUIThread(tag, new ReactStylesDiffMap(props)); } @@ -478,7 +480,9 @@ public void updateView(int tag, String className, ReadableMap props) { if (uiManagerType == FABRIC) { UIManager fabricUIManager = UIManagerHelper.getUIManager(getReactApplicationContext(), uiManagerType); - fabricUIManager.synchronouslyUpdateViewOnUIThread(tag, props); + if (fabricUIManager != null) { + fabricUIManager.synchronouslyUpdateViewOnUIThread(tag, props); + } } else { mUIImplementation.updateView(tag, className, props); } @@ -668,14 +672,20 @@ public void dispatchViewManagerCommand( // the dispatchViewManagerCommand() method is supported by Fabric JS API. if (commandId.getType() == ReadableType.Number) { final int commandIdNum = commandId.asInt(); - UIManagerHelper.getUIManager( - getReactApplicationContext(), ViewUtil.getUIManagerType(reactTag)) - .dispatchCommand(reactTag, commandIdNum, commandArgs); + UIManager uiManager = + UIManagerHelper.getUIManager( + getReactApplicationContext(), ViewUtil.getUIManagerType(reactTag)); + if (uiManager != null) { + uiManager.dispatchCommand(reactTag, commandIdNum, commandArgs); + } } else if (commandId.getType() == ReadableType.String) { final String commandIdStr = commandId.asString(); - UIManagerHelper.getUIManager( - getReactApplicationContext(), ViewUtil.getUIManagerType(reactTag)) - .dispatchCommand(reactTag, commandIdStr, commandArgs); + UIManager uiManager = + UIManagerHelper.getUIManager( + getReactApplicationContext(), ViewUtil.getUIManagerType(reactTag)); + if (uiManager != null) { + uiManager.dispatchCommand(reactTag, commandIdStr, commandArgs); + } } } @@ -801,7 +811,9 @@ public void sendAccessibilityEvent(int tag, int eventType) { if (uiManagerType == FABRIC) { UIManager fabricUIManager = UIManagerHelper.getUIManager(getReactApplicationContext(), uiManagerType); - fabricUIManager.sendAccessibilityEvent(tag, eventType); + if (fabricUIManager != null) { + fabricUIManager.sendAccessibilityEvent(tag, eventType); + } } else { mUIImplementation.sendAccessibilityEvent(tag, eventType); }