Reentrancy Crash in HermesExecutor.cpp

[nitish24p]

Description

I am creating a JSI function to get some values from MMKV (key value store) as a callback (Basically non blocking call. The code i have metioned below works fine when i have my JSC runtime but gives the error when i use hermes engine. I have forked the react-native-mmkv library and added my own method to it to run some code in a background thread.

When i run the same code via JSC the code seems to be working. I have also tried this with react-native version 0.68.5 and with RN 0.66 but got same error

Version

0.68.5

Output of npx react-native info

System:
OS: macOS 12.3.1
CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
Memory: 57.34 MB / 16.00 GB
Shell: 5.8 - /bin/zsh
Binaries:
Node: 12.13.0 - /usr/local/bin/node
Yarn: 1.22.19 - /usr/local/bin/yarn
npm: 6.12.0 - /usr/local/bin/npm
Watchman: 2022.11.14.00 - /usr/local/bin/watchman
Managers:
CocoaPods: 1.11.3 - /usr/local/bin/pod
SDKs:
iOS SDK:
Platforms: DriverKit 21.4, iOS 15.4, macOS 12.3, tvOS 15.4, watchOS 8.5
Android SDK:
API Levels: 25, 28, 29, 30, 31, 32
Build Tools: 27.0.3, 28.0.3, 30.0.2, 30.0.3, 31.0.0, 32.0.0, 32.1.0
System Images: android-25 | Google APIs Intel x86 Atom_64, android-29 | Google APIs Intel x86 Atom, android-29 | Google Play Intel x86 Atom, android-30 | Google Play Intel x86 Atom, android-Tiramisu | Google APIs Intel x86 Atom_64
Android NDK: Not Found
IDEs:
Android Studio: 2021.3 AI-213.7172.25.2113.9123335
Xcode: 13.3/13E113 - /usr/bin/xcodebuild
Languages:
Java: 11.0.8 - /usr/bin/javac
npmPackages:
@react-native-community/cli: Not Found
react: 17.0.2 => 17.0.2
react-native: 0.68.5 => 0.68.5
react-native-macos: Not Found
npmGlobalPackages:
react-native: Not Found

Steps to reproduce

Mentioned in code above

Snack, code example, screenshot, or link to a repository

if (propName == "getStringWithCallback") {
          // MMKV.getString(key: string)
          return jsi::Function::createFromHostFunction(runtime,
                                                       jsi::PropNameID::forAscii(runtime, funcName),
                                                       2,  // key
                                                       [this](jsi::Runtime& runtime,
                                                              const jsi::Value& thisValue,
                                                              const jsi::Value* arguments,
                          
                                                              size_t count) -> jsi::Value {
            if (!arguments[0].isString()) {
              throw jsi::JSError(runtime, "First argument ('key') has to be of type string!");
            }

              auto userCallbackRef = std::make_shared<jsi::Object>(arguments[1].getObject(runtime));
              auto keyName = convertJSIStringToNSString(runtime, arguments[0].getString(runtime));
              dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^(void){

                  auto value = [instance getStringForKey:keyName];
                  if (value != nil) {
                      userCallbackRef->asFunction(runtime).call(runtime, convertNSStringToJSIString(runtime, value)); // CODE BREAKS HERE
          
                  } else {
                      userCallbackRef->asFunction(runtime).call(runtime, jsi::Value::undefined());
                  }
              });
              return jsi::Value::undefined();
          });
        }
    

HermesExecutorFactory.cpp

  void before() {
    std::thread::id this_id = std::this_thread::get_id();
    std::thread::id expected = std::thread::id();

    // A note on memory ordering: the main purpose of these checks is
    // to observe a before/before race, without an intervening after.
    // This will be detected by the compare_exchange_strong atomicity
    // properties, regardless of memory order.
    //
    // For everything else, it is easiest to think of 'depth' as a
    // proxy for any access made inside the VM.  If access to depth
    // are reordered incorrectly, the same could be true of any other
    // operation made by the VM.  In fact, using acquire/release
    // memory ordering could create barriers which mask a programmer
    // error.  So, we use relaxed memory order, to avoid masking
    // actual ordering errors.  Although, in practice, ordering errors
    // of this sort would be surprising, because the decorator would
    // need to call after() without before().

    if (tid.compare_exchange_strong(
            expected, this_id, std::memory_order_relaxed)) {
      // Returns true if tid and expected were the same.  If they
      // were, then the stored tid referred to no thread, and we
      // atomically saved this thread's tid.  Now increment depth.
      assert(depth == 0 && "No thread id, but depth != 0");
      ++depth;
    } else if (expected == this_id) {
      // If the stored tid referred to a thread, expected was set to
      // that value.  If that value is this thread's tid, that's ok,
      // just increment depth again.
      assert(depth != 0 && "Thread id was set, but depth == 0");
      ++depth;
    } else {
      // The stored tid was some other thread.  This indicates a bad
      // programmer error, where VM methods were called on two
      // different threads unsafely.  Fail fast (and hard) so the
      // crash can be analyzed.
      __builtin_trap(); /// <- BREAKS HERE
    }
  }

CRASH LOGS

Translated Report (Full Report Below)
-------------------------------------

Incident Identifier: 4A508795-EFC7-40CE-A6E7-245E096AAC80
CrashReporter Key:   AB1128B5-C66F-8F4F-221D-7368D4CA148C
Hardware Model:      MacBookPro16,1
Process:             MmkvExample [21267]
Path:                /Users/USER/Library/Developer/CoreSimulator/Devices/8C2359C6-756F-47F7-AC17-E01360C33957/data/Containers/Bundle/Application/0FE3606E-D2CA-480B-8BFA-61FBE59F7A95/MmkvExample.app/MmkvExample
Identifier:          com.example.reactnativemmkv
Version:             1.0 (1)
Code Type:           X86-64 (Native)
Role:                Foreground
Parent Process:      launchd_sim [10758]
Coalition:           com.apple.CoreSimulator.SimDevice.8C2359C6-756F-47F7-AC17-E01360C33957 [3744]
Responsible Process: SimulatorTrampoline [4326]

Date/Time:           2022-12-26 19:47:29.3516 +0530
Launch Time:         2022-12-26 19:47:27.8556 +0530
OS Version:          macOS 12.3.1 (21E258)
Release Type:        User
Report Version:      104

Exception Type:  EXC_BAD_INSTRUCTION (SIGILL)
Exception Codes: 0x0000000000000001, 0x0000000000000000
Exception Note:  EXC_CORPSE_NOTIFY
Termination Reason: SIGNAL 4 Illegal instruction: 4
Terminating Process: exc handler [21267]

Triggered by Thread:  1

Application Specific Information:
CoreSimulator 802.6 - Device: iPhone 12 Pro (8C2359C6-756F-47F7-AC17-E01360C33957) - Runtime: iOS 15.4 (19E240) - DeviceType: iPhone 12 Pro
dyld4 config: DYLD_ROOT_PATH=/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot
dyld4 config: DYLD_ROOT_PATH=/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot


Thread 0::  Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib        	       0x10cfd897a mach_msg_trap + 10
1   libsystem_kernel.dylib        	       0x10cfd8ce8 mach_msg + 56
2   CoreFoundation                	       0x10b8fae58 __CFRunLoopServiceMachPort + 319
3   CoreFoundation                	       0x10b8f546e __CFRunLoopRun + 1249
4   CoreFoundation                	       0x10b8f4a90 CFRunLoopRunSpecific + 562
5   GraphicsServices              	       0x10cf7dc8e GSEventRunModal + 139
6   UIKitCore                     	       0x11f21690e -[UIApplication _run] + 928
7   UIKitCore                     	       0x11f21b569 UIApplicationMain + 101
8   MmkvExample                   	       0x107c583c8 main + 104 (main.m:14)
9   dyld_sim                      	       0x109f2bf21 start_sim + 10
10  dyld                          	       0x10fa8351e start + 462

