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 | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index 84fa847b7a4926..e338e5f9c4b103 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 List getOrCreateViewManagers( 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; - } + List viewManagerNames = mViewManagerNames; + if (viewManagerNames != null) { + return viewManagerNames; + } + 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); - } + 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); + SystraceMessage.endSection(TRACE_TAG_REACT_JAVA_BRIDGE).flush(); } + Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE); + mViewManagerNames = new ArrayList<>(uniqueNames); } + return mViewManagerNames; } - return new ArrayList<>(mViewManagerNames); } /** Add a listener to be notified of react instance events. */