Skip to content

Commit f89c0e7

Browse files
DjDeveloperrDjDeveloperr
committed
fix: add round trip frame guard for objc objects
1 parent 0a8aca7 commit f89c0e7

4 files changed

Lines changed: 143 additions & 25 deletions

File tree

‎NativeScript/ffi/ClassMember.mm‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
#include <objc/objc.h>
44
#include <objc/runtime.h>
55
#include <algorithm>
6-
#include <cstdlib>
76
#include <cctype>
7+
#include <cstdlib>
88
#include <cstring>
99
#include <limits>
1010
#include <unordered_set>
@@ -704,6 +704,26 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
704704
return nullptr;
705705
}
706706

707+
class RoundTripCacheFrameGuard {
708+
public:
709+
RoundTripCacheFrameGuard(napi_env env, ObjCBridgeState* bridgeState)
710+
: env_(env), bridgeState_(bridgeState) {
711+
if (bridgeState_ != nullptr) {
712+
bridgeState_->beginRoundTripCacheFrame(env_);
713+
}
714+
}
715+
716+
~RoundTripCacheFrameGuard() {
717+
if (bridgeState_ != nullptr) {
718+
bridgeState_->endRoundTripCacheFrame(env_);
719+
}
720+
}
721+
722+
private:
723+
napi_env env_;
724+
ObjCBridgeState* bridgeState_;
725+
};
726+
707727
napi_value ObjCClassMember::jsCallInit(napi_env env, napi_callback_info cbinfo) {
708728
napi_value jsThis;
709729
ObjCClassMember* method;
@@ -717,6 +737,8 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
717737
return nullptr;
718738
}
719739

740+
RoundTripCacheFrameGuard roundTripCacheFrame(env, method->bridgeState);
741+
720742
SEL sel = method->methodOrGetter.selector;
721743
Class nativeClass = [self class];
722744
std::vector<napi_value> resolvedInitArgs;
@@ -736,9 +758,8 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
736758
return nullptr;
737759
}
738760

739-
ObjCClassMember* newMethod =
740-
findInitializerForArgs(env, &cls->members, nativeClass, argc, callArgs.data(),
741-
&resolvedInitArgs);
761+
ObjCClassMember* newMethod = findInitializerForArgs(env, &cls->members, nativeClass, argc,
762+
callArgs.data(), &resolvedInitArgs);
742763
if (newMethod != nullptr) {
743764
method = newMethod;
744765
} else {
@@ -846,6 +867,8 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
846867
return nullptr;
847868
}
848869

870+
RoundTripCacheFrameGuard roundTripCacheFrame(env, method->bridgeState);
871+
849872
Cif* cif = method->cif;
850873
if (cif == nullptr) {
851874
cif = method->cif =
@@ -971,6 +994,8 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
971994
return nullptr;
972995
}
973996

997+
RoundTripCacheFrameGuard roundTripCacheFrame(env, method->bridgeState);
998+
974999
Cif* cif = method->setterCif;
9751000
if (cif == nullptr) {
9761001
cif = method->setterCif =

‎NativeScript/ffi/ObjCBridge.h‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <string>
1010
#include <unordered_map>
1111
#include <unordered_set>
12+
#include <vector>
1213

1314
#include "AutoreleasePool.h"
1415
#include "CFunction.h"
@@ -86,6 +87,85 @@ class ObjCBridgeState {
8687

8788
void unregisterObject(id object) noexcept;
8889

90+
inline void beginRoundTripCacheFrame(napi_env /*env*/) {
91+
roundTripCacheFrames.emplace_back();
92+
}
93+
94+
inline bool hasRoundTripCacheFrame() const {
95+
return !roundTripCacheFrames.empty();
96+
}
97+
98+
inline void cacheRoundTripObject(napi_env env, id object, napi_value value) {
99+
if (object == nil || roundTripCacheFrames.empty()) {
100+
return;
101+
}
102+
103+
auto& frame = roundTripCacheFrames.back();
104+
if (frame.find(object) != frame.end()) {
105+
return;
106+
}
107+
108+
frame[object] = make_ref(env, value);
109+
}
110+
111+
inline napi_value getRoundTripObject(napi_env env, id object) const {
112+
if (object == nil) {
113+
return nullptr;
114+
}
115+
116+
if (roundTripCacheFrames.empty()) {
117+
auto recent = recentRoundTripCache.find(object);
118+
if (recent == recentRoundTripCache.end()) {
119+
return nullptr;
120+
}
121+
122+
return get_ref_value(env, recent->second);
123+
}
124+
125+
for (auto frame = roundTripCacheFrames.rbegin();
126+
frame != roundTripCacheFrames.rend(); ++frame) {
127+
auto find = frame->find(object);
128+
if (find == frame->end()) {
129+
continue;
130+
}
131+
132+
return get_ref_value(env, find->second);
133+
}
134+
135+
auto recent = recentRoundTripCache.find(object);
136+
if (recent != recentRoundTripCache.end()) {
137+
return get_ref_value(env, recent->second);
138+
}
139+
140+
return nullptr;
141+
}
142+
143+
inline void endRoundTripCacheFrame(napi_env env) {
144+
if (roundTripCacheFrames.empty()) {
145+
return;
146+
}
147+
148+
auto frame = std::move(roundTripCacheFrames.back());
149+
roundTripCacheFrames.pop_back();
150+
151+
if (!roundTripCacheFrames.empty()) {
152+
auto& parent = roundTripCacheFrames.back();
153+
for (auto& entry : frame) {
154+
if (parent.find(entry.first) == parent.end()) {
155+
parent[entry.first] = entry.second;
156+
} else {
157+
napi_delete_reference(env, entry.second);
158+
}
159+
}
160+
return;
161+
}
162+
163+
for (const auto& entry : recentRoundTripCache) {
164+
napi_delete_reference(env, entry.second);
165+
}
166+
recentRoundTripCache = std::move(frame);
167+
}
168+
89169
CFunction* getCFunction(napi_env env, MDSectionOffset offset);
90170

91171
inline StructInfo* getStructInfo(napi_env env, MDSectionOffset offset) {
@@ -145,6 +225,8 @@ class ObjCBridgeState {
145225

146226
private:
147227
std::unordered_map<MDSectionOffset, StructInfo*> structInfoCache;
228+
std::vector<std::unordered_map<id, napi_ref>> roundTripCacheFrames;
229+
std::unordered_map<id, napi_ref> recentRoundTripCache;
148230
void* objc_autoreleasePool;
149231
};
150232

‎NativeScript/ffi/Object.mm‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
#include "Object.h"
2+
#include <cstring>
23
#include "JSObject.h"
34
#include "ObjCBridge.h"
45
#include "js_native_api.h"
56
#include "node_api_util.h"
6-
#include <cstring>
77

88
#import <Foundation/Foundation.h>
99
#include <objc/runtime.h>
@@ -134,6 +134,11 @@ void finalize_objc_object(napi_env /*env*/, void* data, void* hint) {
134134

135135
NAPI_PREAMBLE
136136

137+
auto roundTrip = getRoundTripObject(env, obj);
138+
if (roundTrip != nullptr) {
139+
return roundTrip;
140+
}
141+
137142
auto find = objectRefs.find(obj);
138143
if (find != objectRefs.end()) {
139144
auto value = get_ref_value(env, find->second);
@@ -296,6 +301,11 @@ napi_value findConstructorForObject(napi_env env, ObjCBridgeState* bridgeState,
296301
return nullptr;
297302
}
298303

304+
auto roundTrip = getRoundTripObject(env, obj);
305+
if (roundTrip != nullptr) {
306+
return roundTrip;
307+
}
308+
299309
auto find = objectRefs.find(obj);
300310
if (find != objectRefs.end()) {
301311
auto value = get_ref_value(env, find->second);
@@ -313,8 +323,8 @@ napi_value findConstructorForObject(napi_env env, ObjCBridgeState* bridgeState,
313323
const char* className = objIsClass ? class_getName((Class)obj) : "";
314324
if ((objMetaName != nullptr && std::strstr(objMetaName, "TSObject") != nullptr) ||
315325
(className != nullptr && std::strstr(className, "TSObject") != nullptr)) {
316-
NSLog(@"[getObject class] obj=%p objMeta=%s className=%s classOffset=%u directHit=%d",
317-
obj, objMetaName, className, classOffset, findClass != classesByPointer.end());
326+
NSLog(@"[getObject class] obj=%p objMeta=%s className=%s classOffset=%u directHit=%d", obj,
327+
objMetaName, className, classOffset, findClass != classesByPointer.end());
318328
}
319329
if (findClass != classesByPointer.end()) {
320330
return get_ref_value(env, findClass->second->constructor);

‎NativeScript/ffi/TypeConv.mm‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -988,9 +988,9 @@ void toNative(napi_env env, napi_value value, void* result, bool* shouldFree,
988988

989989
str[len] = '\0';
990990

991-
bool shouldCreateCFString = pointeeType != nullptr &&
992-
(pointeeType->kind == mdTypeNSStringObject ||
993-
pointeeType->kind == mdTypeNSMutableStringObject);
991+
bool shouldCreateCFString =
992+
pointeeType != nullptr && (pointeeType->kind == mdTypeNSStringObject ||
993+
pointeeType->kind == mdTypeNSMutableStringObject);
994994

995995
if (shouldCreateCFString) {
996996
CFStringRef cfStr =
@@ -1105,9 +1105,8 @@ void free(napi_env env, void* value) override {
11051105
return;
11061106
}
11071107

1108-
bool isCFString = pointeeType != nullptr &&
1109-
(pointeeType->kind == mdTypeNSStringObject ||
1110-
pointeeType->kind == mdTypeNSMutableStringObject);
1108+
bool isCFString = pointeeType != nullptr && (pointeeType->kind == mdTypeNSStringObject ||
1109+
pointeeType->kind == mdTypeNSMutableStringObject);
11111110

11121111
if (isCFString) {
11131112
CFRelease((CFStringRef)value);
@@ -1483,8 +1482,7 @@ napi_value toJS(napi_env env, void* value, uint32_t flags) override {
14831482
return null;
14841483
}
14851484

1486-
if ([obj isKindOfClass:[NSNumber class]] &&
1487-
![obj isKindOfClass:[NSDecimalNumber class]]) {
1485+
if ([obj isKindOfClass:[NSNumber class]] && ![obj isKindOfClass:[NSDecimalNumber class]]) {
14881486
if (CFGetTypeID((CFTypeRef)obj) == CFBooleanGetTypeID()) {
14891487
napi_value result;
14901488
napi_get_boolean(env, [obj boolValue], &result);
@@ -1510,6 +1508,11 @@ napi_value toJS(napi_env env, void* value, uint32_t flags) override {
15101508
}
15111509

15121510
auto bridgeState = ObjCBridgeState::InstanceData(env);
1511+
auto roundTrip = bridgeState->getRoundTripObject(env, obj);
1512+
if (roundTrip != nullptr) {
1513+
return roundTrip;
1514+
}
1515+
15131516
auto existing = bridgeState->objectRefs.find(obj);
15141517
if (existing != bridgeState->objectRefs.end()) {
15151518
return get_ref_value(env, existing->second);
@@ -1612,12 +1615,12 @@ void toNative(napi_env env, napi_value value, void* result, bool* shouldFree,
16121615
case napi_function: {
16131616
auto bridgeState = ObjCBridgeState::InstanceData(env);
16141617
auto cacheRoundTrip = [&](id nativeObj) {
1615-
if (nativeObj == nil) {
1618+
if (nativeObj == nil || bridgeState == nullptr ||
1619+
!bridgeState->hasRoundTripCacheFrame()) {
16161620
return;
16171621
}
1618-
if (bridgeState->objectRefs.find(nativeObj) == bridgeState->objectRefs.end()) {
1619-
bridgeState->objectRefs[nativeObj] = make_ref(env, value);
1620-
}
1622+
1623+
bridgeState->cacheRoundTripObject(env, nativeObj, value);
16211624
};
16221625

16231626
if (Pointer::isInstance(env, value)) {
@@ -2195,9 +2198,8 @@ napi_value toJS(napi_env env, void* value, uint32_t flags) override {
21952198
napi_value result;
21962199
napi_create_array_with_length(env, arraySize, &result);
21972200

2198-
size_t elementSize = elementType != nullptr && elementType->type != nullptr
2199-
? elementType->type->size
2200-
: 0;
2201+
size_t elementSize =
2202+
elementType != nullptr && elementType->type != nullptr ? elementType->type->size : 0;
22012203
if (elementSize == 0) {
22022204
elementSize = sizeof(void*);
22032205
}
@@ -2222,9 +2224,8 @@ void toNative(napi_env env, napi_value value, void* result, bool* shouldFree,
22222224
bool* shouldFreeAny) override {
22232225
NAPI_PREAMBLE
22242226

2225-
size_t elementSize = elementType != nullptr && elementType->type != nullptr
2226-
? elementType->type->size
2227-
: 0;
2227+
size_t elementSize =
2228+
elementType != nullptr && elementType->type != nullptr ? elementType->type->size : 0;
22282229
if (elementSize == 0) {
22292230
elementSize = sizeof(void*);
22302231
}

0 commit comments

Comments
 (0)