Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Android] Slow screen opening after react-native update #448

Open
OrlovMaks opened this issue Oct 26, 2023 · 63 comments
Open

[Android] Slow screen opening after react-native update #448

OrlovMaks opened this issue Oct 26, 2023 · 63 comments

Comments

@OrlovMaks
Copy link

Hey, everybody!
After updating react-native (and all libraries) from version 0.70.11 to 0.72.6, I got a problem that screens with SafeAreaView open slowly and without animation.
I couldn't find absolutely no information on how to fix it.

In the video the first screen "Security" is in SafeAreaView, the second screen "Your rank" is just View.
https://github.com/th3rdwave/react-native-safe-area-context/assets/93343100/45623a6f-a038-4372-837a-abfa7466b8e8

My App component is wrapped with (I tried with and without the initialMetrics parameter).

My package.json
{ "react": "18.2.0",
"react-native": "0.72.6",
"react-native-safe-area-context": "4.7.4",
"react-native-screens": "3.27.0",
"react-native-reanimated": "3.5.4",
"@react-navigation/bottom-tabs": "6.5.11",
"@react-navigation/native": "6.1.9",
"@react-navigation/stack": "6.3.20",

@efstathiosntonas
Copy link

Maybe a long shot but, do you by any chance have reduce motion enabled on the device/simulator?

@17Amir17
Copy link

17Amir17 commented Oct 27, 2023

I see this as-well something like a 500ms delay on rendering whatever is inside safe view
RN 0.72.4 looks like only on Android - and Reduce Motion is not enabled
My other App on RN 0.71.6 works fine

@jacobp100
Copy link
Collaborator

There's not really anything to go off here. If you manage to make a minimal reproduction - or better yet, find the actual issue - let us know.

@OrlovMaks
Copy link
Author

@jacobp100
Tomorrow I will make a copy of the application to the public repository and provide a link to it.
Unfortunately, I myself could not solve this problem.

@OrlovMaks
Copy link
Author

@jacobp100 @17Amir17
I prepared a repository with the project from which I removed everything except the structure of my navigation:
https://github.com/OrlovMaks/sample_safeAreaView_issue

While I was preparing this project I found a problem (check the screenshot):
I have detachInactiveScreens={false} everywhere and I make navigation from WalletStackNavigator to general StackNavigator if I remove detachInactiveScreens from this general stack everything works.
https://github.com/th3rdwave/react-native-safe-area-context/assets/93343100/4225fb1f-275d-4252-aefc-ac4d0ad50316

@jacobp100
Copy link
Collaborator

Right so is it a problem with this library or another?

@17Amir17
Copy link

17Amir17 commented Oct 29, 2023

I have this issue with BottomSheetModal. I created https://github.com/17Amir17/SafeAreaContext to reproduce. From the comments in my codebase the reason we have a SafeArea in the modal is because of a rare cutoff of some android device.

Screen.Recording.2023-10-28.at.20.37.46.mov

I'm not really sure how to debug this further but would love to help @jacobp100 if you have any directions

@mgerdes
Copy link

mgerdes commented Nov 1, 2023

@jacobp100 I'm seeing this issue too. It seems like it's because is we are hitting this timeout in SafeAreaView and that freezes the UI for 500ms

if (waitTime >= MAX_WAIT_TIME_NANO) {
Log.w("SafeAreaView", "Timed out waiting for layout.")
}

I debugged a little bit and it seems like the code in runOnNativeModulesQueueThread below is never able to acquire the lock, because the lock is being held the entire time we are waiting in the while loop right below, so it doesn't complete until the timeout ends.

getReactContext(this).runOnNativeModulesQueueThread {
lock.withLock {
if (!done) {
done = true
condition.signal()
}
}
}
lock.withLock {
while (!done && waitTime < MAX_WAIT_TIME_NANO) {
try {
condition.awaitNanos(MAX_WAIT_TIME_NANO)
} catch (ex: InterruptedException) {
// In case of an interrupt just give up waiting.
done = true
}
waitTime += System.nanoTime() - startTime
}
}

I'm not sure what the best fix here is though.

@jacobp100
Copy link
Collaborator

Good catch on that. If you're able to get some flame graph and figure out what exactly is blocking the main thread, that would be helpful. Is this something that shows up only in development - or production too?

@mgerdes
Copy link

mgerdes commented Nov 1, 2023

Good catch on that. If you're able to get some flame graph and figure out what exactly is blocking the main thread, that would be helpful. Is this something that shows up only in development - or production too?

I think the waitForReactLayout in SafeAreaView.kt is what's blocking the main thread. I added these log statements to try to see what's happening

Screenshot 2023-11-01 at 1 18 43 PM

and this was the result:

2023-11-01 13:03:54.865 runOnNativeModulesQueueThread 0
2023-11-01 13:03:54.865 main thread 0
2023-11-01 13:03:54.866 awaitNanos 0
2023-11-01 13:03:55.367 awaitNanos 1
2023-11-01 13:03:55.367 main thread 1
2023-11-01 13:03:55.367 Timed out waiting for layout.
2023-11-01 13:03:55.377 native thread 0
2023-11-01 13:03:55.377 native thread 1

So if I'm looking at this right it seems like the code in runOnNativeModulesQueueThread that's supposed to run on the native thread isn't run while the main thread is blocked and that makes waitForReactLayout block the main thread the entire time.

@jacobp100
Copy link
Collaborator

If you increase the max time nanos to something much longer (say 10s), does it finish before? Or do we have a deadlock?

@mgerdes
Copy link

mgerdes commented Nov 1, 2023

If you increase the max time nanos to something much longer (say 10s), does it finish before? Or do we have a deadlock?

Yea, I just upped it to 10s and the whole app freezes for those 10 seconds it's waiting.

This is what I was logging:

2023-11-01 14:22:29.130  runOnNativeModulesQueueThread 0
2023-11-01 14:22:29.130  main thread 0
2023-11-01 14:22:29.130  awaitNanos 0
2023-11-01 14:22:39.131  awaitNanos 1
2023-11-01 14:22:39.131  main thread 1
2023-11-01 14:22:39.131  Timed out waiting for layout.
2023-11-01 14:22:39.140  native thread 0
2023-11-01 14:22:39.140  native thread 1

So yea, it looks like something isn't letting the code on the native thread run until the main thread times out waiting.

@jacobp100
Copy link
Collaborator

jacobp100 commented Nov 1, 2023

What’s your setup? Version of react native? Are you using fabric?

Also could you check the value of getReactContext(this).isOnNativeModulesQueueThread() (at the start of the function - outside the runOnNativeModulesQueueThread)

@mgerdes
Copy link

mgerdes commented Nov 1, 2023

We are on react-native 0.72.5. No we are not using fabric.

I printed the value of getReactContext(this).isOnNativeModulesQueueThread at the start of the function - outside of runOnNativeModulesQueueThread, and it returns false.

@jacobp100
Copy link
Collaborator

Try adding a printStackTrace in the runOnNativeModulesQueueThread function in the react native library code and see if there’s something else that runs around that time. It sounds like this and another task are in deadlock

@mgerdes
Copy link

mgerdes commented Nov 1, 2023

Here's what the stacktrace looks like in runOnNativeModulesQueueThread

java.lang.Exception: Stack trace
	at java.lang.Thread.dumpStack(Thread.java:1615)
	at com.th3rdwave.safeareacontext.SafeAreaView.waitForReactLayout(SafeAreaView.kt:71)
	at com.th3rdwave.safeareacontext.SafeAreaView.updateInsets(SafeAreaView.kt:52)
	at com.th3rdwave.safeareacontext.SafeAreaView.maybeUpdateInsets(SafeAreaView.kt:113)
	at com.th3rdwave.safeareacontext.SafeAreaView.onAttachedToWindow(SafeAreaView.kt:134)
	at android.view.View.dispatchAttachedToWindow(View.java:21357)
	at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3491)
	at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3498)
	at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3498)
	at android.view.ViewGroup.addViewInner(ViewGroup.java:5291)
	at android.view.ViewGroup.addView(ViewGroup.java:5077)
	at com.facebook.react.views.view.ReactViewGroup.addView(ReactViewGroup.java:516)
	at android.view.ViewGroup.addView(ViewGroup.java:5017)
	at com.facebook.react.uimanager.ViewGroupManager.addView(ViewGroupManager.java:38)
	at com.facebook.react.uimanager.NativeViewHierarchyManager.manageChildren(NativeViewHierarchyManager.java:533)
	at com.swmansion.reanimated.layoutReanimation.ReanimatedNativeHierarchyManager.manageChildren(ReanimatedNativeHierarchyManager.java:300)
	at com.facebook.react.uimanager.UIViewOperationQueue$ManageChildrenOperation.execute(UIViewOperationQueue.java:217)
	at com.facebook.react.uimanager.UIViewOperationQueue$1.run(UIViewOperationQueue.java:915)
	at com.facebook.react.uimanager.UIViewOperationQueue.flushPendingBatches(UIViewOperationQueue.java:1026)
	at com.facebook.react.uimanager.UIViewOperationQueue.-$$Nest$mflushPendingBatches(Unknown Source:0)
	at com.facebook.react.uimanager.UIViewOperationQueue$DispatchUIFrameCallback.doFrameGuarded(UIViewOperationQueue.java:1086)
	at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.java:29)
	at com.facebook.react.modules.core.ReactChoreographer$ReactChoreographerDispatcher.doFrame(ReactChoreographer.java:175)
	at com.facebook.react.modules.core.ChoreographerCompat$FrameCallback$1.doFrame(ChoreographerCompat.java:85)
	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1229)
	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1239)
	at android.view.Choreographer.doCallbacks(Choreographer.java:899)
	at android.view.Choreographer.doFrame(Choreographer.java:827)
	at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1214)
	at android.os.Handler.handleCallback(Handler.java:942)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loopOnce(Looper.java:201)
	at android.os.Looper.loop(Looper.java:288)
	at android.app.ActivityThread.main(ActivityThread.java:7918)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)

@jacobp100
Copy link
Collaborator

Do you have a few traces before and after that?

@mgerdes
Copy link

mgerdes commented Nov 1, 2023

I'm not sure what you mean? Before and after it's inside runOnNativeModulesQueueThread? Or still inside of runOnNativeModulesQueueThread?

@jacobp100
Copy link
Collaborator

I think some other functions - presumably outside of this library - will be calling runOnNativeModulesQueueThread before or after this call. If there are, what are they?

@smokfyz
Copy link

smokfyz commented Nov 2, 2023

Hi. I got the same issue on iOS. Temporary fixed it using useSafeAreaInsets and paddings instead of SafeAreaView .

@enahum
Copy link

enahum commented Nov 20, 2023

facing the same problem after updating to RN 0.72+ including 0.73-alpha(s).. this is in combination with react-native-navigation from wix.

@danilobuerger
Copy link
Contributor

Experiencing the same issue after upgrading to RN 0.72 (from 0.68). @jacobp100 is there anything I can do to help you debug this?

@jacobp100
Copy link
Collaborator

Somebody needs to look into it. There’s only two people that actively maintain this, and I don’t think either of us will investigate this - so if you want this fixed, do take a look for yourself. We are of course on hand to answer any questions etc.

@danilobuerger
Copy link
Contributor

So what I figured out is that

uiManager.uiImplementation.dispatchViewUpdates(-1)

scheduled on the native module queue is called after the lock times out. I would assume that this means that waitForReactLayout blocks the execution of anything on the native modules queue.

@danilobuerger
Copy link
Contributor

@jacobp100 I found the offending code 🥳

In SafeAreaView we call

uiManager.setViewLocalData(id, localData)

before

waitForReactLayout()

This runs

mUIImplementation.setViewLocalData(tag, data);

on the native modules thread.

This in turn calls

dispatchViewUpdatesIfNeeded() -> dispatchViewUpdates()

Since rn 72 (facebook/react-native@bc766ec) we have

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;
}

guarding the entry and this in turn waits for the main thread where we are already waiting for this thread. Deadlock.

@jacobp100
Copy link
Collaborator

🎉 do you know what the fix is?

@danilobuerger
Copy link
Contributor

Sadly no idea yet. The problem is that waitForReactLayout is called inside NativeViewHierarchyManager.manageChildren (see stack trace above) and that synchronizes on the same object as getRootViewNum so it will always deadlock. Maybe @feiyin0719 or @kelset from the original PR introducing this know more 🤷‍♂️

@jacobp100
Copy link
Collaborator

jacobp100 commented Dec 28, 2023

My experience with core dev is they’re too busy to get back to you. If you find a work around though, we here are good at merging and releasing PRs. It sounds like you’re really close!

@EvertEt
Copy link

EvertEt commented Mar 13, 2024

@feiyin0719 Thanks for getting back to me! I just tested your suggestion and it does work in this case, we no longer deadlock. I am unsure if it still fixes your memory leak. Should I push a branch so you can validate?

@feiyin0719 Is there any way we can support testing this change to get it merged back?
See discord/react-native@5c0d4da for a version of the proposed change.

It looks like this issue affects several apps. Thanks in advance

@patrickkempff
Copy link