Thread 1 Crashed::  Dispatch queue: com.apple.root.default-qos
0   MmkvExample                   	       0x1080de692 facebook::react::(anonymous namespace)::ReentrancyCheck::before() + 322 (HermesExecutorFactory.cpp:123)
1   MmkvExample                   	       0x1080de545 facebook::jsi::detail::BeforeCaller<facebook::react::(anonymous namespace)::ReentrancyCheck, void>::before(facebook::react::(anonymous namespace)::ReentrancyCheck&) + 21 (decorator.h:415)
2   MmkvExample                   	       0x1080de523 facebook::jsi::WithRuntimeDecorator<facebook::react::(anonymous namespace)::ReentrancyCheck, facebook::jsi::Runtime, facebook::jsi::Runtime>::Around::Around(facebook::react::(anonymous namespace)::ReentrancyCheck&) + 35 (decorator.h:740)
3   MmkvExample                   	       0x1080de4cd facebook::jsi::WithRuntimeDecorator<facebook::react::(anonymous namespace)::ReentrancyCheck, facebook::jsi::Runtime, facebook::jsi::Runtime>::Around::Around(facebook::react::(anonymous namespace)::ReentrancyCheck&) + 29 (decorator.h:739)
4   MmkvExample                   	       0x1080d5b65 facebook::jsi::WithRuntimeDecorator<facebook::react::(anonymous namespace)::ReentrancyCheck, facebook::jsi::Runtime, facebook::jsi::Runtime>::isFunction(facebook::jsi::Object const&) const + 37 (decorator.h:636)
5   MmkvExample                   	       0x10817ee61 facebook::jsi::Object::isFunction(facebook::jsi::Runtime&) const + 33 (jsi.h:622)
6   MmkvExample                   	       0x10817f49c facebook::jsi::Object::asFunction(facebook::jsi::Runtime&) const & + 60 (jsi.cpp:202)
7   MmkvExample                   	       0x10828485e invocation function for block in MmkvHostObject::get(facebook::jsi::Runtime&, facebook::jsi::PropNameID const&)::$_3::operator()(facebook::jsi::Runtime&, facebook::jsi::Value const&, facebook::jsi::Value const*, unsigned long) const + 110 (MmkvHostObject.mm:178)
8   libdispatch.dylib             	       0x10b7918e4 _dispatch_call_block_and_release + 12
9   libdispatch.dylib             	       0x10b792b25 _dispatch_client_callout + 8
10  libdispatch.dylib             	       0x10b79531e _dispatch_queue_override_invoke + 835
11  libdispatch.dylib             	       0x10b7a3310 _dispatch_root_queue_drain + 424
12  libdispatch.dylib             	       0x10b7a3c5e _dispatch_worker_thread2 + 155
13  libsystem_pthread.dylib       	       0x10ce4ef8a _pthread_wqthread + 256
14  libsystem_pthread.dylib       	       0x10ce4df57 start_wqthread + 15

Javascript code

  React.useEffect(() => {
    storage.getStringWithCallback('foo', (val) => {
      console.log(val);
    });
  }, []);

[nitish24p]

Okay i think i need to have access to the js invoker here and then call invoke Async and this should be fine then

[BlackCat1397]

@nitish24p Any updates?)

[iwater]

@nitish24p Any updates?

[nitish24p]

https://github.com/nitish24p/react-native-multithreadding-jsi/blob/main/cpp/react-native-multithreadding-lite.cpp#L34
I ended up doing something like this, although on a different repo, but the idea was the same, you need access to the js Callinvoker, that can be done by adjusting your podspec file to include the CallInvoker headers, and then you should have access to the jsCallInvoker on the bridge

[iwater]

#facebook/hermes#1011

[redpanda-bit]

https://github.com/nitish24p/react-native-multithreadding-jsi/blob/main/cpp/react-native-multithreadding-lite.cpp#L34 I ended up doing something like this, although on a different repo, but the idea was the same, you need access to the js Callinvoker, that can be done by adjusting your podspec file to include the CallInvoker headers, and then you should have access to the jsCallInvoker on the bridge

Hey @nitish24p how did you learn about CallInvoker and threading related to react native? This seems very advanced.
Also, why did you choose to create global variables for spawnTask and spawnTaskAsync instead of adding two new functions similar to multiply?
Thanks.

[nitish24p]

@carlosalmonte04 i was experimenting with JSI, i learnt about JSI mostly through the following resources

1. https://blog.notesnook.com/getting-started-react-native-jsi/
2. https://github.com/ospfranco/react-native-quick-sqlite
3. Youtube Series, This is the best available out there on the internet

If you see multiple, doesnt need JSI runtime as an arg, its a pure function returning an output, in my case i need the jsi runtime to accces variables passed to the main jsi function hence added it inside the install function

[cortinico]

similar issue here #38059

[numandev1]

In my case, somehow RAM space becomes full, it mostly happens on older iPhone devices like iPhone 8 or X, I have to optimize to make RAM by code refactoring

[github-actions]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[github-actions]

This issue was closed because it has been stalled for 7 days with no activity.