From dfd445cbc69c8bc6c5d1d3d7948472a0a3ae4927 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Thu, 25 May 2023 15:26:12 -0700 Subject: [PATCH] Terminate instead of throwing if TurboModule callback double-called (#37570) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/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: 680aa9aa5e4ca6d8dd04dfe34ec870b86c7640ef --- .../ReactCommon/react/nativemodule/core/CMakeLists.txt | 1 + .../react/nativemodule/core/ReactCommon/TurboCxxModule.cpp | 3 ++- .../core/platform/android/ReactCommon/JavaTurboModule.cpp | 4 ++-- .../core/platform/ios/React-NativeModulesApple.podspec | 1 + .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 3 ++- 5 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/CMakeLists.txt b/packages/react-native/ReactCommon/react/nativemodule/core/CMakeLists.txt index 909cd427f359da..33a3d9b1ef5006 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/CMakeLists.txt +++ b/packages/react-native/ReactCommon/react/nativemodule/core/CMakeLists.txt @@ -32,6 +32,7 @@ target_include_directories(react_nativemodule_core target_link_libraries(react_nativemodule_core fbjni folly_runtime + glog jsi react_bridging react_debug diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboCxxModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboCxxModule.cpp index 71a5d3b4e0f045..fbe27c1e3c5db1 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboCxxModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboCxxModule.cpp @@ -10,6 +10,7 @@ #include #include +#include #include using namespace facebook; @@ -24,7 +25,7 @@ CxxModule::Callback makeTurboCxxModuleCallback( return [weakWrapper, wrapperWasCalled = false](std::vector args) mutable { if (wrapperWasCalled) { - throw std::runtime_error("callback arg cannot be called more than once"); + LOG(FATAL) << "callback arg cannot be called more than once"; } auto strongWrapper = weakWrapper.lock(); diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 9f57f4d2c7f59b..616861539d1749 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include @@ -83,8 +84,7 @@ jni::local_ref createJavaCallbackFromJSIFunction( callbackWrapperOwner = std::move(callbackWrapperOwner), wrapperWasCalled = false](folly::dynamic responses) mutable { if (wrapperWasCalled) { - throw std::runtime_error( - "Callback arg cannot be called more than once"); + LOG(FATAL) << "callback arg cannot be called more than once"; } auto strongWrapper = weakWrapper.lock(); diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/React-NativeModulesApple.podspec b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/React-NativeModulesApple.podspec index 43668653d04140..8824ba73561bf3 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/React-NativeModulesApple.podspec +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/React-NativeModulesApple.podspec @@ -42,6 +42,7 @@ Pod::Spec.new do |s| s.source_files = "ReactCommon/**/*.{mm,cpp,h}" + s.dependency "glog" s.dependency "ReactCommon/turbomodule/core" s.dependency "ReactCommon/turbomodule/bridging" s.dependency "React-callinvoker" diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index f5bcde5867565b..7c4cb68787ac81 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -8,6 +8,7 @@ #import "RCTTurboModule.h" #import "RCTBlockGuard.h" +#include #import #import #import @@ -184,7 +185,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s BOOL __block wrapperWasCalled = NO; RCTResponseSenderBlock callback = ^(NSArray *responses) { if (wrapperWasCalled) { - throw std::runtime_error("callback arg cannot be called more than once"); + LOG(FATAL) << "callback arg cannot be called more than once"; } auto strongWrapper = weakWrapper.lock();