From fb386fccddfe381fd6af5656c13fac802bffd316 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Thu, 8 Jul 2021 12:09:34 -0700 Subject: [PATCH] Fix NullPointerException caused by race condition in ReactInstanceManager.getViewManagerNames method Summary: This diff fixes a NullPointerException that is caused when the method ReactInstanceManager.getViewManagerNames is called at the same time ReactInstanceManager is being destroyed. Following the stacktrace I noticed that this crash can only happen when RN was destroyed by another thread while this method was being executed This diff fixes the NullPointerException, but it doesn't fix the root cause race condition that cuases this bug changelog: [Android][Fixed] Fix NullPointerException caused by race condition in ReactInstanceManager.getViewManagerNames method Reviewed By: JoshuaGross Differential Revision: D29616401 fbshipit-source-id: 6ae8ecdd765d2fe3529fef3237f08b182d8ed243 --- .../facebook/react/ReactInstanceManager.java | 62 ++++++++++--------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index 84fa847b7a4..e338e5f9c4b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -918,38 +918,40 @@ public class ReactInstanceManager { public @Nullable List getViewManagerNames() { Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "ReactInstanceManager.getViewManagerNames"); - if (mViewManagerNames == null) { - ReactApplicationContext context; - synchronized (mReactContextLock) { - context = (ReactApplicationContext) getCurrentReactContext(); - if (context == null || !context.hasActiveReactInstance()) { - return null; - } - } - - synchronized (mPackages) { - if (mViewManagerNames == null) { - Set uniqueNames = new HashSet<>(); - for (ReactPackage reactPackage : mPackages) { - SystraceMessage.beginSection( - TRACE_TAG_REACT_JAVA_BRIDGE, "ReactInstanceManager.getViewManagerName") - .arg("Package", reactPackage.getClass().getSimpleName()) - .flush(); - if (reactPackage instanceof ViewManagerOnDemandReactPackage) { - List names = - ((ViewManagerOnDemandReactPackage) reactPackage).getViewManagerNames(context); - if (names != null) { - uniqueNames.addAll(names); - } - } - SystraceMessage.endSection(TRACE_TAG_REACT_JAVA_BRIDGE).flush(); - } - Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE); - mViewManagerNames = new ArrayList<>(uniqueNames); - } + List viewManagerNames = mViewManagerNames; + if (viewManagerNames != null) { + return viewManagerNames; + } + ReactApplicationContext context; + synchronized (mReactContextLock) { + context = (ReactApplicationContext) getCurrentReactContext(); + if (context == null || !context.hasActiveReactInstance()) { + return null; } } - return new ArrayList<>(mViewManagerNames); + + synchronized (mPackages) { + if (mViewManagerNames == null) { + Set uniqueNames = new HashSet<>(); + for (ReactPackage reactPackage : mPackages) { + SystraceMessage.beginSection( + TRACE_TAG_REACT_JAVA_BRIDGE, "ReactInstanceManager.getViewManagerName") + .arg("Package", reactPackage.getClass().getSimpleName()) + .flush(); + if (reactPackage instanceof ViewManagerOnDemandReactPackage) { + List names = + ((ViewManagerOnDemandReactPackage) reactPackage).getViewManagerNames(context); + if (names != null) { + uniqueNames.addAll(names); + } + } + SystraceMessage.endSection(TRACE_TAG_REACT_JAVA_BRIDGE).flush(); + } + Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE); + mViewManagerNames = new ArrayList<>(uniqueNames); + } + return mViewManagerNames; + } } /** Add a listener to be notified of react instance events. */