Reloading a dev app rapidly crashes the app #44755
Comments
hannojg
added
Needs: Triage 🔍
Type: New Architecture
github-actions
bot
added
Needs: Author Feedback
Needs: Repro
|
@hannojg Thanks for the issue. I just tried myself with an app on 0.74.1 and I could not reproduce the issue: Screen.Recording.2024-06-03.at.10.30.40.mov |
|
I created a new app with 0.74.1 to reproduce your same scenario and environment. I can't repro the issue there either. Screen.Recording.2024-06-03.at.10.48.24.mov |
|
Hmm, this is odd, I can reliably reproduce this issue 🤔 Screen.Recording.2024-06-03.at.12.35.48.mov// Note: I wasn't able to reproduce the issue on a real device; not sure what could make the difference |
github-actions
bot
added
Needs: Attention
|
uhm, you sure you pushed? I only see these commits: Perhaps you committed but not pushed to remote? I just tried with the reproducer (which spins up a new app with 0.74.1) and can't repro it... :/ |
|
So sorry, forgot to push, its up now! |
|
😞 😞 😞 😞 😞 😞 😞 😞 ...no crashes... Screen.Recording.2024-06-03.at.14.07.30.mov |
|
Thanks for testing, so weird it's not reproducible on your end. I see that you use iOS 17.5, where I am using 17.4 (although I wouldn't know what difference that could make). Will upgrade tonight and try to reproduce tmrw on iOS 17.5 (any other ideas what could cause the difference?) |
|
that could be something, actually. I'm using Xcode 15.4 and Apple made several changes in Xcode 15 for C++, in every version basically... So perhaps that fixes? |
cortinico
removed
Needs: Triage 🔍
Needs: Attention
|
I upgraded to Xcode 15.4 and tested with iOS 17.5 and I am still able to reproduce it 😅
|
|
😭 I'm very sad and sorry. But I can't reproduce it. And it is a completely new app, with the reproducer. :( |
|
@hannojg @cipolleschi Hey! I cloned the above repo and I was able to reproduce this issue for iOS Screen.Recording.2024-06-06.at.12.40.10.AM.1.1.1.1.mov |
|
I was able to reproduce it, so I will try to investigate this one. metro-crash-recording1080p.mov |
|
@cipolleschi For now I can tell that the problem occurs, because of the race condition that happens when there are two reloads one after the other. When the reload starts the RCTSurfacePresenter is suspended which deallocates the scheduler with its delegate on the UI thread. In unfortunate situation it may happen right after the I am thinking about the possible solution as it would require checking during the commit to see if there is another reload that just happened and potentially dropping the commit. It would require some locks to stop deallocation from the UI thread during a commit on the JS thread or drop stale commits if there is a new reload and deallocation has already happened. We could also debounce reloads as it would be the easiest and the least invasive solution. |
|
I like the debounce solution. We can isolate it in a single location in the codebase and mitigate the issue to the point that it does not present itself anymore. |
Summary: Regarding the [issue](#44755) where the app sometimes crashes due to race condition when two reloads overlap in unfortunate way. This PR fixes it in some way by introducing throttling on reload command. For now I set it to 700ms as I was still able to reproduce it on 500-550ms for provided repro in the issue. The problem may still happen for bigger apps where reload may take more time to finish. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [GENERAL] [FIXED] - throttle reload command Pull Request resolved: #46416 Test Plan: I've tested on provided repro and a smaller app trying to brake it. Reviewed By: huntie Differential Revision: D62847076 Pulled By: cipolleschi fbshipit-source-id: 6471f792d6b692e87e3e98a699443a88c6ef43cd
Summary: Regarding the [issue](#44755) where the app sometimes crashes due to race condition when two reloads overlap in unfortunate way. This PR fixes it in some way by introducing throttling on reload command. For now I set it to 700ms as I was still able to reproduce it on 500-550ms for provided repro in the issue. The problem may still happen for bigger apps where reload may take more time to finish. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [GENERAL] [FIXED] - throttle reload command Pull Request resolved: #46416 Test Plan: I've tested on provided repro and a smaller app trying to brake it. Reviewed By: huntie Differential Revision: D62847076 Pulled By: cipolleschi fbshipit-source-id: 6471f792d6b692e87e3e98a699443a88c6ef43cd
# Why fix ios reload crash, e.g. `Updates.reloadAsync()` on new architecture mode. close ENG-13646 # How fundamentally, the issue is from react-native core and could relate to facebook/react-native#44755. this pr is trying to send `RCTTriggerReloadCommandListeners` from main thread. that would highly reduce the possibility of having the crash.
fix ios reload crash, e.g. `Updates.reloadAsync()` on new architecture mode. close ENG-13646 fundamentally, the issue is from react-native core and could relate to facebook/react-native#44755. this pr is trying to send `RCTTriggerReloadCommandListeners` from main thread. that would highly reduce the possibility of having the crash. (cherry picked from commit ba9b2da)
cortinico
added
the
Resolution: Fixed
Description
When repeatedly reloading the app pressing "r" in the metro bundler the app crashes
Steps to reproduce
1.1
cd ReproducerApp1.2
yarn1.3
yarn pods1.4 Open the iOS project and build
xed iosNote: The effect is amplified in a real world app with more elements.
React Native Version
0.74.1
Affected Platforms
Runtime - iOS
Areas
Fabric - The New Renderer
Output of
npx react-native infoStacktrace or Logs
Reproducer
https://github.com/hannojg/crash-on-reload
Screenshots and Videos
Screen.Recording.2024-06-03.at.09.25.19.mov
The text was updated successfully, but these errors were encountered: