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

Terminate instead of throwing if TurboModule callback double-called #37570

Closed
wants to merge 1 commit into from

Conversation

NickGerleman
Copy link
Contributor

Summary:
If a TM calls a completion callback, or resolves or rejects a promise multiple times, a C++ exception is thrown.

For an in the wild crash, we are getting this signal as:

  1. libdispatch calls std::terminate while pumping a thread's message queue
  2. The hooked FB app termination handler is called
  3. Our termination handler introspects the currently thrown exception.
  4. The handler introspecting the currently thrown exception sees this C++ exception being thrown, suggesting libdispatch termination may be due to unhandled C++ exception
  5. We have lost the stack trace by this point of the code invoking the returned lambas

I think if we change the time of abort() to when the callback is called, we might be able to preserve the stack trace of module code calling the callback.

Differential Revision: D46175685

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels May 25, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46175685

@analysis-bot
Copy link

analysis-bot commented May 25, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,741,146 +987
android hermes armeabi-v7a 8,052,240 +971
android hermes x86 9,231,490 +1,125
android hermes x86_64 9,082,879 +880
android jsc arm64-v8a 9,303,090 +992
android jsc armeabi-v7a 8,491,763 +970
android jsc x86 9,364,505 +1,124
android jsc x86_64 9,620,002 +877

Base commit: d4f6cf1
Branch: main

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request May 25, 2023
…acebook#37570)

Summary:
Pull Request resolved: facebook#37570

If a TM calls a completion callback, or resolves or rejects a promise multiple times, a C++ exception is thrown.

For an in the wild crash, we are getting this signal as:
1. `libdispatch` calls std::terminate while pumping a thread's message queue
2. The hooked FB app termination handler is called, which introspects for the currently handled exception
4. We are handling this TurboModule C++ exception being thrown, suggesting `libdispatch` termination may be due to catching this C++ exception which was not otherwise handled
4. We have by this point lost the stack trace of the code invoking the callback

I think if we change the timing of `abort()` to when the callback is called, we might be able to preserve the stack trace of module code calling the callback.

Differential Revision: D46175685

fbshipit-source-id: 02494a7aed3ba8c105666281c879ff1805700c4f
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46175685

…acebook#37570)

Summary:
Pull Request resolved: facebook#37570

If a TM calls a completion callback, or resolves or rejects a promise multiple times, a C++ exception is thrown.

For an in the wild crash, we are getting this signal as:
1. `libdispatch` calls std::terminate while pumping a thread's message queue
2. The hooked FB app termination handler is called, which introspects for the currently handled exception
4. We are handling this TurboModule C++ exception being thrown, suggesting `libdispatch` termination may be due to catching this C++ exception which was not otherwise handled
4. We have by this point lost the stack trace of the code invoking the callback

I think if we change the timing of `abort()` to when the callback is called, we might be able to preserve the stack trace of module code calling the callback.

Reviewed By: javache

Differential Revision: D46175685

fbshipit-source-id: 5e53dda665bca0c221e4b6086209455a4d57cb36
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46175685

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label May 25, 2023
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in dfd445c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants