chore: 8.1 memory leak fixes + readme improvements (#123)

Author: NathanWalker

.gitignore

@@ -41,4 +41,4 @@ Thumbs.db
 node_modules/
 package-lock.json
 *.pyc
-v8
+v8

NativeScript/NativeScript-Prefix.pch

@@ -1,7 +1,7 @@
 #ifndef NativeScript_Prefix_pch
 #define NativeScript_Prefix_pch
 
-#define NATIVESCRIPT_VERSION "7.2.0"
+#define NATIVESCRIPT_VERSION "8.1.0-rc.1"
 
 #ifdef DEBUG
 #define SIZEOF_OFF_T 8

NativeScript/runtime/ArgConverter.mm

@@ -268,6 +268,7 @@
                 id adapter = [[DictionaryAdapter alloc] initWithJSObject:value.As<Object>() isolate:isolate];
                 memset(retValue, 0, sizeof(id));
                 *static_cast<id __strong *>(retValue) = adapter;
+                // CFAutorelease(adapter);
                 return;
             }
         } else if (type == BinaryTypeEncodingType::StructDeclarationReference) {

NativeScript/runtime/ClassBuilder.mm

@@ -466,6 +466,8 @@
             return pdw->TypeEncoding()->type;
         } else if (wrapper->Type() == WrapperType::ObjCObject) {
             return BinaryTypeEncodingType::IdEncoding;
+        } else if (wrapper->Type() == WrapperType::PointerType) {
+            return BinaryTypeEncodingType::PointerEncoding;
         }
     }
 

NativeScript/runtime/DataWrapper.h

