Skip to content

Commit 70ebdd0

Browse files
DjDeveloperrDjDeveloperr
committed
fix: harden FFI lifetime safety
1 parent b5dbe0d commit 70ebdd0

30 files changed

Lines changed: 860 additions & 183 deletions

‎NativeScript/ffi/hermes/NativeApiJsi.mm‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ Function CreateNativeApiSelectorGroupFunctionImpl(
305305
cachedDispatchClass = Class(Nil)](
306306
Runtime& runtime, const Value& thisValue, const Value* args,
307307
size_t count) mutable -> Value {
308+
NativeApiRoundTripCacheFrameGuard roundTripFrame(bridge);
308309
if (count >= selectors->size() ||
309310
(*selectors)[count].selectorName.empty()) {
310311
throw JSError(runtime,

‎NativeScript/ffi/jsc/NativeApiJSC.mm‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,6 +1065,7 @@ JSValueRef NativeApiSelectorGroupCall(
10651065

10661066
Runtime& runtime = data->runtime;
10671067
try {
1068+
NativeApiRoundTripCacheFrameGuard roundTripFrame(data->bridge);
10681069
if (argumentCount >= data->selectors->size() ||
10691070
(*data->selectors)[argumentCount].selectorName.empty()) {
10701071
throw JSError(runtime,

‎NativeScript/ffi/quickjs/NativeApiQuickJS.mm‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,6 +1171,7 @@ JSValue NativeApiSelectorGroupCall(JSContext* context, JSValue thisValue,
11711171

11721172
Runtime& runtime = data->runtime;
11731173
try {
1174+
NativeApiRoundTripCacheFrameGuard roundTripFrame(data->bridge);
11741175
size_t count = argc > 0 ? static_cast<size_t>(argc) : 0;
11751176
if (count >= data->selectors->size() ||
11761177
(*data->selectors)[count].selectorName.empty()) {

‎NativeScript/ffi/shared/NativeApiBackendConfig.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ struct NativeApiBackendConfig {
2121
std::function<void(std::function<void()>)> nativeInvocationInvoker = nullptr;
2222
std::function<void(std::function<void()>)> nativeCallbackInvoker = nullptr;
2323
std::function<void(std::function<void()>)> jsThreadCallbackInvoker = nullptr;
24+
std::function<void(std::function<void()>)> jsThreadAsyncCallbackInvoker = nullptr;
2425
bool invokeCallbacksOnNativeCallerThread = false;
2526
bool installGlobalSymbols = false;
2627
};

‎NativeScript/ffi/shared/bridge/Callbacks.mm‎

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -555,13 +555,17 @@ bool releaseBlockCopy(const void* blockPointer) {
555555
}
556556
};
557557

558-
if (bridge == nullptr ||
559-
std::this_thread::get_id() == bridge->jsThreadId()) {
558+
if (bridge == nullptr) {
560559
releaseOnJS();
561-
} else if (const auto& invoker = bridge->jsThreadCallbackInvoker()) {
562-
invoker(std::move(releaseOnJS));
560+
} else if (const auto& asyncInvoker =
561+
bridge->jsThreadAsyncCallbackInvoker()) {
562+
asyncInvoker(std::move(releaseOnJS));
563563
} else if (auto scheduler = bridge->scheduler()) {
564564
scheduler->invokeOnJS(std::move(releaseOnJS));
565+
} else if (std::this_thread::get_id() == bridge->jsThreadId()) {
566+
releaseOnJS();
567+
} else if (const auto& invoker = bridge->jsThreadCallbackInvoker()) {
568+
invoker(std::move(releaseOnJS));
565569
} else {
566570
releaseOnJS();
567571
}
@@ -660,6 +664,43 @@ void invoke(void* ret, void* args[]) {
660664
bool waitForNativeThreadCallback =
661665
currentThreadIsJs && nativeCallbackInvoker &&
662666
gActiveNativeThreadEngineCallbacks.load(std::memory_order_acquire) > 0;
667+
auto dispatchZeroArgVoidBlockAsync = [&]() -> bool {
668+
if (currentThreadIsJs || !returnsVoid || !block_ ||
669+
!signature_->argumentTypes.empty()) {
670+
return false;
671+
}
672+
673+
std::shared_ptr<NativeApiCallback> keepAlive;
674+
try {
675+
keepAlive = shared_from_this();
676+
} catch (const std::bad_weak_ptr&) {
677+
return false;
678+
}
679+
680+
auto asyncCall = [keepAlive = std::move(keepAlive)]() mutable {
681+
std::string asyncError;
682+
keepAlive->invokeOnCurrentThread(nullptr, nullptr, &asyncError);
683+
if (!asyncError.empty()) {
684+
recordNativeCallbackException(asyncError);
685+
}
686+
};
687+
688+
const auto& asyncInvoker = bridge_->jsThreadAsyncCallbackInvoker();
689+
if (asyncInvoker) {
690+
asyncInvoker(std::move(asyncCall));
691+
return true;
692+
}
693+
if (auto scheduler = bridge_->scheduler()) {
694+
scheduler->invokeOnJS(std::move(asyncCall));
695+
return true;
696+
}
697+
return false;
698+
};
699+
700+
if (dispatchZeroArgVoidBlockAsync()) {
701+
return;
702+
}
703+
663704
if (direct && !waitForNativeThreadCallback) {
664705
if (nativeCallerThreadCallback) {
665706
callOnNativeCallerThread();
@@ -745,7 +786,8 @@ void invokeOnCurrentThread(void* ret, void* args[], std::string* error) {
745786
static_cast<size_t>(jsArgs.size()));
746787
}
747788
storeReturnValue(result, ret);
748-
if (std::this_thread::get_id() == bridge_->jsThreadId()) {
789+
if (std::this_thread::get_id() == bridge_->jsThreadId() &&
790+
gSynchronousNativeInvocationDepth == 0) {
749791
runtime_->drainMicrotasks();
750792
}
751793
} catch (const std::exception& exception) {

‎NativeScript/ffi/shared/bridge/HostObject.mm‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -433,8 +433,9 @@ throw JSError(runtime,
433433
"CC_SHA256 is not available.");
434434
}
435435
NativeApiArgumentFrame frame(3);
436-
void* data = pointerFromEngineValue(runtime, args[0], frame);
437-
void* output = pointerFromEngineValue(runtime, args[2], frame);
436+
void* data = pointerFromEngineValue(runtime, bridge, args[0], frame);
437+
void* output =
438+
pointerFromEngineValue(runtime, bridge, args[2], frame);
438439
using CC_SHA256_Fn = unsigned char* (*)(const void*, unsigned long,
439440
unsigned char*);
440441
auto fn = reinterpret_cast<CC_SHA256_Fn>(symbol);

‎NativeScript/ffi/shared/bridge/HostObjects.mm‎

Lines changed: 74 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,13 @@ void setObject(id object) {
4949
public:
5050
NativeApiPointerHostObject(std::shared_ptr<NativeApiBridge> bridge,
5151
void* pointer, std::string kind = "pointer",
52-
bool adopted = false)
52+
bool adopted = false,
53+
std::shared_ptr<Value> backingValue = nullptr)
5354
: bridge_(std::move(bridge)),
5455
pointer_(pointer),
5556
kind_(std::move(kind)),
56-
adopted_(adopted) {}
57+
adopted_(adopted),
58+
backingValue_(std::move(backingValue)) {}
5759

5860
~NativeApiPointerHostObject() override {
5961
if (adopted_ && pointer_ != nullptr) {
@@ -66,6 +68,10 @@ void setObject(id object) {
6668
}
6769

6870
void* pointer() const { return pointer_; }
71+
std::shared_ptr<Value> backingValue() const { return backingValue_; }
72+
void setBackingValue(Runtime& runtime, const Value& value) {
73+
backingValue_ = std::make_shared<Value>(runtime, value);
74+
}
6975
bool adopted() const { return adopted_; }
7076
void adopt() { adopted_ = true; }
7177
void clearWithoutFree() {
@@ -74,6 +80,7 @@ void clearWithoutFree() {
7480
}
7581
pointer_ = nullptr;
7682
adopted_ = false;
83+
backingValue_.reset();
7784
}
7885

7986
Value get(Runtime& runtime, const PropNameID& name) override {
@@ -102,6 +109,7 @@ Value get(Runtime& runtime, const PropNameID& name) override {
102109
self->consumed_ = true;
103110
self->pointer_ = nullptr;
104111
self->adopted_ = false;
112+
self->backingValue_.reset();
105113
return makeNativeObjectValue(runtime, self->bridge_, object, retained);
106114
});
107115
}
@@ -204,6 +212,7 @@ Value get(Runtime& runtime, const PropNameID& name) override {
204212
std::string kind_;
205213
bool adopted_ = false;
206214
bool consumed_ = false;
215+
std::shared_ptr<Value> backingValue_;
207216
};
208217

209218
class NativeApiReferenceHostObject final : public HostObject {
@@ -230,6 +239,7 @@ Value get(Runtime& runtime, const PropNameID& name) override {
230239

231240
void* data() const { return data_; }
232241
const NativeApiType& type() const { return type_; }
242+
std::shared_ptr<Value> backingValue() const { return backingValue_; }
233243
void ensureStorage(Runtime& runtime, NativeApiType type,
234244
NativeApiArgumentFrame& frame, size_t elements = 1);
235245

@@ -640,6 +650,10 @@ Array runtimeMembersArray(Runtime& runtime, Class cls, bool staticMembers) {
640650
}
641651

642652
~NativeApiObjectHostObject() override {
653+
if (bridge_ != nullptr && object_ != nil) {
654+
bridge_->forgetRoundTripValue(object_);
655+
bridge_->forgetObjectExpandos(object_);
656+
}
643657
if (lifetimeState_ != nullptr) {
644658
lifetimeState_->clear();
645659
}
@@ -666,6 +680,10 @@ void storeOwnExpando(Runtime& runtime, const std::string& property,
666680

667681
void disownObject(id expected) {
668682
if (object_ == expected) {
683+
if (bridge_ != nullptr && expected != nil) {
684+
bridge_->forgetRoundTripValue(expected);
685+
bridge_->forgetObjectExpandos(expected);
686+
}
669687
ownsObject_ = false;
670688
object_ = nil;
671689
if (lifetimeState_ != nullptr) {
@@ -906,7 +924,8 @@ Value resolveEnginePrototypeGetter(Runtime& runtime,
906924
Value getterValue = descriptor.getProperty(runtime, "get");
907925
if (getterValue.isObject() &&
908926
getterValue.asObject(runtime).isFunction(runtime)) {
909-
Value thisValue = bridge_->findRoundTripValue(runtime, object_);
927+
Value thisValue = bridge_->findRoundTripValue(runtime, object_,
928+
nullptr, true);
910929
if (thisValue.isObject()) {
911930
*found = true;
912931
return getterValue.asObject(runtime).asFunction(runtime).callWithThis(
@@ -964,7 +983,8 @@ bool invokeEnginePrototypeSetter(Runtime& runtime, const std::string& property,
964983
descriptorValue.asObject(runtime).getProperty(runtime, "set");
965984
if (setterValue.isObject() &&
966985
setterValue.asObject(runtime).isFunction(runtime)) {
967-
Value thisValue = bridge_->findRoundTripValue(runtime, object_);
986+
Value thisValue = bridge_->findRoundTripValue(runtime, object_,
987+
nullptr, true);
968988
if (thisValue.isObject()) {
969989
Value args[] = {Value(runtime, value)};
970990
setterValue.asObject(runtime).asFunction(runtime).callWithThis(
@@ -1145,7 +1165,7 @@ Value get(Runtime& runtime, const PropNameID& name) override {
11451165
return self->callObjectSelector(runtime, selectorName, nullptr,
11461166
args + 1, count - 1);
11471167
}
1148-
return callObjCSelector(runtime, bridge, object, false, selectorName,
1168+
return callObjCSelector(runtime, bridge, object, false, selectorName,
11491169
nullptr, args + 1, count - 1);
11501170
});
11511171
}
@@ -1162,19 +1182,30 @@ Value get(Runtime& runtime, const PropNameID& name) override {
11621182
}
11631183

11641184
id object = self->object_;
1185+
bool ownsObject = self->ownsObject_;
11651186
if (self->bridge_ != nullptr) {
11661187
self->bridge_->forgetRoundTripValue(runtime, object);
1167-
}
1168-
if (self->ownsObject_) {
1169-
[object release];
1188+
self->bridge_->forgetObjectExpandos(object);
11701189
}
11711190
self->object_ = nil;
11721191
self->ownsObject_ = false;
11731192
if (self->lifetimeState_ != nullptr) {
11741193
self->lifetimeState_->clear();
11751194
}
11761195
self->consumed_ = true;
1177-
return makeNativeObjectValue(runtime, self->bridge_, object, retained);
1196+
try {
1197+
Value result =
1198+
makeNativeObjectValue(runtime, self->bridge_, object, retained);
1199+
if (!retained && ownsObject) {
1200+
[object release];
1201+
}
1202+
return result;
1203+
} catch (...) {
1204+
if (!retained && ownsObject) {
1205+
[object release];
1206+
}
1207+
throw;
1208+
}
11781209
});
11791210
}
11801211
if (property == "toString") {
@@ -1183,7 +1214,7 @@ Value get(Runtime& runtime, const PropNameID& name) override {
11831214
runtime, PropNameID::forAscii(runtime, "toString"), 0,
11841215
[object](Runtime& runtime, const Value&, const Value*, size_t) -> Value {
11851216
return NativeApiObjectHostObject::descriptionString(runtime, object);
1186-
});
1217+
});
11871218
}
11881219
if (property == "description") {
11891220
return descriptionString(runtime, object_);
@@ -1611,16 +1642,38 @@ throw JSError(
16111642
} else if (args[0].isObject()) {
16121643
Object object = args[0].asObject(runtime);
16131644
if (object.isHostObject<NativeApiPointerHostObject>(runtime)) {
1614-
pointer = object
1615-
.getHostObject<NativeApiPointerHostObject>(
1616-
runtime)
1617-
->pointer();
1645+
auto pointerHost =
1646+
object.getHostObject<NativeApiPointerHostObject>(
1647+
runtime);
1648+
pointer = pointerHost->pointer();
1649+
if (pointerHost->backingValue() != nullptr) {
1650+
Value backingValue(runtime, *pointerHost->backingValue());
1651+
id backingObject =
1652+
NativeApiObjectHostObject::nativeObjectFromValue(
1653+
runtime, backingValue);
1654+
if (backingObject == static_cast<id>(pointer) &&
1655+
backingObject != nil &&
1656+
[backingObject isKindOfClass:cls]) {
1657+
return backingValue;
1658+
}
1659+
}
16181660
} else if (object.isHostObject<NativeApiReferenceHostObject>(
16191661
runtime)) {
1620-
pointer = object
1621-
.getHostObject<NativeApiReferenceHostObject>(
1622-
runtime)
1623-
->data();
1662+
auto referenceHost =
1663+
object.getHostObject<NativeApiReferenceHostObject>(
1664+
runtime);
1665+
pointer = referenceHost->data();
1666+
if (referenceHost->backingValue() != nullptr) {
1667+
Value backingValue(runtime, *referenceHost->backingValue());
1668+
id backingObject =
1669+
NativeApiObjectHostObject::nativeObjectFromValue(
1670+
runtime, backingValue);
1671+
if (backingObject == static_cast<id>(pointer) &&
1672+
backingObject != nil &&
1673+
[backingObject isKindOfClass:cls]) {
1674+
return backingValue;
1675+
}
1676+
}
16241677
} else if (object.isHostObject<NativeApiObjectHostObject>(
16251678
runtime)) {
16261679
pointer = object
@@ -1782,15 +1835,15 @@ Value makeNativeObjectValue(Runtime& runtime,
17821835
return Value::null();
17831836
}
17841837

1785-
Value cached = bridge->findRoundTripValue(runtime, object);
1838+
Value cached = bridge->findRoundTripValue(runtime, object, nullptr, true);
17861839
if (!cached.isUndefined()) {
17871840
// A consumed wrapper (e.g. an alloc'd placeholder singleton already passed
17881841
// to an initializer) must not be reused: drop the stale entry and re-wrap.
17891842
auto cachedHost =
17901843
cached.isObject()
17911844
? cached.asObject(runtime).getHostObject<NativeApiObjectHostObject>(runtime)
17921845
: nullptr;
1793-
if (cachedHost == nullptr || cachedHost->object() != nil) {
1846+
if (cachedHost != nullptr && cachedHost->object() != nil) {
17941847
if (ownsObject) {
17951848
[object release];
17961849
}
@@ -1816,7 +1869,7 @@ Value makeNativeObjectValue(Runtime& runtime,
18161869
Object prototype = prototypeValue.asObject(runtime);
18171870
SetNativeApiObjectPrototype(runtime, result, prototype);
18181871
}
1819-
bridge->rememberRoundTripValue(
1872+
bridge->rememberScopedRoundTripValue(
18201873
runtime, object, Value(runtime, result),
18211874
nativeObjectIsStringLike(object));
18221875
return result;

‎NativeScript/ffi/shared/bridge/Invocation.mm‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ Value callCFunction(Runtime& runtime,
404404
if (prepared == nullptr) {
405405
throw JSError(runtime, "Native function state is unavailable.");
406406
}
407+
NativeApiRoundTripCacheFrameGuard roundTripFrame(bridge);
407408

408409
MDMetadataReader* metadata = bridge->metadata();
409410
if (metadata == nullptr) {
@@ -1535,6 +1536,7 @@ Value callPreparedObjCSelector(
15351536
throw JSError(runtime,
15361537
"Cannot send Objective-C selector to nil.");
15371538
}
1539+
NativeApiRoundTripCacheFrameGuard roundTripFrame(bridge);
15381540

15391541
const NativeApiSignature& signature = prepared.signature;
15401542
Value fastResult;
@@ -1661,6 +1663,7 @@ Value callObjCSelector(Runtime& runtime,
16611663
throw JSError(runtime,
16621664
"Cannot send Objective-C selector to nil.");
16631665
}
1666+
NativeApiRoundTripCacheFrameGuard roundTripFrame(bridge);
16641667

16651668
SEL selector = sel_registerName(selectorName.c_str());
16661669
Class receiverClass =

0 commit comments

Comments
 (0)