Skip to content

Commit

Permalink
Fix possible deadlock in dispatchViewUpdates (#43643)
Browse files Browse the repository at this point in the history
Summary:
In th3rdwave/react-native-safe-area-context#448 it was noticed that from 0.72 (bc766ec #35889) it was possible to get a deadlock in dispatchViewUpdates. ([More details](th3rdwave/react-native-safe-area-context#448 (comment)))

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](th3rdwave/react-native-safe-area-context#448 (comment)) 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.

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID] [FIXED] - Fixed possible deadlock in dispatchViewUpdates

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[ANDROID] [FIXED] - Fixed possible deadlock in dispatchViewUpdates

Pull Request resolved: #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
  • Loading branch information
EvertEt authored and alfonsocj committed Apr 12, 2024
1 parent 2f98436 commit b741899
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit b741899

Please sign in to comment.