-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[ios] useAnimatedKeyboard causes crash when reloading app #3786
Comments
@Norfeldt I think this may be different, and not just an xcode version issue. I'm encountering it in debug mode on xcode 14.0.1, as well as in the Expo Snack I provided for repro. This also occurs in release builds and on real iPhones when reloading the app (e.g. via expo's |
Thanks for investigating if it is the same issue. Sorry for the disturbance. |
Got similar crash in iOS when tried to reload using expo
|
#3895 opened with reproduction |
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary Fixes #3786 ### Why it crashes: On the main thread `CADisplayLink` calls `updateKeyboardFrame` 60 times per second. Calling `updateKeyboardFrame` calls listener on the JS side. When reloading the runtime gets destroyed on the JavaScript thread. So when those two things happen at the same time (which in this case happens often) we get the crash that we're trying to call a js function on destroyed js runtime. ### Why don't you just remove the listeners in `NativeReanimatedModule::~NativeReanimatedModule()`, we're cleaning up everything here: I've tried and it appeared to work at first but I still got crashes when running [the 1000 listeners example](#3911) and probably that's why: ![Screenshot 2023-01-11 at 23 02 18](https://user-images.githubusercontent.com/6280457/211935579-16ff642d-fb15-469b-909e-e36ba9d72781.png) ![Screenshot 2023-01-11 at 23 02 35](https://user-images.githubusercontent.com/6280457/211935599-88cb9e81-20d7-4f96-bfd0-9c3b5da13b24.png) So when `~NativeReanimatedModule` is being called the runtime related stuff is already deleted and there is a short window of time that the runtime is being deleted and we still call js stuff, or at least that's my theory 🤷♀️ So my proposition for now is to listen for `RCTBridgeDidInvalidateModulesNotification` notification. It's called just before deleting happens. Also I'm using `RCTUnsafeExecuteOnMainQueueSync` because I want to wait until the listeners are completely deleted on the main thread and then delete js stuff. ### A nicer solution? If you look a bit above `[[NSNotificationCenter defaultCenter] postNotificationName:RCTBridgeDidInvalidateModulesNotification` line in React code you'll see that React calls `invalidate` on all modules' data before even posting the notification. Maybe we should clean reanimated stuff here. I haven't explored that though yet and I don't know where is reanimated's module data and what is module data at all and where to put that `invalidate` function, I just got this idea while posting this PR. ## Test plan Just run AnimatedKeyboardExample and reload the app. Also the [the 1000 listeners example](#3911) nicely catches other data races. Tested with changes from #3911 and it works Also tested on FabricExample and surprisingly it also works.
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary Fixes software-mansion#3786 ### Why it crashes: On the main thread `CADisplayLink` calls `updateKeyboardFrame` 60 times per second. Calling `updateKeyboardFrame` calls listener on the JS side. When reloading the runtime gets destroyed on the JavaScript thread. So when those two things happen at the same time (which in this case happens often) we get the crash that we're trying to call a js function on destroyed js runtime. ### Why don't you just remove the listeners in `NativeReanimatedModule::~NativeReanimatedModule()`, we're cleaning up everything here: I've tried and it appeared to work at first but I still got crashes when running [the 1000 listeners example](software-mansion#3911) and probably that's why: ![Screenshot 2023-01-11 at 23 02 18](https://user-images.githubusercontent.com/6280457/211935579-16ff642d-fb15-469b-909e-e36ba9d72781.png) ![Screenshot 2023-01-11 at 23 02 35](https://user-images.githubusercontent.com/6280457/211935599-88cb9e81-20d7-4f96-bfd0-9c3b5da13b24.png) So when `~NativeReanimatedModule` is being called the runtime related stuff is already deleted and there is a short window of time that the runtime is being deleted and we still call js stuff, or at least that's my theory 🤷♀️ So my proposition for now is to listen for `RCTBridgeDidInvalidateModulesNotification` notification. It's called just before deleting happens. Also I'm using `RCTUnsafeExecuteOnMainQueueSync` because I want to wait until the listeners are completely deleted on the main thread and then delete js stuff. ### A nicer solution? If you look a bit above `[[NSNotificationCenter defaultCenter] postNotificationName:RCTBridgeDidInvalidateModulesNotification` line in React code you'll see that React calls `invalidate` on all modules' data before even posting the notification. Maybe we should clean reanimated stuff here. I haven't explored that though yet and I don't know where is reanimated's module data and what is module data at all and where to put that `invalidate` function, I just got this idea while posting this PR. ## Test plan Just run AnimatedKeyboardExample and reload the app. Also the [the 1000 listeners example](software-mansion#3911) nicely catches other data races. Tested with changes from software-mansion#3911 and it works Also tested on FabricExample and surprisingly it also works.
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> Fixes software-mansion#3786 On the main thread `CADisplayLink` calls `updateKeyboardFrame` 60 times per second. Calling `updateKeyboardFrame` calls listener on the JS side. When reloading the runtime gets destroyed on the JavaScript thread. So when those two things happen at the same time (which in this case happens often) we get the crash that we're trying to call a js function on destroyed js runtime. `NativeReanimatedModule::~NativeReanimatedModule()`, we're cleaning up everything here: I've tried and it appeared to work at first but I still got crashes when running [the 1000 listeners example](software-mansion#3911) and probably that's why: ![Screenshot 2023-01-11 at 23 02 18](https://user-images.githubusercontent.com/6280457/211935579-16ff642d-fb15-469b-909e-e36ba9d72781.png) ![Screenshot 2023-01-11 at 23 02 35](https://user-images.githubusercontent.com/6280457/211935599-88cb9e81-20d7-4f96-bfd0-9c3b5da13b24.png) So when `~NativeReanimatedModule` is being called the runtime related stuff is already deleted and there is a short window of time that the runtime is being deleted and we still call js stuff, or at least that's my theory 🤷♀️ So my proposition for now is to listen for `RCTBridgeDidInvalidateModulesNotification` notification. It's called just before deleting happens. Also I'm using `RCTUnsafeExecuteOnMainQueueSync` because I want to wait until the listeners are completely deleted on the main thread and then delete js stuff. If you look a bit above `[[NSNotificationCenter defaultCenter] postNotificationName:RCTBridgeDidInvalidateModulesNotification` line in React code you'll see that React calls `invalidate` on all modules' data before even posting the notification. Maybe we should clean reanimated stuff here. I haven't explored that though yet and I don't know where is reanimated's module data and what is module data at all and where to put that `invalidate` function, I just got this idea while posting this PR. Just run AnimatedKeyboardExample and reload the app. Also the [the 1000 listeners example](software-mansion#3911) nicely catches other data races. Tested with changes from software-mansion#3911 and it works Also tested on FabricExample and surprisingly it also works.
Description
When using the
useAnimatedKeyboard
hook on iOS, attempting to reload the app causes a crash.I've observed this with:
react-native-reanimated
2.10.0
,2.11.0
and2.12.0
expo
46
and47
react-native
0.69
and0.70
in both debug and release schemes (on release, via expo's
Updates.reloadAsync()
)on both the iOS simulator and a real device
Using JSC, the crash happens immediately.
Using hermes, sometimes the crash doesn't happen until you focus a text input (presenting the keyboard) after reloading the app.
I created two snacks to demonstrate the crash:
useAnimatedKeyboard()
commented out.Steps to reproduce
useAnimatedKeyboard()
to any component that's rendered in your app.Snack or a link to a repository
https://snack.expo.dev/@danscan/fa5482
Reanimated version
2.12.0
React Native version
0.70.5
Platforms
iOS
JavaScript runtime
JSC
Workflow
Expo managed workflow
Architecture
Paper (Old Architecture)
Build type
Debug mode
Device
iOS simulator
Device model
No response
Acknowledgements
Yes
The text was updated successfully, but these errors were encountered: