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

Calling JS callback asynchronously from C++ thread. #44813

Closed
AbeereSpark opened this issue Jun 6, 2024 · 16 comments
Closed

Calling JS callback asynchronously from C++ thread. #44813

AbeereSpark opened this issue Jun 6, 2024 · 16 comments
Labels
Newer Patch Available Resolution: Answered When the issue is resolved with a simple answer Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@AbeereSpark
Copy link

AbeereSpark commented Jun 6, 2024

Description

I am encountering an issue where my React Native app crashes when a JavaScript callback is called from C++ code running on a C++ thread asynchronously. However, when I call the same callback on the same thread from where the foo function is called, it works as expected without crashing.

Steps to reproduce

Nil

React Native Version

0.74.1

Affected Platforms

Runtime - Android, Build - Windows

Areas

TurboModule - The New Native Module System, JSI - Javascript Interface, Bridgeless - The New Initialization Flow

Output of npx react-native info

System:
  OS: Windows 10 10.0.19045
  CPU: (4) x64 Intel(R) Core(TM) i5-6500T CPU @ 2.50GHz
  Memory: 6.54 GB / 15.88 GB
Binaries:
  Node:
    version: 20.13.0
    path: E:\Program Files\nodejs\node.EXE
  Yarn:
    version: 3.6.1
    path: ~\AppData\Roaming\npm\yarn.CMD
  npm:
    version: 10.7.0
    path: E:\Program Files\nodejs\npm.CMD
  Watchman: Not Found
SDKs:
  Android SDK:
    API Levels:
      - "29"
      - "30"
      - "31"
      - "34"
    Build Tools:
      - 30.0.2
      - 30.0.3
      - 33.0.1
      - 34.0.0
    System Images:
      - android-30 | Intel x86_64 Atom
      - android-30 | Google Play Intel x86 Atom
      - android-30 | Google APIs ATD Intel x86_64 Atom
      - android-34 | Google APIs Intel x86_64 Atom
    Android NDK: Not Found
  Windows SDK: Not Found
IDEs:
  Android Studio: Not Found
  Visual Studio: Not Found
Languages:
  Java: 17.0.11
  Ruby: Not Found
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.74.1
    wanted: 0.74.1
  react-native-windows: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: false
  newArchEnabled: true
iOS:
  hermesEnabled: Not found
  newArchEnabled: Not found

Stacktrace or Logs

06-06 09:22:40.260 21837 21859 W libEGL  : EGLNativeWindowType 0xb40000786029bbc0 disconnect failed                     06-06 09:22:40.313 21837 22216 I MyApp   : Log message to Android                                                       06-06 09:22:40.367 21837 22216 I ReactNativeJS: 'error:', undefined, 'result:', 'hello world'                           06-06 09:22:40.369 21837 22216 E libc++abi: terminating due to uncaught exception of type facebook::jsi::JSError: Can't find variable: setHello                                                                                                 06-06 09:22:40.369 21837 22216 E libc++abi:                                                                             06-06 09:22:40.369 21837 22216 E libc++abi: callback                                                                    06-06 09:22:40.369 21837 22216 E libc++abi: onFastRefresh@http://localhost:8081/index.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.bikegadgetmainappexample&modulesOnly=false&runModule=true:44263:35                        06-06 09:22:40.369 21837 22216 E libc++abi: performReactRefresh@http://localhost:8081/index.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.bikegadgetmainappexample&modulesOnly=false&runModule=true:62080:34                  06-06 09:22:40.369 21837 22216 E libc++abi: http://localhost:8081/index.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.bikegadgetmainappexample&modulesOnly=false&runModule=true:435:40                                        --------- beginning of crash                                                                                            06-06 09:22:40.370 21837 22216 F libc    : Fatal signal 6 (SIGABRT), code -1 (SI_QUEUE) in tid 22216 (mqt_v_js), pid 21837 (tmainappexample)                                                                                                    06-06 09:22:40.472 22219 22219 I crash_dump64: obtaining output fd from tombstoned, type: kDebuggerdTombstone           06-06 09:22:40.473   249   249 I tombstoned: received crash request for pid 22216                                       06-06 09:22:40.481 22219 22219 I crash_dump64: performing dump of process 21837 (target tid = 22216)                    06-06 09:22:40.497 22219 22219 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***              06-06 09:22:40.498 22219 22219 F DEBUG   : Build fingerprint: 'Firefly/rk356x/rk356x:11/RD2A.211001.002/lwy07101153:userdebug/release-keys'                                                                                                     06-06 09:22:40.498 22219 22219 F DEBUG   : Revision: '0'                                                                06-06 09:22:40.498 22219 22219 F DEBUG   : ABI: 'arm64'                                                                 06-06 09:22:40.499 22219 22219 F DEBUG   : Timestamp: 2024-06-06 09:22:40+0000                                          06-06 09:22:40.499 22219 22219 F DEBUG   : pid: 21837, tid: 22216, name: mqt_v_js  >>> com.bikegadgetmainappexample <<< 06-06 09:22:40.499 22219 22219 F DEBUG   : uid: 10120                                                                   06-06 09:22:40.499 22219 22219 F DEBUG   : signal 6 (SIGABRT), code -1 (SI_QUEUE), fault addr --------                  06-06 09:22:40.499 22219 22219 F DEBUG   : Abort message: 'terminating due to uncaught exception of type facebook::jsi::JSError: Can't find variable: setHello

Reproducer

JSI-App

Screenshots and Videos

App.tsx

import * as React from 'react';
import { StyleSheet, View, Text } from 'react-native';
import { multiply, install} from 'react-native-bikegadget-mainapp';

const callback = (error, result) => {
  console.log('error:', error, 'result:', result);
  setHello("message from js");
}

export default function App() {
  const [result, setResult] = React.useState(multiply(4, 6));
  const [hello, setHello] = React.useState('');

  React.useEffect(() => {
    install();
    setHello(helloWorld());
    foo(callback);
    
    // callback("err123", "hello")
  }, []);

  return (
    <View style={styles.container}>
      <Text>Result: {result}</Text>
      <Text>Hello: {hello}</Text>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
  },
  box: {
    width: 60,
    height: 60,
    marginVertical: 20,
  },
});

C++ module

#include "react-native-bikegadget-mainapp.h"
#include <jsi/jsi.h>
#include <jsi/jsilib.h>
#include <thread>
#include <android/log.h>
#include <ReactCommon/CallInvoker.h>

using namespace facebook::jsi;
using namespace std;

namespace bikegadgetmainapp
{
	// Function to register helloWorld in JSI
	void registerHelloWorld(Runtime &jsiRuntime)
	{
		auto helloWorld = Function::createFromHostFunction(
			jsiRuntime,
			PropNameID::forAscii(jsiRuntime, "helloWorld"),
			0,
			[](Runtime &runtime,
			   const Value &thisValue,
			   const Value *arguments,
			   size_t count) -> Value
			{
				string helloworld = "helloworld Ash 123";
				return Value(runtime, String::createFromUtf8(runtime, helloworld));
			});

		jsiRuntime.global().setProperty(jsiRuntime, "helloWorld", std::move(helloWorld));
	}

	void registerFoo(Runtime &jsiRuntime)
	{
		auto foo = Function::createFromHostFunction(
			jsiRuntime,
			PropNameID::forAscii(jsiRuntime, "foo"),
			1,
			[](Runtime &runtime, const Value &thisValue, const Value *arguments, size_t count) -> Value
			{
				if (count < 1 || !arguments[0].isObject() || !arguments[0].getObject(runtime).isFunction(runtime))
				{
					throw JSError(runtime, "Expected a function as the first argument");
				}

				// Create a shared reference to the user callback
				auto userCallbackRef = std::make_shared<Object>(arguments[0].getObject(runtime));

				// Lambda to be run in the new thread
				auto f = [&runtime, userCallbackRef]()
				{
					auto val = std::make_shared<std::string>("hello world");
					auto error = std::make_shared<Value>(Value::undefined());

					// Periodically log messages to Android log and sleep for 5 seconds
					while (true)
					{
						__android_log_print(ANDROID_LOG_INFO, "MyApp", "Log message to Android");
                        userCallbackRef->asFunction(runtime).call(runtime, *error, *val);
						std::this_thread::sleep_for(std::chrono::seconds(5));
					}
				};

				// // Launch the lambda in a new thread
				std::thread thread_object(f);
				thread_object.detach();

				return Value::undefined();
			});

		jsiRuntime.global().setProperty(jsiRuntime, "foo", std::move(foo));
	}
	// Function to install the JSI bindings
	void install(Runtime &jsiRuntime)
	{
		// Register the helloWorld function
		registerHelloWorld(jsiRuntime);
		registerFoo(jsiRuntime);
	}

	double multiply(double a, double b)
	{
		return a * b;
	}
}
@AbeereSpark AbeereSpark added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Jun 6, 2024
Copy link

github-actions bot commented Jun 6, 2024

⚠️ Newer Version of React Native is Available!
ℹ️ You are on a supported minor version, but it looks like there's a newer patch available - 0.74.2. Please upgrade to the highest patch for your minor or latest and verify if the issue persists (alternatively, create a new project and repro the issue in it). If it does not repro, please let us know so we can close out this issue. This helps us ensure we are looking at issues that still exist in the most recent releases.

@github-actions github-actions bot added Needs: Author Feedback Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. labels Jun 6, 2024
@cortinico
Copy link
Contributor

Please create a repro using the Reproducer Template

@github-actions github-actions bot removed the Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. label Jun 6, 2024
@AbeereSpark
Copy link
Author

@cortinico I have updated the issue with the required ReproducerApp.

@github-actions github-actions bot added Needs: Attention Issues where the author has responded to feedback. and removed Needs: Author Feedback labels Jun 6, 2024
@cortinico cortinico added Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. and removed Needs: Attention Issues where the author has responded to feedback. labels Jun 17, 2024
@huzhanbo1996
Copy link
Contributor

huzhanbo1996 commented Jun 20, 2024

In jsi.h, it specifies that JSRuntime can not be accessed concurrently. This crash is expected.

Represents a JS runtime. Movable, but not copyable. Note that
this object may not be thread-aware, but cannot be used safely from
multiple threads at once. The application is responsible for
ensuring that it is used safely. This could mean using the
Runtime from a single thread, using a mutex, doing all work on a
serial queue, etc. This restriction applies to the methods of
this class, and any method in the API which take a Runtime& as an
argument. Destructors (all but ~Scope), operators, or other methods
which do not take Runtime& as an argument are safe to call from any
thread, but it is still forbidden to make write operations on a single
instance of any class from more than one thread. In addition, to
make shutdown safe, destruction of objects associated with the Runtime
must be destroyed before the Runtime is destroyed, or from the
destructor of a managed HostObject or HostFunction. Informally, this
means that the main source of unsafe behavior is to hold a jsi object
in a non-Runtime-managed object, and not clean it up before the Runtime
is shut down. If your lifecycle is such that avoiding this is hard,
you will probably need to do use your own locks.
class JSI_EXPORT Runtime {}

@AbeereSpark
Copy link
Author

@huzhanbo1996 Suppose only one thread in C++ is accessing the JS Runtime then how can we stop JavaScript from using Runtime when C++ is using it?

@huzhanbo1996
Copy link
Contributor

huzhanbo1996 commented Jun 28, 2024

@huzhanbo1996 Suppose only one thread in C++ is accessing the JS Runtime then how can we stop JavaScript from using Runtime when C++ is using it?

In jsi.h, it suggests following solutions.

this could mean using the
Runtime from a single thread, using a mutex, doing all work on a
serial queue, etc.

It depends on what you want to achieve. In code you post. I presume you want to dispatch the task in another thread and return the result by callback in JS Thread. You could try things like serial queue or ReactContext.runOnJSQueueThread, both need some extra development and beyond this issue's scope. I suggest that you could investigate into more open source C++ RN modules and learn their design/implementation.

@Evgen74
Copy link

Evgen74 commented Jul 8, 2024

hi @AbeereSpark, to ensure safe calls to JS functions from cpp files during asynchronous operations, you must use an jsCallInvoker. Your application crashes because most likely you are trying to access memory that is already allocated for other needs

1st - get jsCallInvoker from bridge context
for ios

auto& runtime = *jsiRuntime;
auto callInvocker = bridge.jsCallInvoker;
install(runtime, callInvocker); // install - cpp function

for android

this.reactApplicationContext.javaScriptContextHolder?.let { contextHolder ->
    this.reactApplicationContext.catalystInstance.jsCallInvokerHolder?.let { callInvokerHolder: CallInvokerHolder ->
        this.nativeInstall(contextHolder.get(), callInvokerHolder)
        return true
    }
}

private external fun nativeInstall(jsi: Long, callInvoker: CallInvokerHolder)

in cpp

void install(Runtime &jsiRuntime, std::shared_ptr<react::CallInvoker> callInvoker)
	{
		// Register the helloWorld function
		registerHelloWorld(jsiRuntime);
		registerFoo(jsiRuntime);
	}

then you will be able to use callInvoker something like this

while (true)
	{
		__android_log_print(ANDROID_LOG_INFO, "MyApp", "Log message to Android");
		callInvoker->invokeAsync([&runtime, error, val, userCallbackRef]{
				userCallbackRef->asFunction(runtime).call(runtime, *error, *val);
		});
		std::this_thread::sleep_for(std::chrono::seconds(5));
	}

I haven't checked this code, maybe you need to play around with pointers, but the general idea is correct

@coado
Copy link
Collaborator

coado commented Sep 11, 2024

@Evgen74 I am not sure if it's possible to get jsCallInvoker from bridge on a bridgeless mode. I am wondering if there is a way to access runtimeExecutor or CallInvoker in TurboModule on the JS thread. There was already a discussion about implementing something like that react-native-community/discussions-and-proposals#196

@coado
Copy link
Collaborator

coado commented Sep 16, 2024

@AbeereSpark You should be able to achieve something like that by generating C++ module which gets access to jsCallInvoker. You can read more about it here.

@cipolleschi
Copy link
Contributor

This seems to be more of a question and @coado kindly replied.
@Evgen74 @AbeereSpark can we consider it solved?

@cortinico cortinico added Needs: Author Feedback and removed Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. labels Sep 17, 2024
@AbeereSpark
Copy link
Author

@coado @cipolleschi Tried to call Synchronously but it crashed and when calling from jsInvoker, the compiler says that the copy constructor is disabled. Please guide if i'm handling it incorrectly.

E:/AndroidSDK/ndk/26.1.10909125/toolchains/llvm/prebuilt/windows-x86_64/sysroot/usr/include/c++/v1/__memory/shared_ptr.h:279:37: error: call to
implicitly-deleted copy constructor of 'facebook::jsi::Function'

#include "NativeSampleModule.h"

namespace facebook::react {

NativeSampleModule::NativeSampleModule(std::shared_ptr<CallInvoker> jsInvoker)
    : NativeSampleModuleCxxSpec(std::move(jsInvoker)) {}

std::string NativeSampleModule::reverseString(jsi::Runtime& rt, std::string input) {
  return std::string(input.rbegin(), input.rend());
}

void NativeSampleModule::storeAndCallJSCallback(jsi::Runtime& rt, const jsi::Function& jsCallback) {
    #if 0
      auto callback = std::make_shared<jsi::Function>(jsCallback);
      jsInvoker_->invokeAsync([callback, &rt]() {
        std::string result = "Callback from C++!";

        callback->call(rt, jsi::String::createFromUtf8(rt, result));
      });
    #endif

    std::string result = "Callback from C++!";
    jsCallback.call(rt, jsi::String::createFromUtf8(rt, result));
}

} // namespace facebook::react

@github-actions github-actions bot added Needs: Attention Issues where the author has responded to feedback. and removed Needs: Author Feedback labels Sep 18, 2024
@cipolleschi
Copy link
Contributor

cipolleschi commented Sep 19, 2024

std::make_sharedjsi::Function(jsCallback)

have you tried std::move(jsCallback) instead? You can't create a copy of the function, but you can move it around.
Have a look at the TimerManager implementation for inspiration.

@hsjoberg
Copy link
Contributor

hsjoberg commented Sep 19, 2024

I think you should also be able to use the helper class AsyncCallback, that can be moved and does the invokeAsync for you.

See here:
reactwg/react-native-new-architecture#216

For function arguments, just use AsyncFunction instead of Function, it bridges to it.

@AbeereSpark
Copy link
Author

@cipolleschi I implemented this approach, and it is working well. I will proceed as you and @hsjoberg suggested. Thanks for the solution @coado.

void NativeSampleModule::install(jsi::Runtime &jsiRuntime) {

    auto callFromCplusplus = jsi::Function::createFromHostFunction(
        jsiRuntime,
        jsi::PropNameID::forAscii(jsiRuntime, "callFromCplusplus"),
        1,
        [this](jsi::Runtime& runtime, const jsi::Value& thisValue, const jsi::Value* arguments, size_t count) -> jsi::Value {

            // Store the callback function reference
            auto userCallbackRef = std::make_shared<jsi::Object>(arguments[0].getObject(runtime));

            // Helper function to create "Hello {counter}" string
            auto createMessage = [&runtime](int counter) {
                return jsi::String::createFromUtf8(runtime, "Hello " + std::to_string(counter));
            };


            std::thread([userCallbackRef, this, &runtime, createMessage]() mutable {
                int counter = 0;  // Initialize counter

                while (true) {
                    // Prepare values
                    auto valPtr = std::make_shared<jsi::String>(createMessage(counter++));
                    auto errorPtr = std::make_shared<jsi::Value>(jsi::Value::undefined());

                    // Async call to JS function
                    jsInvoker_->invokeAsync([userCallbackRef, valPtr, errorPtr, &runtime]() {
                        userCallbackRef->asFunction(runtime).call(runtime, *errorPtr, *valPtr);
                    });

                    // Sleep for 5 seconds
                    std::this_thread::sleep_for(std::chrono::seconds(1));
                }
            }).detach();  // Detach the thread to run independently

            return jsi::Value::undefined();
        }
    );
    jsiRuntime.global().setProperty(jsiRuntime, "callFromCplusplus", std::move(callFromCplusplus));
}

@AbeereSpark
Copy link
Author

@cipolleschi @hsjoberg, both of your solutions are working well. @hsjoberg, your solution simplifies things by eliminating the need for manual handling. Thanks to both of you for your great input!

void NativeSampleModule::storeAndCallJSCallback(jsi::Runtime& rt, jsi::Function jsCallback) {

    auto createMessage = [](int counter) {
        return "Hello " + std::to_string(counter);
    };

    std::thread([callback = std::make_shared<jsi::Function>(std::move(jsCallback)), this, createMessage]() mutable 
    {
        int counter = 0;

        while (true) 
        {
            std::string message = createMessage(counter++);

            auto messagePtr = std::make_shared<std::string>(std::move(message));

            jsInvoker_->invokeAsync([callback, messagePtr, this](jsi::Runtime& rt) 
            {
                auto jsString = jsi::String::createFromUtf8(rt, *messagePtr);
                callback->call(rt, jsString); 
            });

            std::this_thread::sleep_for(std::chrono::milliseconds(10));
        }
    }).detach(); 
}

void NativeSampleModule::callback(jsi::Runtime &rt, AsyncCallback<std::string> callback) {
    std::thread t([callback = std::move(callback)]() {
        int counter = 0;
        while (1)
        {
            /* code */
            callback.call("works without any issues " + std::to_string(counter++));
            std::this_thread::sleep_for(std::chrono::milliseconds(10));
        }
        
    });
    t.detach();
}

@cipolleschi
Copy link
Contributor

Closing as the author is unblocked and the current solution is working fine.

@cortinico cortinico added Resolution: Answered When the issue is resolved with a simple answer and removed Needs: Attention Issues where the author has responded to feedback. labels Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Newer Patch Available Resolution: Answered When the issue is resolved with a simple answer Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

No branches or pull requests

7 participants