From b2ca41eef59b17d212b35baa4a28c4b27a465b5c Mon Sep 17 00:00:00 2001 From: Evert Eti Date: Tue, 26 Mar 2024 15:48:19 -0700 Subject: [PATCH] Fix possible deadlock in dispatchViewUpdates (#43643) Summary: In https://github.com/th3rdwave/react-native-safe-area-context/issues/448 it was noticed that from 0.72 (https://github.com/facebook/react-native/commit/bc766ec7f8b18ddc0ff72a2fff5783eeeff24857 https://github.com/facebook/react-native/pull/35889) it was possible to get a deadlock in dispatchViewUpdates. ([More details](https://github.com/th3rdwave/react-native-safe-area-context/issues/448#issuecomment-1871155936)) This deadlock resulted in a laggy experience for all users using https://github.com/th3rdwave/react-native-safe-area-context/ on Android. To avoid this problem, the author of the original fix [proposed](https://github.com/th3rdwave/react-native-safe-area-context/issues/448#issuecomment-1872121172) this solution which was tested by Discord and many other users. It would be great to have this backported to 0.72 and 0.73 because of the large userbase using react-native-safe-area-context since it's recommended by expo and react-navigation. ## Changelog: [ANDROID] [FIXED] - Fixed possible deadlock in dispatchViewUpdates Pull Request resolved: https://github.com/facebook/react-native/pull/43643 Test Plan: The original memory leak remains fixed, and can be verified in https://github.com/feiyin0719/RNMemoryLeakAndroid. To verify the deadlock is gone, every app using https://github.com/th3rdwave/react-native-safe-area-context will work smoothly and not log any excessive `Timed out waiting for layout` (https://github.com/17Amir17/SafeAreaContext) Reviewed By: arushikesarwani94 Differential Revision: D55339059 Pulled By: zeyap fbshipit-source-id: c067997364fbec734510ce99b9994e89d044384a --- packages/react-native/ReactAndroid/api/ReactAndroid.api | 1 + .../com/facebook/react/uimanager/UIImplementation.java | 8 +------- .../com/facebook/react/uimanager/UIManagerModule.java | 7 ++++++- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index c29f1481e289e3..21703cb8afeb2e 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -4892,6 +4892,7 @@ public class com/facebook/react/uimanager/UIImplementation { public fun dispatchViewUpdates (I)V public fun findSubviewIn (IFFLcom/facebook/react/bridge/Callback;)V public fun getProfiledBatchPerfCounters ()Ljava/util/Map; + public fun getRootViewNum ()I protected fun handleCreateView (Lcom/facebook/react/uimanager/ReactShadowNode;ILcom/facebook/react/uimanager/ReactStylesDiffMap;)V protected fun handleUpdateView (Lcom/facebook/react/uimanager/ReactShadowNode;Ljava/lang/String;Lcom/facebook/react/uimanager/ReactStylesDiffMap;)V public fun manageChildren (ILcom/facebook/react/bridge/ReadableArray;Lcom/facebook/react/bridge/ReadableArray;Lcom/facebook/react/bridge/ReadableArray;Lcom/facebook/react/bridge/ReadableArray;Lcom/facebook/react/bridge/ReadableArray;)V diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java index 0dbe9352b779e8..6b3ac1afd410de 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java @@ -177,7 +177,7 @@ public void removeRootView(int rootViewTag) { * * @return The num of root view */ - private int getRootViewNum() { + public int getRootViewNum() { return mOperationsQueue.getNativeViewHierarchyManager().getRootViewNum(); } @@ -589,12 +589,6 @@ public void measureLayoutRelativeToParent( /** Invoked at the end of the transaction to commit any updates to the node hierarchy. */ public void dispatchViewUpdates(int batchId) { - if (getRootViewNum() <= 0) { - // If there are no RootViews registered, there will be no View updates to dispatch. - // This is a hack to prevent this from being called when Fabric is used everywhere. - // This should no longer be necessary in Bridgeless Mode. - return; - } SystraceMessage.beginSection( Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "UIImplementation.dispatchViewUpdates") .arg("batchId", batchId) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java index 15a1b42542989d..dca4fcf5b13098 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java @@ -707,7 +707,12 @@ public void onBatchComplete() { listener.willDispatchViewUpdates(this); } try { - mUIImplementation.dispatchViewUpdates(batchId); + // If there are no RootViews registered, there will be no View updates to dispatch. + // This is a hack to prevent this from being called when Fabric is used everywhere. + // This should no longer be necessary in Bridgeless Mode. + if (mUIImplementation.getRootViewNum() > 0) { + mUIImplementation.dispatchViewUpdates(batchId); + } } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); }