JSI fails to pass arguments js->c++ when on v8

[OzymandiasTheGreat]

We have a large project using a lot of JSI C++ code. We've previously used Hermes and it all worked fine. To make interoperability between our node code and react native better we'd like to switch to v8.

So I followed the readme to a T and set up v8 (no INTL, no JIT). Everything seemed fine at first, but when calling native code our app crashed with SIGABORT. After some digging I traced this to failing assert(string->IsString()). Some more digging and the string in question is undefined.

After short-circuiting every possible path and leaving just one function, I found that no arguments are passed from javascript to C++. Like, no matter what or how many args I pass from js, on c++ argument count is always 0 and values are undefined.

Now my first guess is that there's something wrong with our project configuration or that v8 is improperly linked. However, I have no clue when it comes to gradle and our gradle person is on vacation.

I know this is off-topic, but could you help me out? Any idea what could cause JSI to fail like this?

[Kudo]

hi there! i am happy to help, but it's hard for me to investigate a problem i cannot reproduce. could you help to provide a minimal reproducible example for me to further investigate?

[OzymandiasTheGreat]

Thank you! That's the thing tho, outside of our big project with custom configs everything works as expected :sweat_smile: I'll try once more by copying our configs into a fresh project and let you know how it goes.

[OzymandiasTheGreat]

So my plan didn't work out, but then I tried the reductive approach. Here's a sample as minimal as I could get it: https://github.com/OzymandiasTheGreat/v8-error

I started removing bits and testing 'till I could keep the error but have no extras. This also allowed me to narrow the problem to libuv setup method in JSI bindings (cpp/uv/uv.cpp:131). It basically just sets up a separate thread to run uv main loop in. However after setting up the thread I can no longer pass arguments from javascript to c++. If I install the bindings, but don't call setup() (no thread) arguments are passed just fine. However my bindings don't work anymore :sweat_smile: Also this works on Hermes just fine, so I guess v8 really doesn't like something about our threads :laughing:

[Kudo]

sorry i have been late to response this issue. the reproducible example is great.

as far as i can tell for issue, because the return statement here, any calls to __hlp_raf.ANYFUNCTION() (including the __hlp_raf.setup()) will go to this statement and it expects the first argument to be string.

so these patch to your js code should make it work:

diff --git a/App.tsx b/App.tsx
index 32eafd7..cc81011 100644
--- a/App.tsx
+++ b/App.tsx
@@ -4,8 +4,8 @@ import { getRAFBindings } from './binding/native'
 const App = () => {
   useEffect(() => {
     const raf = getRAFBindings()
-    // console.log((raf as any).raf_stat('/data/data/com.explibuv/cache'))
-    console.log((raf as any).raf_stat(13))
+    console.log((raf as any).raf_stat('/data/data/com.explibuv/cache'))
+    // console.log((raf as any).raf_stat(13))
   })
   return null
 }
diff --git a/binding/native.ts b/binding/native.ts
index 5e782db..c700fa3 100644
--- a/binding/native.ts
+++ b/binding/native.ts
@@ -6,7 +6,7 @@ export const RAF_BASE: string = HolepunchBindings.RAF_BASE
 export const RAF_TMP_BASE: string = HolepunchBindings.RAF_TMP_BASE

 __hlp_uv.setup()
-__hlp_raf.setup()
+__hlp_raf.setup("SHOULD PASS A STRING")
 __hlp_thrower = (err) => {
   throw err
 }

i am not sure whether this is exactly your original issue. please share more insight if i misunderstand your problem. thank you!

[OzymandiasTheGreat]

Oh boy, this is bigger than I thought. See, your workaround suppresses the error I'm getting, but raf_stat just returns it's arguments now. It's supposed to use raf_stat c++ implementation which calls libuv stat method. So basically all of our JSI implementations are replaced by a function that just returns it's arguments.

This works correctly on hermes, and I think jsc too, but it's been a while since I tried that.

It also works correctly if we don't start uv loop in separate thread, but then uv methods don't work obviously.

[Kudo]

could you share some insight how to test your example?

as i mentioned previous, because the return statement, you just return the first argument as string. that's why raf_stat returns its arguments.

i also tested on hermes for your original example and it crashed. the stackstace broke on jsi::Value::asString, that is pretty much like the v8 assert(string->IsString()). so i think i may test in wrong way or the example might have some problems.

                   libc  F  Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0 in tid 9053 (mqt_js), pid 8999 (com.explibuv)
                  DEBUG  F  #00 pc 000000000000de94  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libjsi.so (facebook::jsi::Value::asString(facebook::jsi::Runtime&) const &+40) (BuildId: df60812d8f71574e0d439b38d66bbed72b72467a)
                         F  #01 pc 0000000000039060  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhlp_bindings.so (BuildId: 2e9d55b2ce8f361dfb36de90dc32290bed759947)
                         F  #02 pc 0000000000038ff4  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhlp_bindings.so (BuildId: 2e9d55b2ce8f361dfb36de90dc32290bed759947)
                         F  #03 pc 0000000000038f38  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhlp_bindings.so (BuildId: 2e9d55b2ce8f361dfb36de90dc32290bed759947)
                         F  #04 pc 0000000000038e98  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhlp_bindings.so (BuildId: 2e9d55b2ce8f361dfb36de90dc32290bed759947)
                         F  #05 pc 0000000000037b64  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhlp_bindings.so (BuildId: 2e9d55b2ce8f361dfb36de90dc32290bed759947)
                         F  #06 pc 00000000002de890  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhermes-executor-debug.so (BuildId: e44ac14c16d2e6209157fd30f6a58213a0d5aaba)
                         F  #07 pc 00000000002de794  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhermes-executor-debug.so (std::__ndk1::function<facebook::jsi::Value (facebook::jsi::Runtime&, facebook::jsi::Value const&, facebook::jsi::Value const*, unsigned lo
                            ng)>::operator()(facebook::jsi::Runtime&, facebook::jsi::Value const&, facebook::jsi::Value const*, unsigned long) const+156) (BuildId: e44ac14c16d2e6209157fd30f6a58213a0d5aaba)
                         F  #08 pc 00000000002de6e8  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhermes-executor-debug.so (facebook::jsi::DecoratedHostFunction::operator()(facebook::jsi::Runtime&, facebook::jsi::Value const&, facebook::jsi::Value const*, unsign
                            ed long)+96) (BuildId: e44ac14c16d2e6209157fd30f6a58213a0d5aaba)
                         F  #09 pc 00000000002de664  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhermes-executor-debug.so (BuildId: e44ac14c16d2e6209157fd30f6a58213a0d5aaba)
                         F  #10 pc 00000000002de5a8  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhermes-executor-debug.so (facebook::jsi::Value std::__ndk1::__invoke_void_return_wrapper<facebook::jsi::Value>::__call<facebook::jsi::DecoratedHostFunction&, facebo
                            ok::jsi::Runtime&, facebook::jsi::Value const&, facebook::jsi::Value const*, unsigned long>(facebook::jsi::DecoratedHostFunction&, facebook::jsi::Runtime&, facebook::jsi::Value const&, facebook::jsi::Value const*&&, unsigned long&&)+144) (BuildId: e44ac14c16d2e6209157fd30f6a5
                            8213a0d5aaba)
                         F  #11 pc 00000000002de4cc  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhermes-executor-debug.so (BuildId: e44ac14c16d2e6209157fd30f6a58213a0d5aaba)
                         F  #12 pc 00000000002dd110  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhermes-executor-debug.so (std::__ndk1::__function::__func<facebook::jsi::DecoratedHostFunction, std::__ndk1::allocator<facebook::jsi::DecoratedHostFunction>, facebo
                            ok::jsi::Value (facebook::jsi::Runtime&, facebook::jsi::Value const&, facebook::jsi::Value const*, unsigned long)>::operator()(facebook::jsi::Runtime&, facebook::jsi::Value const&, facebook::jsi::Value const*&&, unsigned long&&)+148) (BuildId: e44ac14c16d2e6209157fd30f6a58213
                            a0d5aaba)
                         F  #13 pc 0000000000075340  /data/app/~~Qn4MsSW2NKf8zI45dZHfOA==/com.explibuv-_L56B4eRC8hegDpxReI5Ig==/base.apk!libhermes.so (BuildId: 12490b53a3532c9b8f4cdfe302922ded6e9a2573)

[OzymandiasTheGreat]

Sorry for the delay, been swamped with other things. Also, sorry, I messed up the minimal repro and got confused about it 😅

So we updated react-native version and I figured out that when we use macros in c++ debugger gives incorrect or at least incomplete stack trace.

So I remade the minimal repro, made sure it runs on hermes, and tried adding v8 again. Everything above is irrelevant, either because I misunderstood the nature of the error, or because it got fixed in the upgrade. The repo for the repro is the same, I overwrote it with correct code now.

So basically what happens, is when we want to throw error JS side, we call __hlp_thrower from c++. It's a very simple function, it receives JS error object from c++ and throws it.

On hermes this works as expected, but on v8 the app crashes with SIGABRT because assertion string->IsString fails. It seems, and I may be wrong here, v8 treats error object as string and this obviously fails.

