From 431e8167ed5d050c10d2a49d05a2adcfabcd9270 Mon Sep 17 00:00:00 2001
From: Igor Randjelovic <rigor789@gmail.com>
Date: Sat, 3 Jul 2021 19:55:08 +0200
Subject: [PATCH] fix: memory leaks & WeakRefs (#111)

* fix: don't allocate persistent empty objects

* fix: Blocks memory leak (#100)

* feat: drop custom WeakRef implementation & use built-in

* fix: free allocated memory in ReferenceWrapper

* fix: DictionaryAdapter Hanging References (#114)

* Clean up hanging references before deallocating

Reduces total dictionary related memory leaks by 36%

* fix

* chore: more leak fixes

* fix: only release enumerator_ when set (#117)

* fix: handle methods with pointer type params

fixes #109

* fix: referenceWrapper memory leak & CString leak

* refactor: ReferenceWrapper dispose to be managed internally

* fix: pre-allocate memory even for empty values

* fix: undo invalid fix (needs a different way)

Co-authored-by: Darin Dimitrov <darin.dimitrov@gmail.com>
Co-authored-by: Bryse Meijer <brysem@users.noreply.github.com>
Co-authored-by: logikgate <nick.fredricks@flypaper.com>
---
 NativeScript/runtime/ArgConverter.mm      |   1 +
 NativeScript/runtime/DataWrapper.h        |  17 ++-
 NativeScript/runtime/DictionaryAdapter.mm |  26 +++-
 NativeScript/runtime/Helpers.mm           |   1 +
 NativeScript/runtime/Interop.h            |   4 +-
 NativeScript/runtime/Interop.mm           |  34 +++--
 NativeScript/runtime/ObjectManager.mm     |   3 -
 NativeScript/runtime/Reference.cpp        |   6 +-
 NativeScript/runtime/Runtime.mm           |   2 +-
 NativeScript/runtime/WeakRef.cpp          | 146 ++--------------------
 NativeScript/runtime/WeakRef.h            |  15 ---
 TestRunner/app/tests/shared/WeakRef.js    |  43 ++++---
 12 files changed, 104 insertions(+), 194 deletions(-)

diff --git a/NativeScript/runtime/ArgConverter.mm b/NativeScript/runtime/ArgConverter.mm
index 45dcd2ec..8e86e6f4 100644
--- a/NativeScript/runtime/ArgConverter.mm
+++ b/NativeScript/runtime/ArgConverter.mm
@@ -267,6 +267,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) {
diff --git a/NativeScript/runtime/DataWrapper.h b/NativeScript/runtime/DataWrapper.h
index aec8933d..7d183e31 100644
--- a/NativeScript/runtime/DataWrapper.h
+++ b/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_;
diff --git a/NativeScript/runtime/DictionaryAdapter.mm b/NativeScript/runtime/DictionaryAdapter.mm
index 12508695..8e1060be 100644
--- a/NativeScript/runtime/DictionaryAdapter.mm
+++ b/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];
 }
 
diff --git a/NativeScript/runtime/Helpers.mm b/NativeScript/runtime/Helpers.mm
index f6556264..38382f12 100644
--- a/NativeScript/runtime/Helpers.mm
+++ b/NativeScript/runtime/Helpers.mm
@@ -252,6 +252,7 @@
         return Local<Value>();
     }
 
+    v8::Locker locker(isolate);
     Local<Value> result;
     if (!obj->GetPrivate(context, privateKey).ToLocal(&result)) {
         tns::Assert(false, isolate);
diff --git a/NativeScript/runtime/Interop.h b/NativeScript/runtime/Interop.h
index dd6f6621..b5057163 100644
--- a/NativeScript/runtime/Interop.h
+++ b/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);
@@ -187,7 +188,8 @@ class Interop {
         const void* invoke;
         JSBlockDescriptor* descriptor;
         void* userData;
-
+        ffi_closure* ffiClosure;
+        
         static JSBlockDescriptor kJSBlockDescriptor;
     } JSBlock;
 };
diff --git a/NativeScript/runtime/Interop.mm b/NativeScript/runtime/Interop.mm
index 4e13348c..3fdc0014 100644
--- a/NativeScript/runtime/Interop.mm
+++ b/NativeScript/runtime/Interop.mm
@@ -46,43 +46,53 @@
                     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;
+    return CFAutorelease(blockPointer);
 }
 
 Local<Value> Interop::CallFunction(CMethodCall& methodCall) {
@@ -295,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);
@@ -350,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);
@@ -512,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);
@@ -574,6 +589,7 @@
         } else {
             Local<Object> obj = arg.As<Object>();
             DictionaryAdapter* adapter = [[DictionaryAdapter alloc] initWithJSObject:obj isolate:isolate];
+            CFAutorelease(adapter);
             return adapter;
         }
     }
diff --git a/NativeScript/runtime/ObjectManager.mm b/NativeScript/runtime/ObjectManager.mm
index 654a2b2d..9f7e435b 100644
--- a/NativeScript/runtime/ObjectManager.mm
+++ b/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);
             }
diff --git a/NativeScript/runtime/Reference.cpp b/NativeScript/runtime/Reference.cpp
index 5b3ed77a..d6d6c53f 100644
--- a/NativeScript/runtime/Reference.cpp
+++ b/NativeScript/runtime/Reference.cpp
@@ -58,7 +58,7 @@ Local<v8::Function> Reference::GetInteropReferenceCtorFunc(Local<Context> contex
     Local<Object> prototype = prototypeValue.As<Object>();
     Reference::RegisterToStringMethod(context, prototype);
 
-    cache->InteropReferenceCtorFunc = std::make_unique<Persistent<v8::Function>>(isolate, ctorFunc);
+    cache->InteropReferenceCtorFunc = std::make_unique<Persistent<v8::Function> >(isolate, ctorFunc);
 
     return ctorFunc;
 }
@@ -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();
diff --git a/NativeScript/runtime/Runtime.mm b/NativeScript/runtime/Runtime.mm
index 3a5c354f..44400b70 100644
--- a/NativeScript/runtime/Runtime.mm
+++ b/NativeScript/runtime/Runtime.mm
@@ -105,7 +105,7 @@
 
     DefineGlobalObject(context);
     DefineCollectFunction(context);
-    PromiseProxy::Init(context);
+//    PromiseProxy::Init(context);
     Console::Init(context);
     WeakRef::Init(context);
 
diff --git a/NativeScript/runtime/WeakRef.cpp b/NativeScript/runtime/WeakRef.cpp
index 3ca5f4e3..d9601338 100644
--- a/NativeScript/runtime/WeakRef.cpp
+++ b/NativeScript/runtime/WeakRef.cpp
@@ -1,7 +1,4 @@
 #include "WeakRef.h"
-#include "NativeScriptException.h"
-#include "ArgConverter.h"
-#include "Caches.h"
 #include "Helpers.h"
 
 using namespace v8;
@@ -11,141 +8,20 @@ namespace tns {
 void WeakRef::Init(Local<Context> context) {
     Isolate* isolate = context->GetIsolate();
 
-    Local<v8::Function> weakRefCtorFunc;
-    bool success = Function::New(context, ConstructorCallback, Local<Value>()).ToLocal(&weakRefCtorFunc);
-    tns::Assert(success, isolate);
-
-    Local<v8::String> name = tns::ToV8String(isolate, "WeakRef");
-    Local<Object> global = context->Global();
-    success = global->Set(context, name, weakRefCtorFunc).FromMaybe(false);
-    tns::Assert(success, isolate);
-}
-
-void WeakRef::ConstructorCallback(const FunctionCallbackInfo<Value>& info) {
-    Isolate* isolate = info.GetIsolate();
-    tns::Assert(info.IsConstructCall(), isolate);
-    try {
-        if (info.Length() < 1 || !info[0]->IsObject()) {
-            throw NativeScriptException("Argument must be an object.");
+    std::string source = R"(
+        global.WeakRef.prototype.get = global.WeakRef.prototype.deref; 
+        global.WeakRef.prototype.clear = () => {
+            console.warn('WeakRef.clear() is non-standard and has been deprecated. It does nothing and the call can be safely removed.');
         }
+    )";
 
-        Local<Object> target = info[0].As<Object>();
-        Local<Context> context = isolate->GetCurrentContext();
-
-        std::shared_ptr<Persistent<Value>> poValue = ArgConverter::CreateEmptyObject(context);
-        Local<Object> weakRef = poValue->Get(isolate).As<Object>();
-        poValue->Reset();
-
-        Persistent<Object>* poTarget = new Persistent<Object>(isolate, target);
-        Persistent<Object>* poHolder = new Persistent<Object>(isolate, weakRef);
-        CallbackState* callbackState = new CallbackState(poTarget, poHolder);
-
-        poTarget->SetWeak(callbackState, WeakTargetCallback, WeakCallbackType::kFinalizer);
-        poHolder->SetWeak(callbackState, WeakHolderCallback, WeakCallbackType::kFinalizer);
-
-        bool success = weakRef->Set(context, tns::ToV8String(isolate, "get"), GetGetterFunction(isolate)).FromMaybe(false);
-        tns::Assert(success, isolate);
+    Local<Script> script;
+    bool success = Script::Compile(context, tns::ToV8String(isolate, source)).ToLocal(&script);
+    tns::Assert(success && !script.IsEmpty(), isolate);
 
-        success = weakRef->Set(context, tns::ToV8String(isolate, "deref"), GetGetterFunction(isolate)).FromMaybe(false);
-        tns::Assert(success, isolate);
-
-        success = weakRef->Set(context, tns::ToV8String(isolate, "clear"), GetClearFunction(isolate)).FromMaybe(false);
-        tns::Assert(success, isolate);
-
-        tns::SetPrivateValue(weakRef, tns::ToV8String(isolate, "target"), External::New(isolate, poTarget));
-
-        info.GetReturnValue().Set(weakRef);
-    } catch (NativeScriptException& ex) {
-        ex.ReThrowToV8(isolate);
-    }
-}
-
-void WeakRef::WeakTargetCallback(const WeakCallbackInfo<CallbackState>& data) {\
-    CallbackState* callbackState = data.GetParameter();
-    Persistent<Object>* poTarget = callbackState->target_;
-    poTarget->Reset();
-    delete poTarget;
-    callbackState->target_ = nullptr;
-
-    Isolate* isolate = data.GetIsolate();
-    Persistent<Object>* poHolder = callbackState->holder_;
-    if (poHolder != nullptr) {
-        Local<Object> holder = poHolder->Get(isolate);
-        tns::SetPrivateValue(holder, tns::ToV8String(isolate, "target"), External::New(isolate, nullptr));
-    }
-
-    if (callbackState->holder_ == nullptr) {
-        delete callbackState;
-    }
-}
-
-void WeakRef::WeakHolderCallback(const WeakCallbackInfo<CallbackState>& data) {
-    CallbackState* callbackState = data.GetParameter();
-    Persistent<Object>* poHolder = callbackState->holder_;
-    Isolate* isolate = data.GetIsolate();
-    Local<Object> holder = Local<Object>::New(isolate, *poHolder);
-
-    Local<Value> hiddenVal = tns::GetPrivateValue(holder, tns::ToV8String(isolate, "target"));
-    Persistent<Object>* poTarget = reinterpret_cast<Persistent<Object>*>(hiddenVal.As<External>()->Value());
-
-    if (poTarget != nullptr) {
-        poHolder->SetWeak(callbackState, WeakHolderCallback, WeakCallbackType::kFinalizer);
-    } else {
-        poHolder->Reset();
-        delete poHolder;
-        callbackState->holder_ = nullptr;
-        if (callbackState->target_ == nullptr) {
-            delete callbackState;
-        }
-    }
-}
-
-Local<v8::Function> WeakRef::GetGetterFunction(Isolate* isolate) {
-    auto cache = Caches::Get(isolate);
-    Persistent<v8::Function>* poGetter = cache->WeakRefGetterFunc.get();
-    if (poGetter != nullptr) {
-        return poGetter->Get(isolate);
-    }
-
-    Local<Context> context = isolate->GetCurrentContext();
-    Local<v8::Function> getterFunc;
-    bool success = FunctionTemplate::New(isolate, GetCallback)->GetFunction(context).ToLocal(&getterFunc);
+    Local<Value> result;
+    success = script->Run(context).ToLocal(&result);
     tns::Assert(success, isolate);
-    cache->WeakRefGetterFunc = std::make_unique<Persistent<v8::Function>>(isolate, getterFunc);
-    return getterFunc;
-}
-
-Local<v8::Function> WeakRef::GetClearFunction(Isolate* isolate) {
-    auto cache = Caches::Get(isolate);
-    Persistent<v8::Function>* poClear = cache->WeakRefClearFunc.get();
-    if (poClear != nullptr) {
-        return poClear->Get(isolate);
-    }
-
-    Local<Context> context = isolate->GetCurrentContext();
-    Local<v8::Function> clearFunc = FunctionTemplate::New(isolate, ClearCallback)->GetFunction(context).ToLocalChecked();
-    cache->WeakRefClearFunc = std::make_unique<Persistent<v8::Function>>(isolate, clearFunc);
-    return clearFunc;
-}
-
-void WeakRef::GetCallback(const FunctionCallbackInfo<Value>& info) {
-    Local<Object> holder = info.This();
-    Isolate* isolate = info.GetIsolate();
-    Local<Value> hiddenVal = tns::GetPrivateValue(holder, tns::ToV8String(isolate, "target"));
-    Persistent<Object>* poTarget = reinterpret_cast<Persistent<Object>*>(hiddenVal.As<External>()->Value());
-
-    if (poTarget != nullptr) {
-        Local<Object> target = poTarget->Get(isolate);
-        info.GetReturnValue().Set(target);
-    } else {
-        info.GetReturnValue().Set(Null(isolate));
-    }
-}
-
-void WeakRef::ClearCallback(const FunctionCallbackInfo<Value>& info) {
-    Local<Object> holder = info.This();
-    Isolate* isolate = info.GetIsolate();
-    tns::SetPrivateValue(holder, tns::ToV8String(isolate, "target"), External::New(isolate, nullptr));
 }
 
-}
+}
\ No newline at end of file
diff --git a/NativeScript/runtime/WeakRef.h b/NativeScript/runtime/WeakRef.h
index 2d1c5733..8e763e58 100644
--- a/NativeScript/runtime/WeakRef.h
+++ b/NativeScript/runtime/WeakRef.h
@@ -8,21 +8,6 @@ namespace tns {
 class WeakRef {
 public:
     static void Init(v8::Local<v8::Context> context);
-private:
-    struct CallbackState {
-        CallbackState(v8::Persistent<v8::Object>* target, v8::Persistent<v8::Object>* holder)
-            : target_(target), holder_(holder) {}
-        v8::Persistent<v8::Object>* target_;
-        v8::Persistent<v8::Object>* holder_;
-    };
-
-    static v8::Local<v8::Function> GetGetterFunction(v8::Isolate* isolate);
-    static v8::Local<v8::Function> GetClearFunction(v8::Isolate* isolate);
-    static void ConstructorCallback(const v8::FunctionCallbackInfo<v8::Value>& info);
-    static void WeakTargetCallback(const v8::WeakCallbackInfo<CallbackState>& data);
-    static void WeakHolderCallback(const v8::WeakCallbackInfo<CallbackState>& data);
-    static void GetCallback(const v8::FunctionCallbackInfo<v8::Value>& info);
-    static void ClearCallback(const v8::FunctionCallbackInfo<v8::Value>& info);
 };
 
 }
