App crashes on reload in development mode with JSC Engine #4783
Comments
github-actions
bot
added
Missing repro
|
Same here #4778 |
|
@victor-asdf yes, looks like the same but we identified that the root cause is JSC runtime which should help narrow down the potential scope for debugging and investigation. |
|
@ykliuiev Thanks. I have tried all versions of 3.x and it seems happen all the time |
|
@tomekzaw cc |
|
iOS App is crashing in iOS 16.5.1 & above in during first launch in Real Device. Not found in the older iOS version. When: OS version: iOS 16.5.1 & above. Not seen in below version Podfile changes: use_frameworks! :linkage => :static // use static frameworks Crash Log:Crashed: com.facebook.react.JavaScript |
|
Same here! The app crashes whenever we reload the app in development mode and when hermes = false :( |
|
added simple repro just in case https://github.com/ykliuiev/reanimated-crash-repro |
github-actions
bot
added
Repro provided
|
Same here 1464 |
|
Crash also happens (ios only) when doing a codepush update in the production app since it reloads the app in order to apply a new bundle. Would be great to at least get a workaround or a patch. |
|
We will check it, but please give us a bit of time 🙏 |
|
I'm also facing the same issue. Please kindly assist us by performing a quick check. Thank you. |
|
@piaskowyk thank you! The whole stack trace on sentry is huge, but here is some relevant part I think: |
|
@tomekzaw @piaskowyk Please provide us with feedback, as we are experiencing an issue and need to resolve it quickly 🥺 Thank you very much |
|
This issue is a bit more complicated, Tomek is still working on it in free time. |
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
|
Here's an update: we investigated the issue and found the root cause of the problem. Before we come up with a proper fix, here's a workaround:
JSCRuntime::~JSCRuntime() {
// On shutting down and cleaning up: when JSC is actually torn down,
// it calls JSC::Heap::lastChanceToFinalize internally which
// finalizes anything left over. But at this point,
// JSValueUnprotect() can no longer be called. We use an
// atomic<bool> to avoid unsafe unprotects happening after shutdown
// has started.
ctxInvalid_ = true;
JSGlobalContextRelease(ctx_);
-#ifndef NDEBUG
- assert(
- objectCounter_ == 0 && "JSCRuntime destroyed with a dangling API object");
- assert(
- stringCounter_ == 0 && "JSCRuntime destroyed with a dangling API string");
-#endif
} |
|
Thank you very much, you are awesome @tomekzaw |
|
hey @tomekzaw do you think that without this patch, could this be fixed by using a timeout and reload the app after certain seconds later. I saw this comment in another issue here which is pretty much the same as this one. |
thank you for the patch @tomekzaw, However for anyone attempting this patch, this caused a crash in production for us. The crash was not consistently happening so it may be hard to catch. |
|
I second this, this is a thing. I'll try the suggested workaround meanwhile. Edit: it does work, but i'm yet to find a way to patch this on the pods postinstall. Any suggestions on how to do it, while we wait for this to be fixed?. |
you aware about patch-package ? |
@tomekzaw is there an update on the fix? The suggested workaround introduces a potential memory corruption, so running the app is not safe / may have unspecified consequences. |
|
@leontiy Unfortunately not and this is unlikely to change as we lean towards supporting only Hermes so JSC is not a priority at the moment. |
|
I remember we still had issues enabling Hermes for iOS with internationalisation so this isn't great to hear. Will have to see if there's been any progress on getting Hermes to work. |
|
I think most of the Intl APIs are now working with hermes @dwxw . We are still using JSC for iOS because we see significantly lower memory usage (which is quite surprising). It's been a while since I checked the last time. The whole RN ecosystem is moving towards hermes, so I think what @tomekzaw says makes sense. |
|
It was this: facebook/hermes#1188 |
|
Our team was forced to switch back to JSC since Hermes works terribly with Dates (facebook/hermes#930) |
|
Am I the only one also having this issue on Android? And for Android the suggested patch doesn't work - probably because Android doesn't use the |
|
Facing the same issue Reanimated version: 3.0.2 |
|
Facing the same issue on 0.73, i think should be a quick fix for that on development mode. |
|
I facing same issue on 0.68.2 and react-native-reanimated: 2.14.4 |
|
I facing same issue on "react-native-reanimated": "^3.6.2" and "react-native": "0.71.3" |
in my case "react-native-reanimated": "^3.6.2" and "react-native": "0.71.3" is reload success. thanks |
but what's the relation of this code to our project? |
|
I'm also hitting this crash on Android hot reload. There's indifferent Hermes support for react-native-firebase so we're stuck with JSC for now, too. |
|
I applied the patch, reinstalled all the Pods still doesn't work I'm using Expo |
|
If you are not facing the Hermes problems above (or they have already been fixed) but you are having trouble with hot reload, then you can try turning it on: https://reactnative.dev/docs/hermes#android # Use this property to enable or disable the Hermes JS engine.
# If set to false, you will be using JSC instead.
-hermesEnabled=false
+hermesEnabled=trueFixed for me with - |
|
I'm experiencing the same issues
Please continue supporting JSC 🙏 rn 0.73.7 |
|
"react-native-reanimated": "^3.4.2", |
|
This fixed for me: Remember to remove https://www.npmjs.com/package/@react-native-community/masked-view |
|
It reads as if this is a memory leak, which may still happen with Hermes. Unfortunately, runtime performance of Hermes is not good enough to switch over just yet, despite the recent improvements. If Reanimated wants to drop JSC support that's OK, but can we get an official statement? I need to make decision to stay on Reanimated or not. No hard feelings. |
|
Also seeing this issue in both iOS and Android with RN 72. Is there a decision on JSC support moving forward? It would be good to know so we can decide if we need to move away from reanimated, those of us who are not planning on moving to Hermes any time soon. |
|
same issue here. how to fix this ? |
For anyone having this problem, may seem kinda obvious but...There are only three options here: 1) and bestUpgrade your whole app to hermes, update your reanimated and be good(best solution) 2)Remove reanimated and deal with lack of packages because a lot of packages depends on reanimated. 3)Someone from the community do a pull request candidate to fix this issue and give up on future compatibility with newer versions of reanimated(temporary fix for legacy apps) |
this is truelly a life saver |
|
Enabling hermes for some people is not an option (I haven't seem reports for date-fns, but there's instructions for luxon on using jsc intl so I assume that, again, after years, no one in RN uses any lib that needs timezone and hermes combined). About the patch: Can be done for iOS:(like mentioned above) diff --git a/node_modules/react-native/ReactCommon/jsc/JSCRuntime.cpp b/node_modules/react-native/ReactCommon/jsc/JSCRuntime.cpp
index 235028a..e89c4d5 100644
--- a/node_modules/react-native/ReactCommon/jsc/JSCRuntime.cpp
+++ b/node_modules/react-native/ReactCommon/jsc/JSCRuntime.cpp
@@ -390,12 +390,13 @@ JSCRuntime::~JSCRuntime() {
// No need to unprotect nativeStateSymbol_ since the heap is getting torn down
// anyway
JSGlobalContextRelease(ctx_);
-#ifndef NDEBUG
- assert(
- objectCounter_ == 0 && "JSCRuntime destroyed with a dangling API object");
- assert(
- stringCounter_ == 0 && "JSCRuntime destroyed with a dangling API string");
-#endif
+ // https://github.com/software-mansion/react-native-reanimated/issues/4783#issuecomment-1713329472
+ // #ifndef NDEBUG
+ // assert(
+ // objectCounter_ == 0 && "JSCRuntime destroyed with a dangling API object");
+ // assert(
+ // stringCounter_ == 0 && "JSCRuntime destroyed with a dangling API string");
+ // #endif
}
std::shared_ptr<const jsi::PreparedJavaScript> JSCRuntime::prepareJavaScript(For android if you're using jsc:jsc for android seems to be distributed as an aar build, so patching the cpp file will do nothing... so our alternatives are: cry, distribute jsc without the asserts ourselves, fix hermes, fix our date libs to work with hermes or fix reanimated. I'll stick with the first one until I find a way to make this work. At least it seems to be isolated to non release builds.. 😪 |
This thread has been open for over a year now and there is still no real solution in sight, so a clear answer as to whether JSC will be supported in the future would indeed be welcome.. In the meantime I would like to understand the suggested patch a little bit more. Is it safe to apply it for production or should it only be used in development? What I also don't understand is that the error only seems to occur in Debug-Builds, but the patch seems to remove code that is only relevant for release-builds (#ifndef NDEBUG -> only relevant if NOT DEBUG, right?)? Can anyone explain this to me? |
## **Description** This PR fixes reload crash on IOS adding a patch found here: software-mansion/react-native-reanimated#4783 (comment) https://github.com/user-attachments/assets/f5e39b2e-ed33-446f-9837-56ce1fc54688 ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Cal Leung <[email protected]>
and others
Description
After upgrading to react-native-reanimated v3, we noticed that app crashes on reload in development mode with JSC engine. It's reproducible in
Example/app in react-native-reanimated repo and can confirm it does not happen with hermes engine enabled.Steps to reproduce
:hermes_enabled => flags[:hermes_enabled], :fabric_enabled => flags[:fabric_enabled],to
:hermes_enabled => false, :fabric_enabled => false,pod installcommand fromExample/iosfolderThank you :raised:
Snack or a link to a repository
https://github.com/ykliuiev/reanimated-crash-repro
Reanimated version
3.3.0
React Native version
0.72.3
Platforms
iOS
JavaScript runtime
JSC
Workflow
React Native (without Expo)
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: