Skip to content

Commit bd41da4

Browse files
committed
fix(ffi): stabilize direct block callback reuse
1 parent 1660511 commit bd41da4

4 files changed

Lines changed: 29 additions & 16 deletions

File tree

NativeScript/ffi/shared/jsi/NativeApiJsiBridge.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,7 @@ class NativeApiJsiBridge {
508508
if (native == nullptr) {
509509
return;
510510
}
511+
std::lock_guard<std::mutex> lock(roundTripValuesMutex_);
511512
roundTripValues_[normalizeRuntimePointer(
512513
reinterpret_cast<uintptr_t>(native))] =
513514
std::make_shared<Value>(runtime, value);
@@ -517,6 +518,7 @@ class NativeApiJsiBridge {
517518
if (native == nullptr) {
518519
return Value::undefined();
519520
}
521+
std::lock_guard<std::mutex> lock(roundTripValuesMutex_);
520522
auto it = roundTripValues_.find(
521523
normalizeRuntimePointer(reinterpret_cast<uintptr_t>(native)));
522524
if (it == roundTripValues_.end() || it->second == nullptr) {
@@ -529,6 +531,7 @@ class NativeApiJsiBridge {
529531
if (native == nullptr) {
530532
return;
531533
}
534+
std::lock_guard<std::mutex> lock(roundTripValuesMutex_);
532535
roundTripValues_.erase(
533536
normalizeRuntimePointer(reinterpret_cast<uintptr_t>(native)));
534537
}
@@ -1506,6 +1509,7 @@ class NativeApiJsiBridge {
15061509
std::unordered_map<std::string, NativeApiSymbol> protocolSymbolsByRuntimeName_;
15071510
std::unordered_map<uintptr_t, NativeApiSymbol> classSymbolsByRuntimePointer_;
15081511
std::unordered_map<uintptr_t, NativeApiSymbol> protocolSymbolsByRuntimePointer_;
1512+
mutable std::mutex roundTripValuesMutex_;
15091513
std::unordered_map<uintptr_t, std::shared_ptr<Value>> roundTripValues_;
15101514
std::unordered_map<uintptr_t, std::shared_ptr<Value>> classValues_;
15111515
std::unordered_map<uintptr_t, std::shared_ptr<Value>> classPrototypes_;

NativeScript/ffi/shared/jsi/NativeApiJsiCallbacks.h

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -413,15 +413,15 @@ class NativeApiJsiCallback final
413413
retainedBlockCopies_.push_back({blockPointer, std::move(self)});
414414
}
415415

416-
void releaseBlockCopy(const void* blockPointer) {
416+
bool releaseBlockCopy(const void* blockPointer) {
417417
if (!block_) {
418-
return;
418+
return false;
419419
}
420420
std::shared_ptr<NativeApiJsiCallback> keepAlive;
421421
try {
422422
keepAlive = shared_from_this();
423423
} catch (const std::bad_weak_ptr&) {
424-
return;
424+
return false;
425425
}
426426
std::lock_guard<std::mutex> lock(retainedBlockCopiesMutex_);
427427
auto it = retainedBlockCopies_.end();
@@ -432,15 +432,14 @@ class NativeApiJsiCallback final
432432
return retained.blockPointer == blockPointer;
433433
});
434434
}
435-
if (it == retainedBlockCopies_.end() && !retainedBlockCopies_.empty()) {
436-
it = retainedBlockCopies_.end() - 1;
437-
}
438435
if (it != retainedBlockCopies_.end()) {
439436
if (bridge_ != nullptr && it->blockPointer != nullptr) {
440437
bridge_->forgetRoundTripValue(it->blockPointer);
441438
}
442439
retainedBlockCopies_.erase(it);
440+
return true;
443441
}
442+
return false;
444443
}
445444

446445
void invoke(void* ret, void* args[]) {
@@ -751,8 +750,11 @@ void nativeApiJsiBlockDispose(void* src) {
751750
if (block == nullptr || block->callback == nullptr) {
752751
return;
753752
}
754-
static_cast<NativeApiJsiCallback*>(block->callback)->releaseBlockCopy(block);
755-
block->callback = nullptr;
753+
bool released =
754+
static_cast<NativeApiJsiCallback*>(block->callback)->releaseBlockCopy(block);
755+
if (released) {
756+
block->callback = nullptr;
757+
}
756758
}
757759

758760
void nativeApiJsiCallbackTrampoline(ffi_cif*, void* ret, void* args[],

NativeScript/ffi/shared/jsi/NativeApiJsiConversion.h

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -751,15 +751,20 @@ void convertJsiArgument(Runtime& runtime,
751751
if (value.isObject()) {
752752
Object object = value.asObject(runtime);
753753
void* nativePointer = nullptr;
754-
if (readNativePointerProperty(runtime, object, &nativePointer)) {
755-
*static_cast<void**>(target) = nativePointer;
756-
break;
757-
}
758754
if (object.isFunction(runtime)) {
755+
std::string functionKind = stringPropertyOrEmpty(runtime, object, "kind");
756+
if (functionKind == "block" || functionKind == "functionPointer" ||
757+
functionKind == "functionReference") {
758+
if (readNativePointerProperty(runtime, object, &nativePointer)) {
759+
*static_cast<void**>(target) = nativePointer;
760+
break;
761+
}
762+
}
763+
759764
auto threadPolicy = readJsiCallbackThreadPolicy(runtime, object);
760-
auto callback = createJsiCallback(
761-
runtime, bridge, type, object.asFunction(runtime),
762-
type.kind == metagen::mdTypeBlock, threadPolicy);
765+
auto callback =
766+
createJsiCallback(runtime, bridge, type, object.asFunction(runtime),
767+
type.kind == metagen::mdTypeBlock, threadPolicy);
763768
void* pointer = callback->functionPointer();
764769
if (type.kind == metagen::mdTypeBlock) {
765770
frame.addLifetime(callback);

NativeScript/ffi/v8/NativeApiV8.mm

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,9 @@ void NativeApiV8LazyGlobalGetter(v8::Local<v8::Name>,
108108
}
109109
return;
110110
}
111-
global->DefineOwnProperty(context, nameValue, result, v8::DontEnum).FromMaybe(false);
111+
if (global->Delete(context, nameValue).FromMaybe(false)) {
112+
global->DefineOwnProperty(context, nameValue, result, v8::DontEnum).FromMaybe(false);
113+
}
112114
info.GetReturnValue().Set(result);
113115
}
114116

0 commit comments

Comments
 (0)