So yeah, I reported the error wrong, because the macros with incomplete stack traces had me confused. Could you please look into this again? If you want to try hermes to see how it works just go a couple of commits back in repo. This time I made sure the repro is actually representative of the error.

[Kudo]

that's a great repro, thanks @OzymandiasTheGreat!

i get to find a problem in my implementation. before publishing a fix, i would like to have your verification and make sure everything works as expected. could you apply the following patch and try again locally? you could use patch-package to apply to patch or simply edit the file.

patches/react-native-v8+1.5.2.patch

diff --git a/node_modules/react-native-v8/src/v8runtime/V8Runtime.cpp b/node_modules/react-native-v8/src/v8runtime/V8Runtime.cpp
index 7d4f4eb..7b85e75 100644
--- a/node_modules/react-native-v8/src/v8runtime/V8Runtime.cpp
+++ b/node_modules/react-native-v8/src/v8runtime/V8Runtime.cpp
@@ -215,8 +215,10 @@ jsi::Value V8Runtime::ExecuteScript(
 void V8Runtime::ReportException(v8::Isolate *isolate, v8::TryCatch *tryCatch)
     const {
   v8::HandleScope scopedHandle(isolate);
+  v8::Local<v8::Context> context(isolate->GetCurrentContext());
+  v8::Local<v8::Value> exceptionValue = tryCatch->Exception()->ToString(context).ToLocalChecked();
   std::string exception =
-      JSIV8ValueConverter::ToSTLString(isolate, tryCatch->Exception());
+      JSIV8ValueConverter::ToSTLString(isolate, exceptionValue);
   v8::Local<v8::Message> message = tryCatch->Message();
   if (message.IsEmpty()) {
     // V8 didn't provide any extra information about this error; just
@@ -225,7 +227,6 @@ void V8Runtime::ReportException(v8::Isolate *isolate, v8::TryCatch *tryCatch)
     return;
   } else {
     std::ostringstream ss;
-    v8::Local<v8::Context> context(isolate->GetCurrentContext());

     // Print (filename):(line number): (message).
     ss << JSIV8ValueConverter::ToSTLString(

[OzymandiasTheGreat]

I'm sorry, I can't test your patch. I spent all day today trying to do an android build, and it crashes on startup with couldn't find DSO to load: libv8executor.so caused by: couldn't find DSO to load: libv8android.so The thing is this happens without the patch as well, I can't even build fresh clone from github 😞

[capezzbr]

I'm sorry, I can't test your patch.

@OzymandiasTheGreat to fix that error run cd app/android && ./gradlew extractSOFiles && cd - and build the Android app again

[capezzbr]

@Kudo I've applied your patch, and the crash now doesn't seem to be coming from V8 anymore, but the app does still crash

2022-11-05 01:15:48.161 12164-12302 ReactNativeJS           com.myapp123.android                 E  ReferenceError: NavigationContainer is not defined

                                                                                                    This error is located at:
                                                                                                        in Root
                                                                                                        in RCTView (created by View)
                                                                                                        in View (created by AppContainer)
                                                                                                        in RCTView (created by View)
                                                                                                        in View (created by AppContainer)
                                                                                                        in AppContainer
                                                                                                        in app(RootComponent), js engine: v8
2022-11-05 01:15:48.174 12164-12302 libc                    com.myapp123.android                 A  Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x4 in tid 12302 (mqt_js), pid 12164 (oinbase.android)
2022-11-05 01:15:48.406 12320-12320 DEBUG                   pid-12320                            A  Cmdline: com.myapp123.android
2022-11-05 01:15:48.406 12320-12320 DEBUG                   pid-12320                            A  pid: 12164, tid: 12302, name: mqt_js  >>> com.myapp123.android <<<
2022-11-05 01:15:48.406 12320-12320 DEBUG                   pid-12320                            A        #05 pc 00000000000bcc0c  [anon:dalvik-classes2.dex extracted in memory from /data/app/~~1jFhbJQ6lsQXpntsbuWrFQ==/com.myapp123.android-XRDZSo4_mdt4HKTDlAHvTQ==/base.apk!classes2.dex] (com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage+0)
2022-11-05 01:15:48.406 12320-12320 DEBUG                   pid-12320                            A        #11 pc 00000000000bcd9a  [anon:dalvik-classes2.dex extracted in memory from /data/app/~~1jFhbJQ6lsQXpntsbuWrFQ==/com.myapp123.android-XRDZSo4_mdt4HKTDlAHvTQ==/base.apk!classes2.dex] (com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run+74)

I am also using a different test case, where I basically have a JS issue

hope this helps 👍