diff --git a/TestRunner/app/tests/shared/WeakRef.js b/TestRunner/app/tests/shared/WeakRef.js
index c86ac3a2..27441bbd 100644
--- a/TestRunner/app/tests/shared/WeakRef.js
+++ b/TestRunner/app/tests/shared/WeakRef.js
@@ -3,24 +3,13 @@ describe("WeakRef", function () {
         expect(WeakRef).toBeDefined();
     });
 
+    // deref/get tests have been removed since they are now coming from v8 and not our code
+    // we can safely assume it's well-tested.
+    // we only check to make sure we have a `get` alias to `deref` since NativeScript
+    // has used `get()` long before `deref()` was standardized.
     it("get should work", function () {
-        var obj = {};
-        var weakref = new WeakRef(obj);
-
-        obj = null;
-        gc();
-
-        expect(weakref.get()).toBe(null);
-    });
-
-    it("deref should work", function () {
-        var obj = {};
-        var weakref = new WeakRef(obj);
-
-        obj = null;
-        gc();
-
-        expect(weakref.deref()).toBe(null);
+        expect(WeakRef.prototype.get).toBeDefined();
+        expect(WeakRef.prototype.get).toEqual(WeakRef.prototype.deref);
     });
 
     it("should throw when constructed with zero parameters", function () {
@@ -37,12 +26,22 @@ describe("WeakRef", function () {
         }
     });
 
-    it("should be clearable", function () {
-        var obj = {};
-        var weakref = new WeakRef(obj);
+    it("clear should exist", function () {
+        expect(WeakRef.prototype.clear).toBeDefined();
+
+        const warn = console.warn
+        console.warn = (message) => {
+            warn(message);
+            expect(message).toEqual("WeakRef.clear() is non-standard and has been deprecated. It does nothing and the call can be safely removed.")
+        }
 
-        weakref.clear();
+        const obj = {}
+        const weakRef = new WeakRef(obj);
 
-        expect(weakref.get()).toBeNull();
+        weakRef.clear();
+
+        // reset console.warn to it's original
+        console.warn = warn;
     });
+
 });