@@ -259,6 +259,17 @@ class ReferenceWrapper: public BaseDataWrapper {
           disposeData_(false) {
     }
 
+    ~ReferenceWrapper() {
+        if(this->value_ != nullptr) {
+            value_->Reset();
+            delete value_;
+        }
+        
+        if (this->data_ != nullptr) {
+            std::free(this->data_);
+        }
+    }
+
     const WrapperType Type() {
         return WrapperType::Reference;
     }
@@ -291,16 +302,12 @@ class ReferenceWrapper: public BaseDataWrapper {
     }
 
     void SetData(void* data, bool disposeData = false) {
-        if (this->data_ != nullptr && data != nullptr && this->disposeData_) {
+        if (this->data_ != nullptr && this->disposeData_) {
             std::free(this->data_);
         }
         this->data_ = data;
         this->disposeData_ = disposeData;
     }
-
-    bool ShouldDisposeData() {
-        return this->disposeData_;
-    }
 private:
     BaseDataWrapper* typeWrapper_;
     v8::Persistent<v8::Value>* value_;

NativeScript/runtime/DictionaryAdapter.mm

@@ -55,6 +55,10 @@ - (id)nextObject {
 }
 
 - (void)dealloc {
+    self->isolate_ = nullptr;
+    self->map_ = nil;
+    self->cache_ = nil;
+    
     [super dealloc];
 }
 
@@ -138,6 +142,10 @@ - (NSArray*)allObjects {
 }
 
 - (void)dealloc {
+    self->isolate_ = nullptr;
+    self->dictionary_ = nil;
+    self->cache_ = nil;
+    
     [super dealloc];
 }
 
@@ -147,6 +155,7 @@ @implementation DictionaryAdapter {
     Isolate* isolate_;
     std::shared_ptr<Persistent<Value>> object_;
     std::shared_ptr<Caches> cache_;
+    NSEnumerator* enumerator_;
 }
 
 - (instancetype)initWithJSObject:(Local<Object>)jsObject isolate:(Isolate*)isolate {
@@ -225,10 +234,14 @@ - (NSEnumerator*)keyEnumerator {
     Local<Value> obj = self->object_->Get(self->isolate_);
 
     if (obj->IsMap()) {
-        return [[DictionaryAdapterMapKeysEnumerator alloc] initWithMap:self->object_ isolate:self->isolate_ cache:self->cache_];
+        self->enumerator_ = [[DictionaryAdapterMapKeysEnumerator alloc] initWithMap:self->object_ isolate:self->isolate_ cache:self->cache_];
+        
+        return self->enumerator_;
     }
 
-    return [[DictionaryAdapterObjectKeysEnumerator alloc] initWithProperties:self->object_ isolate:self->isolate_ cache:self->cache_];
+    self->enumerator_ = [[DictionaryAdapterObjectKeysEnumerator alloc] initWithProperties:self->object_ isolate:self->isolate_ cache:self->cache_];
+    
+    return self->enumerator_;
 }
 
 - (void)dealloc {
@@ -240,6 +253,15 @@ - (void)dealloc {
         delete wrapper;
     }
     self->object_->Reset();
+    self->isolate_ = nullptr;
+    self->cache_ = nil;
+    self->object_ = nil;
+    
+    if (self->enumerator_ != nullptr) {
+        // CFAutorelease(self->enumerator_);
+        self->enumerator_ = nullptr;
+    }
+    
     [super dealloc];
 }
 

NativeScript/runtime/Helpers.mm

@@ -256,6 +256,7 @@
         return Local<Value>();
     }
 
+    v8::Locker locker(isolate);
     Local<Value> result;
     if (!obj->GetPrivate(context, privateKey).ToLocal(&result)) {
         tns::Assert(false, isolate);

NativeScript/runtime/Interop.h

@@ -126,6 +126,7 @@ class Interop {
     static id ToObject(v8::Local<v8::Context> context, v8::Local<v8::Value> arg);
     static v8::Local<v8::Value> GetPrimitiveReturnType(v8::Local<v8::Context> context, BinaryTypeEncodingType type, BaseCall* call);
 private:
+    static std::pair<IMP, ffi_closure*> CreateMethodInternal(const uint8_t initialParamIndex, const uint8_t argsCount, const TypeEncoding* typeEncoding, FFIMethodCallback callback, void* userData);
     static CFTypeRef CreateBlock(const uint8_t initialParamIndex, const uint8_t argsCount, const TypeEncoding* typeEncoding, FFIMethodCallback callback, void* userData);
     template <typename T>
     static void SetStructValue(v8::Local<v8::Value> value, void* destBuffer, ptrdiff_t position);
@@ -138,6 +139,7 @@ class Interop {
     static void RegisterAdoptFunction(v8::Local<v8::Context> context, v8::Local<v8::Object> interop);
     static void RegisterSizeOfFunction(v8::Local<v8::Context> context, v8::Local<v8::Object> interop);
     static void SetFFIParams(v8::Local<v8::Context> context, const TypeEncoding* typeEncoding, FFICall* call, const int argsCount, const int initialParameterIndex, V8Args& args);
+    static bool isRefTypeEqual(const TypeEncoding* typeEncoding,const char* clazz);
     static v8::Local<v8::Array> ToArray(v8::Local<v8::Object> object);
     static v8::Local<v8::Value> StructToValue(v8::Local<v8::Context> context, void* result, StructInfo structInfo, std::shared_ptr<v8::Persistent<v8::Value>> parentStruct);
     static const TypeEncoding* CreateEncoding(BinaryTypeEncodingType type);
@@ -186,7 +188,8 @@ class Interop {
         const void* invoke;
         JSBlockDescriptor* descriptor;
         void* userData;
-
+        ffi_closure* ffiClosure;
+        
         static JSBlockDescriptor kJSBlockDescriptor;
     } JSBlock;
 };

NativeScript/runtime/Interop.mm

@@ -46,40 +46,50 @@
                     v8::Locker locker(isolate);
                     Isolate::Scope isolate_scope(isolate);
                     HandleScope handle_scope(isolate);
+                    BlockWrapper* blockWrapper = static_cast<BlockWrapper*>(tns::GetValue(isolate, callback));
                     tns::DeleteValue(isolate, callback);
                     wrapper->callback_->Reset();
+                    delete blockWrapper;
                 }
             }
             delete wrapper;
+            ffi_closure_free(block->ffiClosure);
             block->~JSBlock();
         }
     }
 };
 
