Skip to content

Commit 134e9d2

Browse files
committed
Refactor libffi and metadata generator for improved memory management and type handling
- Enhanced memory management in Timers.mm by simplifying deallocation logic. - Improved CFTypeRef handling in TNSReturnsRetained.m to ensure proper memory ownership semantics. - Added metadata generator support for function return ownership tracking with new flags and attributes. - Implemented vector and complex type handling in TypeSpec and MetadataWriter for better type serialization. - Updated test scripts to ensure metadata generator is built before running tests, enhancing build reliability. - Added utility functions for cursor attribute checks in metadata generation.
1 parent 9111dc1 commit 134e9d2

33 files changed

Lines changed: 1087 additions & 442 deletions

File tree

NativeScript/ffi/CFunction.mm

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "ObjCBridge.h"
44
#include "ffi/NativeScriptException.h"
55
#include "ffi/Tasks.h"
6+
#include <cstring>
67
#ifdef ENABLE_JS_RUNTIME
78
#include "jsr.h"
89
#endif
@@ -15,8 +16,8 @@
1516
MDSectionOffset originalOffset = offset;
1617
auto name = metadata->getString(offset);
1718
offset += sizeof(MDSectionOffset);
18-
auto signature = metadata->getOffset(offset);
1919
offset += sizeof(MDSectionOffset);
20+
offset += sizeof(MDFunctionFlag);
2021

2122
napi_property_descriptor prop = {
2223
.utf8name = name,
@@ -134,7 +135,14 @@
134135
}
135136
}
136137

137-
return cif->returnType->toJS(env, rvalue, kCStringAsReference);
138+
MDFunctionFlag functionFlags = bridgeState->metadata->getFunctionFlag(
139+
offset + sizeof(MDSectionOffset) * 2);
140+
uint32_t toJSFlags = kCStringAsReference;
141+
if ((functionFlags & mdFunctionReturnOwned) != 0) {
142+
toJSFlags |= kReturnOwned;
143+
}
144+
145+
return cif->returnType->toJS(env, rvalue, toJSFlags);
138146
}
139147

140148
CFunction::~CFunction() { cif = nullptr; }

NativeScript/ffi/ClassBuilder.mm

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,7 @@ NSUInteger JS_SymbolIteratorCountByEnumerating(id self, SEL _cmd, NSFastEnumerat
523523
closure->func = make_ref(env, func);
524524
else
525525
closure->propertyName = name;
526+
closure->selector = desc->selector;
526527
closure->thisConstructor = constructor;
527528
class_replaceMethod(nativeClass, desc->selector, (IMP)closure->fnptr, encoding);
528529
break;
@@ -537,6 +538,7 @@ NSUInteger JS_SymbolIteratorCountByEnumerating(id self, SEL _cmd, NSFastEnumerat
537538
closure->func = make_ref(env, func);
538539
else
539540
closure->propertyName = name;
541+
closure->selector = desc->selector;
540542
closure->thisConstructor = constructor;
541543
class_replaceMethod(nativeClass, desc->selector, (IMP)closure->fnptr, encoding.c_str());
542544
break;

NativeScript/ffi/ClassMember.mm

Lines changed: 193 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <cstdlib>
88
#include <cstring>
99
#include <limits>
10+
#include <memory>
1011
#include <unordered_set>
1112
#include "ClassBuilder.h"
1213
#include "Closure.h"
@@ -429,6 +430,44 @@ bool canConvertToType(napi_env env, napi_value value, std::shared_ptr<TypeConv>
429430
}
430431
}
431432

433+
inline bool selectorEndsWith(SEL selector, const char* suffix) {
434+
if (selector == nullptr || suffix == nullptr) {
435+
return false;
436+
}
437+
438+
const char* selectorName = sel_getName(selector);
439+
if (selectorName == nullptr) {
440+
return false;
441+
}
442+
443+
size_t selectorLength = strlen(selectorName);
444+
size_t suffixLength = strlen(suffix);
445+
if (selectorLength < suffixLength) {
446+
return false;
447+
}
448+
449+
return strcmp(selectorName + selectorLength - suffixLength, suffix) == 0;
450+
}
451+
452+
inline bool isNSErrorOutMethodSignature(SEL selector, Cif* cif) {
453+
if (cif == nullptr || cif->argc == 0 || cif->argTypes.empty()) {
454+
return false;
455+
}
456+
457+
if (!selectorEndsWith(selector, "error:")) {
458+
return false;
459+
}
460+
461+
auto lastArgType = cif->argTypes[cif->argc - 1];
462+
return lastArgType != nullptr && lastArgType->kind == mdTypePointer;
463+
}
464+
465+
inline void throwArgumentsCountError(napi_env env, size_t actualCount, size_t expectedCount) {
466+
std::string message = "Actual arguments count: \"" + std::to_string(actualCount) +
467+
"\". Expected: \"" + std::to_string(expectedCount) + "\".";
468+
napi_throw_error(env, "NativeScriptException", message.c_str());
469+
}
470+
432471
bool isPlainObjectLiteral(napi_env env, napi_value value) {
433472
if (value == nullptr) {
434473
return false;
@@ -842,6 +881,95 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
842881
ObjCBridgeState* bridgeState_;
843882
};
844883

884+
namespace {
885+
886+
size_t getCifArgumentStorageSize(Cif* cif, unsigned int argumentIndex,
887+
unsigned int implicitArgumentCount) {
888+
if (cif == nullptr || cif->cif.arg_types == nullptr) {
889+
return sizeof(void*);
890+
}
891+
892+
const unsigned int ffiIndex = argumentIndex + implicitArgumentCount;
893+
if (ffiIndex >= cif->cif.nargs) {
894+
return sizeof(void*);
895+
}
896+
897+
ffi_type* ffiArgType = cif->cif.arg_types[ffiIndex];
898+
size_t storageSize = ffiArgType != nullptr ? ffiArgType->size : 0;
899+
if (storageSize == 0) {
900+
storageSize = sizeof(void*);
901+
}
902+
903+
return storageSize;
904+
}
905+
906+
class CifArgumentStorage {
907+
public:
908+
CifArgumentStorage(Cif* cif, unsigned int implicitArgumentCount) {
909+
if (cif == nullptr || cif->argc == 0) {
910+
return;
911+
}
912+
913+
buffers_.resize(cif->argc, nullptr);
914+
for (unsigned int i = 0; i < cif->argc; i++) {
915+
size_t storageSize = getCifArgumentStorageSize(cif, i, implicitArgumentCount);
916+
void* storage = malloc(storageSize);
917+
if (storage == nullptr) {
918+
valid_ = false;
919+
break;
920+
}
921+
922+
memset(storage, 0, storageSize);
923+
buffers_[i] = storage;
924+
}
925+
}
926+
927+
~CifArgumentStorage() {
928+
for (void* buffer : buffers_) {
929+
if (buffer != nullptr) {
930+
free(buffer);
931+
}
932+
}
933+
}
934+
935+
bool valid() const { return valid_; }
936+
937+
void* at(unsigned int index) const {
938+
if (index >= buffers_.size()) {
939+
return nullptr;
940+
}
941+
942+
return buffers_[index];
943+
}
944+
945+
private:
946+
bool valid_ = true;
947+
std::vector<void*> buffers_;
948+
};
949+
950+
std::unique_ptr<void, decltype(&::free)> allocateCifReturnStorage(Cif* cif) {
951+
size_t storageSize = 0;
952+
if (cif != nullptr) {
953+
storageSize = cif->rvalueLength;
954+
if (storageSize == 0 && cif->cif.rtype != nullptr) {
955+
storageSize = cif->cif.rtype->size;
956+
}
957+
}
958+
959+
if (storageSize == 0) {
960+
storageSize = sizeof(void*);
961+
}
962+
963+
void* storage = malloc(storageSize);
964+
if (storage != nullptr) {
965+
memset(storage, 0, storageSize);
966+
}
967+
968+
return std::unique_ptr<void, decltype(&::free)>(storage, &::free);
969+
}
970+
971+
} // namespace
972+
845973
napi_value ObjCClassMember::jsCallInit(napi_env env, napi_callback_info cbinfo) {
846974
napi_value jsThis;
847975
ObjCClassMember* method;
@@ -895,6 +1023,13 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
8951023
method->bridgeState->getMethodCif(env, method->methodOrGetter.signatureOffset);
8961024
}
8971025

1026+
CifArgumentStorage argStorage(cif, 2);
1027+
if (!argStorage.valid()) {
1028+
napi_throw_error(env, "NativeScriptException",
1029+
"Unable to allocate argument storage for Objective-C call.");
1030+
return nullptr;
1031+
}
1032+
8981033
if (!resolvedInitArgs.empty()) {
8991034
argc = resolvedInitArgs.size();
9001035
if (argc != cif->argc) {
@@ -921,7 +1056,7 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
9211056
if (cif->argc > 0) {
9221057
for (unsigned int i = 0; i < cif->argc; i++) {
9231058
shouldFree[i] = false;
924-
avalues[i + 2] = cif->avalues[i];
1059+
avalues[i + 2] = argStorage.at(i);
9251060
cif->argTypes[i]->toNative(env, cif->argv[i], avalues[i + 2], &shouldFree[i], &shouldFreeAny);
9261061
}
9271062
}
@@ -1106,23 +1241,47 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
11061241
return nullptr;
11071242
}
11081243

1244+
CifArgumentStorage argStorage(cif, 2);
1245+
if (!argStorage.valid()) {
1246+
napi_throw_error(env, "NativeScriptException",
1247+
"Unable to allocate argument storage for Objective-C call.");
1248+
return nullptr;
1249+
}
1250+
1251+
auto rvalueStorage = allocateCifReturnStorage(cif);
1252+
if (!rvalueStorage) {
1253+
napi_throw_error(env, "NativeScriptException",
1254+
"Unable to allocate return value storage for Objective-C call.");
1255+
return nullptr;
1256+
}
1257+
1258+
SEL selectedSelector = selectedMethod->selector;
1259+
const bool isNSErrorOutMethod = isNSErrorOutMethodSignature(selectedSelector, cif);
1260+
if (!cif->isVariadic && isNSErrorOutMethod) {
1261+
if (actualArgc > cif->argc || actualArgc + 1 < cif->argc) {
1262+
throwArgumentsCountError(env, actualArgc, cif->argc);
1263+
return nullptr;
1264+
}
1265+
}
1266+
11091267
napi_value jsUndefined = nullptr;
11101268
napi_get_undefined(env, &jsUndefined);
11111269
for (size_t i = 0; i < cif->argc; i++) {
11121270
cif->argv[i] = i < jsArgs.size() ? jsArgs[i] : jsUndefined;
11131271
}
11141272

1115-
SEL selectedSelector = selectedMethod->selector;
1116-
11171273
void* avalues[cif->cif.nargs];
1118-
void* rvalue = cif->rvalue;
1274+
void* rvalue = rvalueStorage.get();
11191275

11201276
avalues[0] = (void*)&self;
11211277
avalues[1] = (void*)&selectedSelector;
11221278

11231279
bool shouldFreeAny = false;
11241280
bool shouldFree[cif->argc];
11251281
std::vector<id> fallbackBlocksToRelease;
1282+
NSError* implicitNSError = nil;
1283+
const bool hasImplicitNSErrorOutArg =
1284+
isNSErrorOutMethod && !cif->isVariadic && actualArgc + 1 == cif->argc;
11261285

11271286
auto blockEncodingForSelector = [](const char* selectorName,
11281287
unsigned int argIndex) -> const char* {
@@ -1146,10 +1305,15 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
11461305
if (cif->argc > 0) {
11471306
for (unsigned int i = 0; i < cif->argc; i++) {
11481307
shouldFree[i] = false;
1149-
avalues[i + 2] = cif->avalues[i];
1308+
avalues[i + 2] = argStorage.at(i);
11501309
const char* selectorNameForLog = sel_getName(selectedSelector);
11511310
const char* blockEncoding = blockEncodingForSelector(selectorNameForLog, i);
11521311

1312+
if (hasImplicitNSErrorOutArg && i == cif->argc - 1) {
1313+
*((NSError***)avalues[i + 2]) = &implicitNSError;
1314+
continue;
1315+
}
1316+
11531317
bool convertedViaBlockFallback = false;
11541318
if (blockEncoding != nullptr && cif->argTypes[i]->kind == mdTypeAnyObject) {
11551319
napi_valuetype jsArgType = napi_undefined;
@@ -1189,6 +1353,14 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
11891353
}
11901354
}
11911355

1356+
if (hasImplicitNSErrorOutArg && implicitNSError != nil) {
1357+
const char* errorMessage = [[implicitNSError description] UTF8String];
1358+
NativeScriptException nativeScriptException(errorMessage != nullptr ? errorMessage
1359+
: "Unknown NSError");
1360+
nativeScriptException.ReThrowToJS(env);
1361+
return nullptr;
1362+
}
1363+
11921364
const char* selectorName = sel_getName(selectedSelector);
11931365
if (strcmp(selectorName, "class") == 0) {
11941366
const bool receiverIsClass = class_isMetaClass(object_getClass(self));
@@ -1248,8 +1420,15 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
12481420
method->bridgeState->getMethodCif(env, method->methodOrGetter.signatureOffset);
12491421
}
12501422

1423+
auto rvalueStorage = allocateCifReturnStorage(cif);
1424+
if (!rvalueStorage) {
1425+
napi_throw_error(env, "NativeScriptException",
1426+
"Unable to allocate return value storage for Objective-C getter call.");
1427+
return nullptr;
1428+
}
1429+
12511430
void* avalues[2] = {&self, &method->methodOrGetter.selector};
1252-
void* rvalue = cif->rvalue;
1431+
void* rvalue = rvalueStorage.get();
12531432

12541433
// NSLog(@"objcNativeCall: %p, %@", self, NSStringFromSelector(method->methodOrGetter.selector));
12551434

@@ -1303,7 +1482,14 @@ inline id assertSelf(napi_env env, napi_value jsThis) {
13031482
method->bridgeState->getMethodCif(env, method->setter.signatureOffset);
13041483
}
13051484

1306-
void* avalues[3] = {&self, &method->setter.selector, cif->avalues[0]};
1485+
CifArgumentStorage argStorage(cif, 2);
1486+
if (!argStorage.valid()) {
1487+
napi_throw_error(env, "NativeScriptException",
1488+
"Unable to allocate argument storage for Objective-C setter call.");
1489+
return nullptr;
1490+
}
1491+
1492+
void* avalues[3] = {&self, &method->setter.selector, argStorage.at(0)};
13071493
void* rvalue = nullptr;
13081494

13091495
bool shouldFree = false;

NativeScript/ffi/Closure.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ class Closure {
3030
bool isGetter = false;
3131
bool isSetter = false;
3232
std::string propertyName;
33+
SEL selector = nullptr;
3334
napi_threadsafe_function tsfn;
3435

3536
std::thread::id jsThreadId = std::this_thread::get_id();

0 commit comments

Comments
 (0)