We have the same problem, we are trying to apply the patch from discord/react-native@5c0d4da to see if it fixes our issue.

@vanHoi
Copy link

vanHoi commented Mar 13, 2024

We also have this issue in our app. When applying the patch from discord/react-native@5c0d4da and building React Native from source, it fixes the issue.

@yayvery
Copy link

yayvery commented Mar 13, 2024

thanks for confirming it fixes the issue @vanHoi ! I'll try to get this reviewed & merged into react-native (need to figure out the CLA situation tho)

@EvertEt
Copy link

EvertEt commented Mar 14, 2024

Thanks @yayvery , let us know if you need help or want someone else to open the PR.

It would be great to ask (like @kelset suggested) to have this fix cherry picked to 0.72.

@vanHoi
Copy link

vanHoi commented Mar 14, 2024

Thanks @yayvery for the solution! Just to clarify, we have applied this patch to React Native 0.73.6, the issues still persists there.

@EvertEt
Copy link

EvertEt commented Mar 16, 2024

I can also confirm the patch works on 0.72.12.

@jackylu0124
Copy link

@yayvery @EvertEt @vanHoi hey all, I have a quick question, is this fix incorporated into the React Native 0.72.12 version? If not, is there a plan to incorporate this fix into the next 0.72 patch version? Thanks a lot in advance!

I think this issue (#219 (comment)) is also related to one discussed above.

@EvertEt
Copy link

EvertEt commented Mar 22, 2024

@yayvery @EvertEt @vanHoi hey all, I have a quick question, is this fix incorporated into the React Native 0.72.12 version? If not, is there a plan to incorporate this fix into the next 0.72 patch version? Thanks a lot in advance!

I think this issue (#219 (comment)) is also related to one discussed above.

@jackylu0124 the fix has not been merged in react native yet, and thus also not yet backported to 0.72.

@jackylu0124
Copy link

@yayvery @EvertEt @vanHoi hey all, I have a quick question, is this fix incorporated into the React Native 0.72.12 version? If not, is there a plan to incorporate this fix into the next 0.72 patch version? Thanks a lot in advance!
I think this issue (#219 (comment)) is also related to one discussed above.

@jackylu0124 the fix has not been merged in react native yet, and thus also not yet backported to 0.72.

@EvertEt I see, thank you for the clarification! Is there a plan/or a task already made for merging the fix into React Native and backporting it to 0.72? Or are we not at that step yet?

@EvertEt
Copy link

EvertEt commented Mar 24, 2024

thanks for confirming it fixes the issue @vanHoi ! I'll try to get this reviewed & merged into react-native (need to figure out the CLA situation tho)

@jackylu0124, @yayvery was going to look into this but not sure what the timeline would be. Feel free to take a look.

@EvertEt
Copy link

EvertEt commented Mar 25, 2024

@feiyin0719 @yayvery Since this is quite a big issue for our project, I've gone ahead and opened a PR facebook/react-native#43643 describing the issue as best as I could. Comments and possible improvements would be much appreciated.

@yayvery
Copy link

yayvery commented Mar 25, 2024

Thanks for owning the PR @EvertEt !

facebook-github-bot pushed a commit to facebook/react-native that referenced this issue Mar 26, 2024
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.

## Changelog:

<!-- 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
@ziga-hvalec
Copy link

ziga-hvalec commented Apr 3, 2024

this does not fix the issue for me.

This is my patch file. Did I miss something?

I'm using
"react-native-safe-area-context": "4.9.0",
"react-native": "0.73.6",

diff --git a/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java b/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java
index 894f323..0363c73 100644
--- a/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java
+++ b/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java
@@ -177,7 +177,7 @@ public class UIImplementation {
    *
    * @return The num of root view
    */
-  private int getRootViewNum() {
+  public int getRootViewNum() {
     return mOperationsQueue.getNativeViewHierarchyManager().getRootViewNum();
   }
 
@@ -610,12 +610,6 @@ public class UIImplementation {
 
   /** 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/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java b/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java
index 9ff04bc..2f863ad 100644
--- a/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java
+++ b/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java
@@ -719,7 +719,12 @@ public class UIManagerModule extends ReactContextBaseJavaModule
       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);
     }

@EvertEt
Copy link

EvertEt commented Apr 3, 2024

@ziga-hvalec Make sure to also build React Native from the source: https://reactnative.dev/contributing/how-to-build-from-source

alanjhughes pushed a commit to facebook/react-native that referenced this issue Apr 9, 2024
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
cortinico pushed a commit to facebook/react-native that referenced this issue Apr 9, 2024
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.

## Changelog:

<!-- 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
alfonsocj pushed a commit to facebook/react-native that referenced this issue Apr 12, 2024
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
@matt-dalton
Copy link

So do we know which React Native versions (if any) include the fix?

@ziga-hvalec
Copy link

Should be 0.73.7.

@matt-dalton
Copy link

Amazing..thanks for confirming

@EvertEt
Copy link

EvertEt commented Apr 22, 2024

A fix for this has been merged in v0.72.13, v0.73.7 and v0.74.0-rc8.
The issue can be closed now here IMO.

douglowder pushed a commit to react-native-tvos/react-native-tvos that referenced this issue Apr 22, 2024
Summary:
In th3rdwave/react-native-safe-area-context#448 it was noticed that from 0.72 (facebook/react-native@bc766ec facebook/react-native#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: facebook/react-native#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
@derrh
Copy link

derrh commented May 6, 2024

We are still getting reports of ANR issues after releasing this patch (React Native 0.72.13). Or is this perhaps a different issue?

tb.b
	at jdk.internal.misc.Unsafe.park(Native Method)
	at java.util.concurrent.locks.LockSupport.parkNanos(LockSupport.java:252)
	at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.awaitNanos(AbstractQueuedSynchronizer.java:1672)
	at com.th3rdwave.safeareacontext.k.i(SafeAreaView.kt:47)
	at com.th3rdwave.safeareacontext.k.g(SafeAreaView.kt:75)
	at com.th3rdwave.safeareacontext.k.f(SafeAreaView.kt:24)
	at com.th3rdwave.safeareacontext.k.onAttachedToWindow(SafeAreaView.kt:21)
	at android.view.View.dispatchAttachedToWindow(View.java:20874)
	at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3490)
	at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3497)
	at android.view.ViewGroup.addViewInner(ViewGroup.java:5293)
	at android.view.ViewGroup.addView(ViewGroup.java:5076)
	at com.facebook.react.views.view.i.addView(ReactViewGroup.java:4)
	at android.view.ViewGroup.addView(ViewGroup.java:5016)
	at com.facebook.react.views.view.ReactClippingViewManager.addView(ReactClippingViewManager.java:6)
	at com.facebook.react.views.view.ReactClippingViewManager.addView(ReactClippingViewManager.java:2)
	at com.facebook.react.uimanager.NativeViewHierarchyManager.manageChildren(NativeViewHierarchyManager.java:417)
	at com.swmansion.reanimated.layoutReanimation.ReanimatedNativeHierarchyManager.manageChildren(ReanimatedNativeHierarchyManager.java:8)
	at com.facebook.react.uimanager.UIViewOperationQueue$ManageChildrenOperation.execute(UIViewOperationQueue.java:15)
	at com.facebook.react.uimanager.UIViewOperationQueue$1.run(UIViewOperationQueue.java:139)
	at com.facebook.react.uimanager.UIViewOperationQueue.flushPendingBatches(UIViewOperationQueue.java:54)
	at com.facebook.react.uimanager.UIViewOperationQueue.x(UIViewOperationQueue.java:1)
	at com.facebook.react.uimanager.UIViewOperationQueue$DispatchUIFrameCallback.doFrameGuarded(UIViewOperationQueue.java:32)
	at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.java:1)
	at com.facebook.react.modules.core.ReactChoreographer$ReactChoreographerDispatcher.doFrame(ReactChoreographer.java:47)
	at com.facebook.react.modules.core.ChoreographerCompat$FrameCallback$1.doFrame(ChoreographerCompat.java:3)
	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1035)
	at android.view.Choreographer.doCallbacks(Choreographer.java:845)
	at android.view.Choreographer.doFrame(Choreographer.java:775)
	at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1022)
	at android.os.Handler.handleCallback(Handler.java:980)
	at android.os.Handler.dispatchMessage(Handler.java:104)
	at android.os.Looper.loopOnce(Looper.java:238)
	at android.os.Looper.loop(Looper.java:357)
	at android.app.ActivityThread.main(ActivityThread.java:8098)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1026)

@AndrewTotsky
Copy link

AndrewTotsky commented Jul 5, 2024

Temporary fixed it using import { SafeAreaView } from 'react-native-safe-area-context/src/SafeAreaView.tsx'; instead of import { SafeAreaView } from 'react-native-safe-area-context';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.