Skip to content

Commit 891d632

Browse files
JakubGoneraJakubGonera
committed
fix: ensure thread-safe runtime usage
1 parent 2b64c74 commit 891d632

File tree

6 files changed

+60
-52
lines changed

6 files changed

+60
-52
lines changed

‎common/rnexecutorch/Log.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@ namespace rnexecutorch {
1515
#ifdef __ANDROID__
1616
android_LogPriority androidLogLevel(LOG_LEVEL logLevel) {
1717
switch (logLevel) {
18-
case LOG_LEVEL::INFO:
18+
case LOG_LEVEL::Info:
1919
default:
2020
return ANDROID_LOG_INFO;
21-
case LOG_LEVEL::ERROR:
21+
case LOG_LEVEL::Error:
2222
return ANDROID_LOG_ERROR;
23-
case LOG_LEVEL::DEBUG:
23+
case LOG_LEVEL::Debug:
2424
return ANDROID_LOG_DEBUG;
2525
}
2626
}
@@ -49,14 +49,14 @@ void log(LOG_LEVEL logLevel, const char *fmt, ...) {
4949
#ifdef __APPLE__
5050

5151
switch (logLevel) {
52-
case LOG_LEVEL::INFO:
52+
case LOG_LEVEL::Info:
5353
default:
5454
os_log_info(OS_LOG_DEFAULT, "%s", buf);
5555
break;
56-
case LOG_LEVEL::ERROR:
56+
case LOG_LEVEL::Error:
5757
os_log_error(OS_LOG_DEFAULT, "%s", buf);
5858
break;
59-
case LOG_LEVEL::DEBUG:
59+
case LOG_LEVEL::Debug:
6060
os_log_debug(OS_LOG_DEFAULT, "%s", buf);
6161
break;
6262
}

‎common/rnexecutorch/Log.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
namespace rnexecutorch {
44

5-
enum class LOG_LEVEL { INFO, ERROR, DEBUG };
5+
enum class LOG_LEVEL { Info, Error, Debug };
66

77
// const char* instead of const std::string& as va_start doesn't take references
88
void log(LOG_LEVEL logLevel, const char *fmt, ...);

‎common/rnexecutorch/RnExecutorchInstaller.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ jsi::Function RnExecutorchInstaller::loadStyleTransfer(
2626
auto styleTransferPtr =
2727
std::make_shared<StyleTransfer>(source, &runtime);
2828
auto styleTransferHostObject =
29-
std::make_shared<ModelHostObject<StyleTransfer>>(
30-
styleTransferPtr, &runtime, jsCallInvoker);
29+
std::make_shared<ModelHostObject<StyleTransfer>>(styleTransferPtr,
30+
jsCallInvoker);
3131

3232
return jsi::Object::createFromHostObject(runtime,
3333
styleTransferHostObject);

‎common/rnexecutorch/host_objects/ModelHostObject.h‎

Lines changed: 47 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -34,48 +34,56 @@ template <typename Model> class ModelHostObject : public JsiHostObject {
3434
errorMessage, sizeof(errorMessage),
3535
"Argument count mismatch, was expecting: %zu but got: %zu",
3636
forwardArgCount, count);
37-
callInvoker->invokeAsync(
38-
[errorMessage, &promise]() { promise->reject(errorMessage); });
37+
promise->reject(errorMessage);
3938
return;
4039
}
4140

42-
// We need to dispatch a thread if we want the forward to be
43-
// asynchronous
44-
std::thread([args, this, promise, &runtime]() {
45-
try {
46-
auto argsConverted = jsiconversion::createArgsTupleFromJsi(
47-
&Model::forward, args, runtime);
48-
auto result = std::apply(std::bind_front(&Model::forward, model),
49-
argsConverted);
50-
callInvoker->invokeAsync(
51-
[promise, result = std::move(result)](jsi::Runtime &runtime) {
52-
promise->resolve(
53-
jsiconversion::getJsiValue(std::move(result), runtime));
54-
});
55-
} catch (const std::runtime_error &e) {
56-
// This catch should be merged with the next two
57-
// (std::runtime_error and jsi::JSError inherits from
58-
// std::exception) HOWEVER react native has broken RTTI which
59-
// breaks proper exception type checking. Remove when the
60-
// following change is present in our version:
61-
// https://github.com/facebook/react-native/commit/3132cc88dd46f95898a756456bebeeb6c248f20e
62-
callInvoker->invokeAsync(
63-
[&e, promise]() { promise->reject(e.what()); });
64-
return;
65-
} catch (const jsi::JSError &e) {
66-
callInvoker->invokeAsync(
67-
[&e, promise]() { promise->reject(e.what()); });
68-
return;
69-
} catch (const std::exception &e) {
70-
callInvoker->invokeAsync(
71-
[&e, promise]() { promise->reject(e.what()); });
72-
return;
73-
} catch (...) {
74-
callInvoker->invokeAsync(
75-
[promise]() { promise->reject("Unknown error"); });
76-
return;
77-
}
78-
}).detach();
41+
try {
42+
auto argsConverted = jsiconversion::createArgsTupleFromJsi(
43+
&Model::forward, args, runtime);
44+
45+
// We need to dispatch a thread if we want the forward to be
46+
// asynchronous. In this thread all accesses to jsi::Runtime need to
47+
// be done via the callInvoker.
48+
std::thread([this, promise,
49+
argsConverted = std::move(argsConverted)]() {
50+
try {
51+
auto result = std::apply(
52+
std::bind_front(&Model::forward, model), argsConverted);
53+
54+
callInvoker->invokeAsync([promise, result = std::move(result)](
55+
jsi::Runtime &runtime) {
56+
promise->resolve(
57+
jsiconversion::getJsiValue(std::move(result), runtime));
58+
});
59+
} catch (const std::runtime_error &e) {
60+
// This catch should be merged with the next two
61+
// (std::runtime_error and jsi::JSError inherits from
62+
// std::exception) HOWEVER react native has broken RTTI which
63+
// breaks proper exception type checking. Remove when the
64+
// following change is present in our version:
65+
// https://github.com/facebook/react-native/commit/3132cc88dd46f95898a756456bebeeb6c248f20e
66+
callInvoker->invokeAsync(
67+
[&e, promise]() { promise->reject(e.what()); });
68+
return;
69+
} catch (const jsi::JSError &e) {
70+
callInvoker->invokeAsync(
71+
[&e, promise]() { promise->reject(e.what()); });
72+
return;
73+
} catch (const std::exception &e) {
74+
callInvoker->invokeAsync(
75+
[&e, promise]() { promise->reject(e.what()); });
76+
return;
77+
} catch (...) {
78+
callInvoker->invokeAsync(
79+
[promise]() { promise->reject("Unknown error"); });
80+
return;
81+
}
82+
}).detach();
83+
} catch (...) {
84+
promise->reject(
85+
"Couldn't parse JS arguments in native forward function");
86+
}
7987
});
8088

8189
return promise;

‎common/rnexecutorch/models/BaseModel.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#include <rnexecutorch/models/BaseModel.h>
1+
#include "BaseModel.h"
22

33
#include <rnexecutorch/Log.h>
44

‎common/rnexecutorch/models/StyleTransfer.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
#include "StyleTransfer.h"
22

3+
#include <rnexecutorch/Log.h>
4+
#include <rnexecutorch/data_processing/ImageProcessing.h>
5+
36
#include <span>
47

58
#include <executorch/extension/tensor/tensor.h>
69
#include <opencv2/opencv.hpp>
710

8-
#include <rnexecutorch/Log.h>
9-
#include <rnexecutorch/data_processing/ImageProcessing.h>
10-
1111
namespace rnexecutorch {
1212
using namespace facebook;
1313
using executorch::extension::Module;

0 commit comments

Comments
 (0)