From b741899f9994d270803e38bd98ce81adc8ba93fc 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. [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 --- .../com/facebook/react/uimanager/UIImplementation.java | 8 +------- .../com/facebook/react/uimanager/UIManagerModule.java | 7 ++++++- 2 files changed, 7 insertions(+), 8 deletions(-) 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 894f32380bda2c..0363c73c9de8a9 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(); } @@ -610,12 +610,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 9ff04bc2b07a4f..2f863adc91e6b1 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 @@ -719,7 +719,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); }