-IMP Interop::CreateMethod(const uint8_t initialParamIndex, const uint8_t argsCount, const TypeEncoding* typeEncoding, FFIMethodCallback callback, void* userData) {
+std::pair<IMP, ffi_closure*> Interop::CreateMethodInternal(const uint8_t initialParamIndex, const uint8_t argsCount, const TypeEncoding* typeEncoding, FFIMethodCallback callback, void* userData) {
     void* functionPointer;
     ffi_closure* closure = static_cast<ffi_closure*>(ffi_closure_alloc(sizeof(ffi_closure), &functionPointer));
     ParametrizedCall* call = ParametrizedCall::Get(typeEncoding, initialParamIndex, initialParamIndex + argsCount);
     ffi_status status = ffi_prep_closure_loc(closure, call->Cif, callback, userData, functionPointer);
     tns::Assert(status == FFI_OK);
 
-    return (IMP)functionPointer;
+    return std::make_pair((IMP)functionPointer, closure);
+
+}
+
+IMP Interop::CreateMethod(const uint8_t initialParamIndex, const uint8_t argsCount, const TypeEncoding* typeEncoding, FFIMethodCallback callback, void* userData) {
+    std::pair<IMP, ffi_closure*> result = Interop::CreateMethodInternal(initialParamIndex, argsCount, typeEncoding, callback, userData);
+    return result.first;
 }
 
 CFTypeRef Interop::CreateBlock(const uint8_t initialParamIndex, const uint8_t argsCount, const TypeEncoding* typeEncoding, FFIMethodCallback callback, void* userData) {
     JSBlock* blockPointer = reinterpret_cast<JSBlock*>(malloc(sizeof(JSBlock)));
-    void* functionPointer = (void*)CreateMethod(initialParamIndex, argsCount, typeEncoding, callback, userData);
+
+    std::pair<IMP, ffi_closure*> result = Interop::CreateMethodInternal(initialParamIndex, argsCount, typeEncoding, callback, userData);
 
     *blockPointer = {
         .isa = nullptr,
         .flags = JSBlock::BLOCK_HAS_COPY_DISPOSE | JSBlock::BLOCK_NEEDS_FREE | (1 /* ref count */ << 1),
         .reserved = 0,
-        .invoke = functionPointer,
+        .invoke = (void*)result.first,
         .descriptor = &JSBlock::kJSBlockDescriptor,
+        .userData = userData,
+        .ffiClosure = result.second,
     };
 
-    blockPointer->userData = userData;
-
     object_setClass((__bridge id)blockPointer, objc_getClass("__NSMallocBlock__"));
 
     return blockPointer;
@@ -125,13 +135,22 @@
     }
 }
 
+bool Interop::isRefTypeEqual(const TypeEncoding* typeEncoding, const char* clazz){
+    std::string n(&typeEncoding->details.interfaceDeclarationReference.name.value());
+    return n.compare(clazz) == 0;
+}
+
 void Interop::WriteValue(Local<Context> context, const TypeEncoding* typeEncoding, void* dest, Local<Value> arg) {
     Isolate* isolate = context->GetIsolate();
 
     if (arg.IsEmpty() || arg->IsNullOrUndefined()) {
         ffi_type* ffiType = FFICall::GetArgumentType(typeEncoding, true);
         size_t size = ffiType->size;
         memset(dest, 0, size);
+    } else if (tns::IsBool(arg) && typeEncoding->type == BinaryTypeEncodingType::InterfaceDeclarationReference && isRefTypeEqual(typeEncoding, "NSNumber")) {
+        bool value = tns::ToBool(arg);
+        NSNumber *num = [NSNumber numberWithBool: value];
+        Interop::SetValue(dest, num);
     } else if (tns::IsBool(arg) && typeEncoding->type == BinaryTypeEncodingType::IdEncoding) {
         bool value = tns::ToBool(arg);
         NSObject* o = @(value);
@@ -286,8 +305,10 @@
                     Local<Value> value = referenceWrapper->Value() != nullptr ? referenceWrapper->Value()->Get(isolate) : Local<Value>();
                     ffi_type* ffiType = FFICall::GetArgumentType(innerType);
                     data = calloc(1, ffiType->size);
-                    referenceWrapper->SetData(data);
+
+                    referenceWrapper->SetData(data, true);
                     referenceWrapper->SetEncoding(innerType);
+
                     // Initialize the ref/out parameter value before passing it to the function call
                     if (!value.IsEmpty()) {
                         Interop::WriteValue(context, innerType, data, value);
@@ -341,7 +362,7 @@
         BaseDataWrapper* baseWrapper = tns::GetValue(isolate, arg);
         if (baseWrapper != nullptr && baseWrapper->Type() == WrapperType::Block) {
             BlockWrapper* wrapper = static_cast<BlockWrapper*>(baseWrapper);
-            blockPtr = wrapper->Block();
+            blockPtr = Block_copy(wrapper->Block());
         } else {
             std::shared_ptr<Persistent<Value>> poCallback = std::make_shared<Persistent<Value>>(isolate, arg);
             MethodCallbackWrapper* userData = new MethodCallbackWrapper(isolate, poCallback, 1, argsCount, blockTypeEncoding);
@@ -503,13 +524,16 @@
             Local<ArrayBuffer> buffer = arg.As<ArrayBuffer>();
             NSDataAdapter* adapter = [[NSDataAdapter alloc] initWithJSObject:buffer isolate:isolate];
             Interop::SetValue(dest, adapter);
+            // CFAutorelease(adapter);
         } else if (tns::IsArrayOrArrayLike(isolate, obj)) {
             Local<v8::Array> array = Interop::ToArray(obj);
             ArrayAdapter* adapter = [[ArrayAdapter alloc] initWithJSObject:array isolate:isolate];
             Interop::SetValue(dest, adapter);
+            // CFAutorelease(adapter);
         } else {
             DictionaryAdapter* adapter = [[DictionaryAdapter alloc] initWithJSObject:obj isolate:isolate];
             Interop::SetValue(dest, adapter);
+            // CFAutorelease(adapter);
         }
     } else {
         tns::Assert(false, isolate);
@@ -565,6 +589,7 @@
         } else {
             Local<Object> obj = arg.As<Object>();
             DictionaryAdapter* adapter = [[DictionaryAdapter alloc] initWithJSObject:obj isolate:isolate];
+            // CFAutorelease(adapter);
             return adapter;
         }
     }

NativeScript/runtime/MetadataBuilder.mm

@@ -532,6 +532,7 @@
             }
 
             names.emplace(methodName, 0);
+            
         }
     }
 }

NativeScript/runtime/ObjectManager.mm

@@ -112,9 +112,6 @@
         case WrapperType::Reference: {
             ReferenceWrapper* referenceWrapper = static_cast<ReferenceWrapper*>(wrapper);
             if (referenceWrapper->Data() != nullptr) {
-                if (referenceWrapper->ShouldDisposeData()) {
-                    std::free(referenceWrapper->Data());
-                }
                 referenceWrapper->SetData(nullptr);
                 referenceWrapper->SetEncoding(nullptr);
             }

NativeScript/runtime/Reference.cpp

@@ -80,6 +80,10 @@ void Reference::ReferenceConstructorCallback(const FunctionCallbackInfo<Value>&
             val = new Persistent<Value>(isolate, info[1]);
         }
     }
+    
+    if(val != nullptr) {
+        val->SetWeak();
+    }
 
     ReferenceWrapper* wrapper = new ReferenceWrapper(typeWrapper, val);
     Local<Object> thiz = info.This();