-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
C++ STL Library on Android does not support exception_ptr
#48027
Comments
Warning Could not parse version: We could not find or parse the version number of React Native in your issue report. Please use the template, and report your version including major, minor, and patch numbers - e.g. 0.76.2. |
Warning Could not parse version: We could not find or parse the version number of React Native in your issue report. Please use the template, and report your version including major, minor, and patch numbers - e.g. 0.76.2. |
The react-native/packages/react-native/ReactAndroid/src/main/jni/react/jni/CMakeLists.txt Lines 11 to 14 in 91e217f
I also added |
exception_ptr
exception_ptr
Nope. We are using libc++. When we bumped to NDK 26, this got bumped to LLVM 17. IIRC latest beta NDK uses libc++ based on LLVM 19. All of the build logic we control should also have RTTI enabled I think. I could imagine weirdness happening if a library is using static STL, and RN is using shared STL (so we have different copies of exception types/ODR issues), or if NDK is setting some special flags that XCode isn't. IIRC libc++ had some different ways of being able to implement RTTI, which we do not fuss with ourselves. |
hm, I can't seem to pinpoint it. It's definitely broken in a blank app created with the expo 52 template bootstrapper. |
@mrousavy have you tried on a vanilla Android App without React Native? |
@cortinico Yep, in a blank new Android C++ template it works: |
We would have to diff the compilation flag you're using for your react-native/packages/react-native/ReactAndroid/cmake-utils/ReactNative-application.cmake Lines 53 to 67 in 67bff87
|
I added all of those compile options to the
|
I also added the options from here: react-native/packages/react-native/ReactAndroid/build.gradle.kts Lines 529 to 547 in 67bff87
..and it still works. My guess is that it's caused by some of the libraries being linked having a different C++ STL shipped with them? react-native/packages/react-native/ReactAndroid/cmake-utils/ReactNative-application.cmake Lines 70 to 81 in 67bff87
|
I verified that my changes actually worked by replacing What I found weird is that it still prints the correct exception message after I set |
FBJNI definitely ships with a libc++ in shared linking: We'll have to make sure the NDK version used is the same as we recently updated fbjni, hermes, and RN to use NDK 27. If you happen to use RN 0.76 (on NDK 26) but a recent version of fbjni (build with NDK 27) that could be reason |
Well I am using react-native 0.76.3 - so that's probably pinning a proper fbjni version, no? That should be NDK 26. |
I'll try to upgrade to RN 0.77 |
Have you tried on a non-expo project? Also Fresco could be the culprit here |
Yes, Nitro's I'm upgrading to RN 77 right now (mrousavy/nitro#383) |
Let me know if this solves it however 77 RC0 is pretty unstable |
Great, then I have no idea why this is happening. Not sure how to help further. I also checked the Android NDK and Google Issue tracker, but there is no mention of anything like that |
Thanks for your help so far Nico! I'll try what I can do, I spent a few hours too many on this already so I'm not sure how far I can take debugging this. |
What's the deal with SoLoader - do we still need that? I thought it's only for pre-Android 23, and we're past that now. I have a feeling that SoLoader's lib merging thingy might cause some issues here... |
Didn't we change the second arg to Might be the cause of the issue..? 👀 |
Yeah we changed now because we implemented Native Library Merging: Is |
Ok I just tried in RN 0.75, and it also doesn't work there: |
Ok so at least is not the So Merging that caused this. Still the whole SoLoader infrastructure might affect this, but I'm not sure how given that SoLoader doesn't bring in any native code. |
Here's a deep dive on how LLVM matches these: https://github.com/llvm/llvm-project/blob/3c83054bec3326ccf338eeda56e67e8cd83a3b2a/llvm/docs/ExceptionHandling.rst#trycatch And here is where it rethrows: https://github.com/llvm/llvm-project/blob/ca3180ad6e39304177deac112bd78739d85fe32b/libcxx/src/support/runtime/exception_pointer_cxxabi.ipp#L58 Notably, libc++ with NDK does not use I would recommend creating an issue on the NDK repo, as it may gain more traction (and I think is closer to the source of the bug). |
But it worked in a blank Android app, it only fails in a blank react-native app 🤔 I'll try to use the same NDK version as in react-native to confirm that it is not just an NDK version issue. |
Last factor I could think of… does it happen in a blank Android app using shared stl? |
Yep, as mentioned here, I created a blank Android app and copied the compile & build options from build.gradle & CMake over, including Still worked there. |
Ha, I just ran into this. Leaving a message here for visibility but there is definitely something broken and no mention on the internet of this behavior. |
The OP asked about this issue on StackOverflow - If I could I would close this issue as invalid. This is not an issue of the library, the behavior is expected, this is the invalid OP's code that relies on implementation defined behavior. Some
support.exception#propagation-10
The OP catches the exception of unspecified type |
@sergio-nsk so how would the correct code look like then? Assuming I want to use |
Imagine what you can do with an exception object in the
try {
throw std::runtime_error("Test exception");
} catch (const std::exception& e) {
auto eptr = std::current_exception();
try {
std::rethrow_exception(eptr);
} catch (const std::exception& e) {
__android_log_print(ANDROID_LOG_ERROR, "EXCEPTION", "Caught std::exception: %s", e.what());
} catch (...) {
__android_log_print(ANDROID_LOG_ERROR, "EXCEPTION", "Caught unknown exception!!");
}
} |
Did you try your code? In a blank new Android app, this code works as expected (catches the In a blank new React Native app (0.76.5), adding this code to Afaik I used the same NDK version, Shared C++ STL, and same gradle versions, but I'd need to double-check all of that to confirm.
I disagree. I think this is a bug in react-native. |
Could it be possible that some library react-native depends on has different symbols or typeinfos for |
Here is a comment saying that on hermes std::exception has a different typedef, maybe that is causing the issue: |
I don't need to do that when I see an invalid code.
This behavior is expected, they use different C++ runtimes. |
@sergio-nsk did you read my reply? Throwing a try {
throw std::runtime_error("test");
} catch (const std::exception&) {
// do nothing
} This crashes the app because the I am not Bjarne Stroustrup, but I believe this is not how C++ should work. |
Which reply? You have never shown such code. You have shown a different one try {
std::rethrow_exception(eptr);
} catch (const std::exception& e) { |
That's your code, lol. But we're going off-topic here. The behaviour is not working as expected, there's a bug, and it's not "invalid". I'll continue my investigations to see why this is happening soon. |
Can anyone please run |
I took a bit of time to try to pin-point the root cause of this. I used the rn-tester app to reproduce this:
Results
I'm not sure we can acquit "so merging" just yet. Line 19 in 44705fe
I tried adding Testing with "so merging" disabledIs there an "easy" way to revert or disable the "so merging" behavior to test the hypothesis that this is the culprit? react-native/packages/react-native/ReactAndroid/src/main/jni/react/jni/CMakeLists.txt Line 27 in 44705fe
|
@tmikov can you help craft a command I can easily run on my Mac? I tried
This is the result of running
It does list |
@kraenhansen nope sadly there is no easy way to do so. You'll have to build from source, revert all the |
Description
For some reason,
std::exception_ptr
is broken in react-native.I have added this code to
react-native/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp
:And it logs
If you run the same code on iOS (replace log with
NSLog
), it works as expected:So for some reason, the Android C++ lib fails to properly rethrow
std::exception_ptr
. ❌iOS uses libc++ since ever.
Android uses libc++ since 2015, and apparently used libstdc++ (from GNU) before 2015 - could it be that libstdc++ is still used today somehow?
From what I'm seeing in RN's
build.gradle
, it correctly uses libc++:react-native/packages/react-native/ReactAndroid/build.gradle.kts
Line 535 in 91e217f
Anyways... The code above does not work as expected. I think I read somewhere that
exception_ptr
does not work on ARMv5 since it needs some compare instruction that isn't available there, but afaik my device is ARMv8.I have confirmed that;
Steps to reproduce
npx create-expo-app@latest
)npx expo prebuild
OnLoad.cpp
npx expo android
React Native Version
0.76.2
Affected Platforms
Runtime - Android
Output of
npx react-native info
Stacktrace or Logs
Reproducer
https://github.com/mrousavy/react-native-exception_ptr-bug
Screenshots and Videos
No response
The text was updated successfully, but these errors were encountered: