Skip to content

Commit 5bc3504

Browse files
DjDeveloperrDjDeveloperr
committed
fix(objc): preserve bridge object ownership
1 parent 9891875 commit 5bc3504

3 files changed

Lines changed: 104 additions & 42 deletions

File tree

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

Lines changed: 54 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,9 @@ Value get(Runtime& runtime, const PropNameID& name) override {
231231
backingValue_(std::move(backingValue)) {}
232232

233233
~NativeApiReferenceHostObject() override {
234+
for (id object : retainedObjects_) {
235+
[object release];
236+
}
234237
if (ownsData_ && data_ != nullptr) {
235238
free(data_);
236239
data_ = nullptr;
@@ -242,6 +245,7 @@ Value get(Runtime& runtime, const PropNameID& name) override {
242245
std::shared_ptr<Value> backingValue() const { return backingValue_; }
243246
void ensureStorage(Runtime& runtime, NativeApiType type,
244247
NativeApiArgumentFrame& frame, size_t elements = 1);
248+
void retainObjectSlot(size_t index, id object);
245249

246250
Value get(Runtime& runtime, const PropNameID& name) override;
247251
NativeApiHostSetResult set(Runtime& runtime, const PropNameID& name, const Value& value) override;
@@ -262,6 +266,7 @@ void ensureStorage(Runtime& runtime, NativeApiType type,
262266
size_t byteLength_ = 0;
263267
std::shared_ptr<Value> pendingValue_;
264268
std::shared_ptr<Value> backingValue_;
269+
std::vector<id> retainedObjects_;
265270
};
266271

267272
class NativeApiStructObjectHostObject final : public HostObject {
@@ -643,6 +648,9 @@ Array runtimeMembersArray(Runtime& runtime, Class cls, bool staticMembers) {
643648
object_(object),
644649
ownsObject_(ownsObject),
645650
lifetimeState_(std::make_shared<NativeApiObjectLifetimeState>(object)) {
651+
if (bridge_ != nullptr && object_ != nil) {
652+
bridge_->retainObjectExpandoOwner(object_);
653+
}
646654
if (object_ != nil && !ownsObject_) {
647655
[object_ retain];
648656
ownsObject_ = true;
@@ -653,7 +661,9 @@ Array runtimeMembersArray(Runtime& runtime, Class cls, bool staticMembers) {
653661
~NativeApiObjectHostObject() override {
654662
if (bridge_ != nullptr && object_ != nil) {
655663
bridge_->forgetRoundTripValue(object_);
656-
bridge_->forgetObjectExpandos(object_);
664+
bridge_->releaseObjectExpandoOwner(
665+
object_, class_conformsToProtocol(object_getClass(object_),
666+
@protocol(NativeApiClassBuilderProtocol)));
657667
}
658668
if (lifetimeState_ != nullptr) {
659669
lifetimeState_->clear();
@@ -679,11 +689,11 @@ void storeOwnExpando(Runtime& runtime, const std::string& property,
679689
}
680690
}
681691

682-
void disownObject(id expected) {
692+
void disownObject(id expected, bool preserveExpandos = false) {
683693
if (object_ == expected) {
684694
if (bridge_ != nullptr && expected != nil) {
685695
bridge_->forgetRoundTripValue(expected);
686-
bridge_->forgetObjectExpandos(expected);
696+
bridge_->releaseObjectExpandoOwner(expected, preserveExpandos);
687697
}
688698
ownsObject_ = false;
689699
wrapperRetainedObject_ = false;
@@ -737,21 +747,23 @@ throw JSError(runtime,
737747
classWrapper.emplace(classWrapperValue.asObject(runtime));
738748
}
739749
bridge_->forgetRoundTripValue(runtime, receiver);
740-
bridge_->forgetObjectExpandos(receiver);
741750
}
742751

743752
Value result =
744753
callObjCSelector(runtime, bridge_, receiver, false, selectorName, member,
745754
args, count, dispatchSuperClass);
746755
if (initializer) {
747756
id resultObject = nativeObjectFromValue(runtime, result);
748-
disownObject(receiver);
757+
disownObject(receiver, resultObject == receiver);
749758
if (resultObject != nil) {
750759
// Re-adopt the init result on this host object so that JS overrides
751760
// returning `this` still have a valid native object.
752761
object_ = resultObject;
753762
ownsObject_ = true;
754763
wrapperRetainedObject_ = true;
764+
if (bridge_ != nullptr) {
765+
bridge_->retainObjectExpandoOwner(object_);
766+
}
755767
if (lifetimeState_ != nullptr) {
756768
lifetimeState_->setObject(object_);
757769
}
@@ -792,21 +804,23 @@ throw JSError(runtime,
792804
classWrapper.emplace(classWrapperValue.asObject(runtime));
793805
}
794806
bridge_->forgetRoundTripValue(runtime, receiver);
795-
bridge_->forgetObjectExpandos(receiver);
796807
}
797808

798809
Value result = callPreparedObjCSelector(
799810
runtime, bridge_, receiver, false, prepared, args, count,
800811
dispatchSuperClass);
801812
if (initializer) {
802813
id resultObject = nativeObjectFromValue(runtime, result);
803-
disownObject(receiver);
814+
disownObject(receiver, resultObject == receiver);
804815
if (resultObject != nil) {
805816
// Re-adopt the init result on this host object so that JS overrides
806817
// returning `this` still have a valid native object.
807818
object_ = resultObject;
808819
ownsObject_ = true;
809820
wrapperRetainedObject_ = true;
821+
if (bridge_ != nullptr) {
822+
bridge_->retainObjectExpandoOwner(object_);
823+
}
810824
if (lifetimeState_ != nullptr) {
811825
lifetimeState_->setObject(object_);
812826
}
@@ -829,9 +843,8 @@ throw JSError(runtime,
829843
return result;
830844
}
831845

832-
Value prototypeFunctionForProperty(Runtime& runtime,
833-
const std::string& property) {
834-
if (object_ == nil || property.empty()) {
846+
Value classPrototypeForObject(Runtime& runtime) {
847+
if (object_ == nil) {
835848
return Value::undefined();
836849
}
837850

@@ -847,12 +860,32 @@ Value prototypeFunctionForProperty(Runtime& runtime,
847860
runtime, objc_lookUpClass(symbol->runtimeName.c_str()));
848861
}
849862
}
850-
if (!classWrapperValue.isObject()) {
863+
if (classWrapperValue.isObject()) {
864+
Object classWrapper = classWrapperValue.asObject(runtime);
865+
Value prototypeValue = classWrapper.getProperty(runtime, "prototype");
866+
if (prototypeValue.isObject()) {
867+
return prototypeValue;
868+
}
869+
}
870+
return bridge_->findClassPrototype(runtime, object_getClass(object_));
871+
}
872+
873+
Value engineThisValueForObject(Runtime& runtime) {
874+
Value thisValue = bridge_->findRoundTripValue(runtime, object_,
875+
nullptr, true);
876+
if (thisValue.isObject()) {
877+
return thisValue;
878+
}
879+
return makeNativeObjectValue(runtime, bridge_, object_, false);
880+
}
881+
882+
Value prototypeFunctionForProperty(Runtime& runtime,
883+
const std::string& property) {
884+
if (property.empty()) {
851885
return Value::undefined();
852886
}
853887

854-
Object classWrapper = classWrapperValue.asObject(runtime);
855-
Value prototypeValue = classWrapper.getProperty(runtime, "prototype");
888+
Value prototypeValue = classPrototypeForObject(runtime);
856889
if (!prototypeValue.isObject()) {
857890
return Value::undefined();
858891
}
@@ -897,17 +930,7 @@ Value resolveEnginePrototypeGetter(Runtime& runtime,
897930
if (object_ == nil || property.empty()) {
898931
return Value::undefined();
899932
}
900-
Value classWrapperValue =
901-
bridge_->findObjectExpando(runtime, object_, "__nativeApiClassWrapper");
902-
if (!classWrapperValue.isObject()) {
903-
classWrapperValue =
904-
bridge_->findClassValue(runtime, object_getClass(object_));
905-
}
906-
if (!classWrapperValue.isObject()) {
907-
return Value::undefined();
908-
}
909-
Value prototypeValue =
910-
classWrapperValue.asObject(runtime).getProperty(runtime, "prototype");
933+
Value prototypeValue = classPrototypeForObject(runtime);
911934
if (!prototypeValue.isObject()) {
912935
return Value::undefined();
913936
}
@@ -928,8 +951,7 @@ Value resolveEnginePrototypeGetter(Runtime& runtime,
928951
Value getterValue = descriptor.getProperty(runtime, "get");
929952
if (getterValue.isObject() &&
930953
getterValue.asObject(runtime).isFunction(runtime)) {
931-
Value thisValue = bridge_->findRoundTripValue(runtime, object_,
932-
nullptr, true);
954+
Value thisValue = engineThisValueForObject(runtime);
933955
if (thisValue.isObject()) {
934956
*found = true;
935957
return getterValue.asObject(runtime).asFunction(runtime).callWithThis(
@@ -956,17 +978,7 @@ bool invokeEnginePrototypeSetter(Runtime& runtime, const std::string& property,
956978
if (object_ == nil || property.empty()) {
957979
return false;
958980
}
959-
Value classWrapperValue =
960-
bridge_->findObjectExpando(runtime, object_, "__nativeApiClassWrapper");
961-
if (!classWrapperValue.isObject()) {
962-
classWrapperValue =
963-
bridge_->findClassValue(runtime, object_getClass(object_));
964-
}
965-
if (!classWrapperValue.isObject()) {
966-
return false;
967-
}
968-
Value prototypeValue =
969-
classWrapperValue.asObject(runtime).getProperty(runtime, "prototype");
981+
Value prototypeValue = classPrototypeForObject(runtime);
970982
if (!prototypeValue.isObject()) {
971983
return false;
972984
}
@@ -987,8 +999,7 @@ bool invokeEnginePrototypeSetter(Runtime& runtime, const std::string& property,
987999
descriptorValue.asObject(runtime).getProperty(runtime, "set");
9881000
if (setterValue.isObject() &&
9891001
setterValue.asObject(runtime).isFunction(runtime)) {
990-
Value thisValue = bridge_->findRoundTripValue(runtime, object_,
991-
nullptr, true);
1002+
Value thisValue = engineThisValueForObject(runtime);
9921003
if (thisValue.isObject()) {
9931004
Value args[] = {Value(runtime, value)};
9941005
setterValue.asObject(runtime).asFunction(runtime).callWithThis(
@@ -1190,7 +1201,7 @@ Value get(Runtime& runtime, const PropNameID& name) override {
11901201
bool wrapperRetainedObject = self->wrapperRetainedObject_;
11911202
if (self->bridge_ != nullptr) {
11921203
self->bridge_->forgetRoundTripValue(runtime, object);
1193-
self->bridge_->forgetObjectExpandos(object);
1204+
self->bridge_->releaseObjectExpandoOwner(object);
11941205
}
11951206
self->object_ = nil;
11961207
self->ownsObject_ = false;
@@ -1491,7 +1502,9 @@ throw JSError(
14911502
// Engines whose exotic property storage doesn't fall back to own
14921503
// properties need the JS-owned set resolved here: invoke a JS-prototype
14931504
// setter if present, otherwise store the value as a bridge expando.
1494-
if (!invokeEnginePrototypeSetter(runtime, property, value)) {
1505+
bool invokedPrototypeSetter =
1506+
invokeEnginePrototypeSetter(runtime, property, value);
1507+
if (!invokedPrototypeSetter) {
14951508
storeOwnExpando(runtime, property, value);
14961509
}
14971510
NATIVE_API_SET_RETURN(true);

‎NativeScript/ffi/objc/shared/bridge/ObjCBridge.mm‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -958,6 +958,34 @@ void setObjectExpando(Runtime& runtime, const void* native,
958958
objectExpandosGeneration_.fetch_add(1, std::memory_order_release);
959959
}
960960

961+
void retainObjectExpandoOwner(const void* native) {
962+
if (native == nullptr) {
963+
return;
964+
}
965+
objectExpandoOwnerCounts_[
966+
normalizeRuntimePointer(reinterpret_cast<uintptr_t>(native))] += 1;
967+
}
968+
969+
void releaseObjectExpandoOwner(const void* native,
970+
bool preserveExpandos = false) {
971+
if (native == nullptr) {
972+
return;
973+
}
974+
uintptr_t key =
975+
normalizeRuntimePointer(reinterpret_cast<uintptr_t>(native));
976+
auto ownerIt = objectExpandoOwnerCounts_.find(key);
977+
if (ownerIt != objectExpandoOwnerCounts_.end()) {
978+
if (ownerIt->second > 1) {
979+
ownerIt->second -= 1;
980+
return;
981+
}
982+
objectExpandoOwnerCounts_.erase(ownerIt);
983+
}
984+
if (!preserveExpandos) {
985+
forgetObjectExpandos(native);
986+
}
987+
}
988+
961989
Value findObjectExpando(Runtime& runtime, const void* native,
962990
const std::string& property) const {
963991
if (native == nullptr || property.empty()) {
@@ -1013,8 +1041,9 @@ void forgetObjectExpandos(const void* native) {
10131041
if (native == nullptr) {
10141042
return;
10151043
}
1044+
auto key = normalizeRuntimePointer(reinterpret_cast<uintptr_t>(native));
10161045
objectExpandos_.erase(
1017-
normalizeRuntimePointer(reinterpret_cast<uintptr_t>(native)));
1046+
key);
10181047
objectExpandosGeneration_.fetch_add(1, std::memory_order_release);
10191048
}
10201049

@@ -2117,6 +2146,7 @@ static void appendSurfaceMember(
21172146
std::unordered_map<uintptr_t,
21182147
std::unordered_map<std::string, std::shared_ptr<Value>>>
21192148
objectExpandos_;
2149+
std::unordered_map<uintptr_t, size_t> objectExpandoOwnerCounts_;
21202150
std::atomic<uint64_t> objectExpandosGeneration_{1};
21212151
std::unordered_map<Class, std::unordered_map<std::string, CachedPropertyGetter>>
21222152
propertyGetterCache_;

‎NativeScript/ffi/objc/shared/bridge/TypeConv.mm‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,19 @@ bool nativeTypeStoresObjectiveCObject(const NativeApiType& type) {
460460
}
461461
}
462462

463+
void NativeApiReferenceHostObject::retainObjectSlot(size_t index, id object) {
464+
if (retainedObjects_.size() <= index) {
465+
retainedObjects_.resize(index + 1, nil);
466+
}
467+
id previous = retainedObjects_[index];
468+
if (previous == object) {
469+
return;
470+
}
471+
[object retain];
472+
retainedObjects_[index] = object;
473+
[previous release];
474+
}
475+
463476
std::optional<size_t> parseArrayIndexProperty(const std::string& property) {
464477
if (property.empty()) {
465478
return std::nullopt;
@@ -1149,6 +1162,9 @@ throw JSError(runtime,
11491162
if (data_ != nullptr && pendingValue_ != nullptr) {
11501163
Value pending(runtime, *pendingValue_);
11511164
convertEngineArgument(runtime, bridge_, type_, pending, data_, frame);
1165+
if (nativeTypeStoresObjectiveCObject(type_)) {
1166+
retainObjectSlot(0, *static_cast<id*>(data_));
1167+
}
11521168
pendingValue_.reset();
11531169
}
11541170
}
@@ -1218,6 +1234,9 @@ throw JSError(runtime,
12181234
void* slot = static_cast<uint8_t*>(data_) +
12191235
(slotIndex * referenceElementStride(type_));
12201236
convertEngineArgument(runtime, bridge_, type_, value, slot, frame);
1237+
if (nativeTypeStoresObjectiveCObject(type_)) {
1238+
retainObjectSlot(slotIndex, *static_cast<id*>(slot));
1239+
}
12211240
NATIVE_API_SET_RETURN(true);
12221241
}
12231242

0 commit comments

Comments
 (0)