Skip to content

Commit

Permalink
Terminate instead of throwing if TurboModule callback double-called (#…
Browse files Browse the repository at this point in the history
…37570)

Summary:
Pull Request resolved: #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
  • Loading branch information
NickGerleman authored and facebook-github-bot committed May 25, 2023
1 parent 1691801 commit dfd445c
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 4 deletions.
Expand Up @@ -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
Expand Down
Expand Up @@ -10,6 +10,7 @@
#include <vector>

#include <ReactCommon/TurboModuleUtils.h>
#include <glog/logging.h>
#include <jsi/JSIDynamic.h>

using namespace facebook;
Expand All @@ -24,7 +25,7 @@ CxxModule::Callback makeTurboCxxModuleCallback(
return [weakWrapper,
wrapperWasCalled = false](std::vector<folly::dynamic> 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();
Expand Down
Expand Up @@ -10,6 +10,7 @@
#include <string>

#include <fbjni/fbjni.h>
#include <glog/logging.h>
#include <jsi/jsi.h>

#include <ReactCommon/TurboModule.h>
Expand Down Expand Up @@ -83,8 +84,7 @@ jni::local_ref<JCxxCallbackImpl::JavaPart> 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();
Expand Down
Expand Up @@ -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"
Expand Down
Expand Up @@ -8,6 +8,7 @@
#import "RCTTurboModule.h"
#import "RCTBlockGuard.h"

#include <glog/logging.h>
#import <objc/message.h>
#import <objc/runtime.h>
#import <atomic>
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit dfd445c

Please sign in to comment.