fix memory leaks and runtime cleanup

ammarahm-ed · ammarahm-ed · commit 67bf4ff4c505 · 2025-02-09T12:03:26.000+05:00
diff --git a/test-app/runtime/src/main/cpp/runtime/Runtime.cpp b/test-app/runtime/src/main/cpp/runtime/Runtime.cpp
@@ -38,8 +38,6 @@
 using namespace tns;
 using namespace std;
 
-
-
 bool tns::LogEnabled = false;
 
 void Runtime::Init(JavaVM *vm) {
@@ -219,6 +217,11 @@ void Runtime::Init(JNIEnv *_env, jstring filesPath, jstring nativeLibsDir,
                                      return mode;
                                  });
 
+    napi_util::napi_set_function(env, global, "napiFunction",
+                                 [](napi_env _env, napi_callback_info) -> napi_value {
+                                     return nullptr;
+                                 });
+
     SimpleProfiler::Init(env, global);
 
     CallbackHandlers::CreateGlobalCastFunctions(env);
@@ -338,7 +341,6 @@ ObjectManager *Runtime::GetObjectManager() const {
 }
 
 Runtime::~Runtime() {
-    delete this->m_objectManager;
     delete this->m_loopTimer;
 
 #ifdef __V8__
@@ -369,13 +371,16 @@ std::string Runtime::ReadFileText(const std::string &filePath) {
 }
 
 void Runtime::DestroyRuntime() {
+    MetadataNode::onDisposeEnv(env);
+    ArgConverter::onDisposeEnv(env);
     this->js_method_cache->cleanupCache();
     delete this->js_method_cache;
     this->m_module.DeInit();
     Console::onDisposeEnv(env);
     CallbackHandlers::RemoveEnvEntries(env);
     tns::GlobalHelpers::onDisposeEnv(env);
     napi_close_handle_scope(env, this->global_scope);
+    delete this->m_objectManager;
     js_free_napi_env(env);
     Runtime::thread_id_to_rt_cache.Remove(this->my_thread_id);
     id_to_runtime_cache.Remove(m_id);
diff --git a/test-app/runtime/src/main/cpp/runtime/conversion/ArgConverter.cpp b/test-app/runtime/src/main/cpp/runtime/conversion/ArgConverter.cpp
@@ -248,9 +248,15 @@ u16string ArgConverter::ConvertToUtf16String(napi_env env, napi_value s) {
     }
 }
 
-void ArgConverter::onDisposeIsolate(napi_env env) {
+void ArgConverter::onDisposeEnv(napi_env env) {
     auto itFound = s_type_long_operations_cache.find(env);
     if (itFound != s_type_long_operations_cache.end()) {
+        if (itFound->second->LongNumberCtorFunc) {
+            napi_delete_reference(env, itFound->second->LongNumberCtorFunc);
+        }
+        if (itFound->second->NanNumberObject) {
+            napi_delete_reference(env, itFound->second->NanNumberObject);
+        }
         delete itFound->second;
         s_type_long_operations_cache.erase(itFound);
     }
diff --git a/test-app/runtime/src/main/cpp/runtime/conversion/ArgConverter.h b/test-app/runtime/src/main/cpp/runtime/conversion/ArgConverter.h
@@ -99,7 +99,7 @@ namespace tns {
             return result;
         }
 
-        static void onDisposeIsolate(napi_env env);
+        static void onDisposeEnv(napi_env env);
 
     private:
 
diff --git a/test-app/runtime/src/main/cpp/runtime/metadata/MetadataNode.cpp b/test-app/runtime/src/main/cpp/runtime/metadata/MetadataNode.cpp
@@ -652,14 +652,10 @@ MetadataNode::GetCachedExtendedClassData(napi_env env, const string &proxyClassN
 }
 
 MetadataNode::MetadataNodeCache *MetadataNode::GetMetadataNodeCache(napi_env env) {
-    MetadataNodeCache *cache;
-    auto itFound = s_metadata_node_cache.find(env);
-    if (itFound == s_metadata_node_cache.end()) {
-        cache = new MetadataNodeCache;
-        s_metadata_node_cache.emplace(env, cache);
-    } else {
-        cache = itFound->second;
-    }
+    auto cache = s_metadata_node_cache.Get(env);
+    if (cache) return cache;
+    cache = new MetadataNodeCache;
+    s_metadata_node_cache.Insert(env, cache);
     return cache;
 }
 
@@ -834,61 +830,45 @@ napi_value MetadataNode::CreateWrapper(napi_env env) {
 }
 
 napi_value MetadataNode::PackageGetterCallback(napi_env env, napi_callback_info info) {
-    size_t argc = 1;
-    napi_value args[1];
-    napi_value thisArg;
-    void *data;
-
-    napi_get_cb_info(env, info, &argc, args, &thisArg, &data);
-
+    NAPI_CALLBACK_BEGIN(0)
     try {
-
-        auto *methodInfo = static_cast<PackageGetterMethodData *>(data);
-
-        if (methodInfo->value != nullptr) {
-            napi_value value = napi_util::get_ref_value(env, methodInfo->value);
-            if (!napi_util::is_null_or_undefined(env, value)) {
-                return value;
-            }
+        auto childTreeNode = static_cast<MetadataTreeNode *>(data);
+        auto node = GetOrCreateInternal(childTreeNode->parent);
+        DEBUG_WRITE("Get package item: %s", childTreeNode->name.c_str());
+        auto hiddenPropName = "__" + childTreeNode->name;
+        napi_value hiddenValue;
+        napi_get_named_property(env, jsThis, hiddenPropName.c_str(), &hiddenValue);
+
+        if (!napi_util::is_null_or_undefined(env, hiddenValue)) {
+            DEBUG_WRITE("Get cached package item: %s", hiddenPropName.c_str());
+            return hiddenValue;
         }
 
-        auto node = methodInfo->node;
-
-        uint8_t nodeType = s_metadataReader.GetNodeType(node->m_treeNode);
-
-        auto child = GetChildMetadataForPackage(node, methodInfo->utf8name);
-        auto foundChild = child.treeNode != nullptr;
-        napi_value cachedItem = nullptr;
-        if (foundChild) {
-            auto childNode = MetadataNode::GetOrCreateInternal(child.treeNode);
-            auto cache = GetMetadataNodeCache(env);
-            if (methodInfo->value != nullptr) {
-                cache->CtorFuncCache.erase(childNode->m_treeNode);
-            }
-            cachedItem = childNode->CreateWrapper(env);
-            methodInfo->value = napi_util::make_ref(env, cachedItem);
-
-            uint8_t childNodeType = s_metadataReader.GetNodeType(child.treeNode);
-            bool isInterface = s_metadataReader.IsNodeTypeInterface(childNodeType);
-            if (isInterface) {
-                // For all java interfaces we register the special Symbol.hasInstance property
-                // which is invoked by the instanceof operator (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/hasInstance).
-                // For example:
-                //
-                // Object.defineProperty(android.view.animation.Interpolator, Symbol.hasInstance, {
-                //    value: function(obj) {
-                //        return true;
-                //    }
-                // });
-                RegisterSymbolHasInstanceCallback(env, child, cachedItem);
-            }
-
-            if (node->m_name == "org/json" && child.name == "JSONObject") {
-                JSONObjectHelper::RegisterFromFunction(env, cachedItem);
-            }
+        auto childNode = MetadataNode::GetOrCreateInternal(childTreeNode);
+        hiddenValue = childNode->CreateWrapper(env);
+
+        uint8_t childNodeType = s_metadataReader.GetNodeType(childTreeNode);
+        bool isInterface = s_metadataReader.IsNodeTypeInterface(childNodeType);
+        if (isInterface) {
+            // For all java interfaces we register the special Symbol.hasInstance property
+            // which is invoked by the instanceof operator (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/hasInstance).
+            // For example:
+            //
+            // Object.defineProperty(android.view.animation.Interpolator, Symbol.hasInstance, {
+            //    value: function(obj) {
+            //        return true;
+            //    }
+            // });
+            RegisterSymbolHasInstanceCallback(env, childTreeNode, hiddenValue);
+        }
+        auto parentNode = GetOrCreateInternal(childTreeNode->parent);
+        if (parentNode->m_name == "org/json" && childTreeNode->name == "JSONObject") {
+            JSONObjectHelper::RegisterFromFunction(env, hiddenValue);
         }
 
-        return cachedItem;
+        napi_set_named_property(env, jsThis, hiddenPropName.c_str(), hiddenValue);
+
+        return hiddenValue;
 
     } catch (NativeScriptException &e) {
         e.ReThrowToNapi(env);
@@ -901,18 +881,17 @@ napi_value MetadataNode::PackageGetterCallback(napi_env env, napi_callback_info
         NativeScriptException nsEx(std::string("Error: c++ exception!"));
         nsEx.ReThrowToNapi(env);
     }
-
-    return nullptr;
 }
 
-void MetadataNode::RegisterSymbolHasInstanceCallback(napi_env env, const MetadataEntry &entry,
+void MetadataNode::RegisterSymbolHasInstanceCallback(napi_env env, const MetadataTreeNode* treeNode,
                                                      napi_value interface) {
     if (napi_util::is_undefined(env, interface) || napi_util::is_null(env, interface)) {
         return;
     }
 
     JEnv jEnv;
-    auto className = GetJniClassName(entry);
+
+    auto className = GetJniClassName(treeNode);
     auto clazz = jEnv.FindClass(className);
     if (clazz == nullptr) {
         return;
@@ -976,9 +955,10 @@ napi_value MetadataNode::SymbolHasInstanceCallback(napi_env env, napi_callback_i
 
 }
 
-std::string MetadataNode::GetJniClassName(const MetadataEntry &entry) {
+
+std::string MetadataNode::GetJniClassName(const MetadataTreeNode * node) {
     std::stack<string> s;
-    MetadataTreeNode *node = entry.treeNode;
+
     while (node != nullptr && !node->name.empty()) {
         s.push(node->name);
         node = node->parent;
@@ -1002,29 +982,21 @@ napi_value MetadataNode::CreatePackageObject(napi_env env) {
 
     if (ptrChildren != nullptr) {
         const auto &children = *ptrChildren;
-
         auto lastChildName = "";
-
         for (auto childNode: children) {
-
-            auto *info = new PackageGetterMethodData();
-            info->utf8name = childNode->name.c_str();
-            info->node = this;
-            info->value = nullptr;
-
-            if (strcmp(info->utf8name, lastChildName) == 0) {
+            if (strcmp(childNode->name.c_str(), lastChildName) == 0) {
                 continue;
             }
-            lastChildName = info->utf8name;
+            lastChildName = childNode->name.c_str();
             napi_property_descriptor descriptor{
                     childNode->name.c_str(),
                     nullptr,
                     nullptr,
                     PackageGetterCallback,
                     nullptr,
                     nullptr,
-                    napi_default,
-                    info};
+                    napi_default_jsproperty,
+                    childNode};
             napi_define_properties(env, packageObj, 1, &descriptor);
         }
     }
@@ -1377,10 +1349,22 @@ napi_value MetadataNode::GetConstructorFunctionInternal(napi_env env, MetadataTr
     auto itFound = cache->CtorFuncCache.find(treeNode);
     if (itFound != cache->CtorFuncCache.end()) {
         instanceMethodsCallbackData = itFound->second.instanceMethodCallbacks;
-        if (itFound->second.constructorFunction == nullptr)
-            return nullptr;
-        auto value = napi_util::get_ref_value(env, itFound->second.constructorFunction);
-        if (!napi_util::is_null_or_undefined(env, value)) return value;
+        if (itFound->second.constructorFunction != nullptr) {
+            auto value = napi_util::get_ref_value(env, itFound->second.constructorFunction);
+            if (!napi_util::is_null_or_undefined(env, value)) {
+                return value;
+            }
+        }
+    }
+
+    if (itFound != cache->CtorFuncCache.end()) {
+        if (itFound->second.constructorFunction != nullptr) {
+            napi_delete_reference(env, itFound->second.constructorFunction);
+        }
+        for (const auto &data: itFound->second.instanceMethodCallbacks) {
+            delete data;
+        }
+        cache->CtorFuncCache.erase(itFound);
     }
 
     auto node = GetOrCreateInternal(treeNode);
@@ -2075,15 +2059,34 @@ void MetadataNode::BuildMetadata(const std::string &filesPath) {
 
 void MetadataNode::onDisposeEnv(napi_env env) {
     {
-        auto it = s_metadata_node_cache.find(env);
-        if (it != s_metadata_node_cache.end()) {
-            delete it->second;
-            s_metadata_node_cache.erase(it);
+        auto it = s_metadata_node_cache.Get(env);
+        if (it != nullptr) {
+            for (const auto &entry: it->CtorFuncCache) {
+                if (entry.second.constructorFunction == nullptr) {
+                    napi_delete_reference(env, entry.second.constructorFunction);
+                }
+                for (const auto data: entry.second.instanceMethodCallbacks) {
+                    delete data;
+                }
+            }
+            it->CtorFuncCache.clear();
+
+            for (const auto &entry: it->ExtendedCtorFuncCache) {
+                if (entry.second.extendedCtorFunction == nullptr) {
+                    napi_delete_reference(env, entry.second.extendedCtorFunction);
+                }
+            }
+            it->ExtendedCtorFuncCache.clear();
         }
+        s_metadata_node_cache.Remove(env);
+        delete it;
     }
     {
         auto it = s_arrayObjects.find(env);
         if (it != s_arrayObjects.end()) {
+            if (it->second != nullptr) {
+                napi_delete_reference(env, it->second);
+            }
             s_arrayObjects.erase(it);
         }
     }
@@ -2095,8 +2098,7 @@ MetadataReader MetadataNode::s_metadataReader;
 robin_hood::unordered_map<std::string, MetadataNode *> MetadataNode::s_name2NodeCache;
 robin_hood::unordered_map<std::string, MetadataTreeNode *> MetadataNode::s_name2TreeNodeCache;
 robin_hood::unordered_map<MetadataTreeNode *, MetadataNode *> MetadataNode::s_treeNode2NodeCache;
-robin_hood::unordered_map<napi_env, MetadataNode::MetadataNodeCache *> MetadataNode::s_metadata_node_cache;
+tns::ConcurrentMap<napi_env, MetadataNode::MetadataNodeCache *> MetadataNode::s_metadata_node_cache;
 robin_hood::unordered_map<napi_env, napi_ref> MetadataNode::s_arrayObjects;
 
-// TODO
 bool MetadataNode::s_profilerEnabled = false;
diff --git a/test-app/runtime/src/main/cpp/runtime/metadata/MetadataNode.h b/test-app/runtime/src/main/cpp/runtime/metadata/MetadataNode.h
@@ -104,7 +104,7 @@ class MetadataNode {
     static bool IsValidExtendName(napi_env env, napi_value name);
     static bool GetExtendLocation(napi_env env, std::string& extendLocation, bool isTypeScriptExtend);
     static ExtendedClassCacheData GetCachedExtendedClassData(napi_env env, const std::string& proxyClassName);
-    static std::string GetJniClassName(const MetadataEntry &entry);
+    static std::string GetJniClassName(const MetadataTreeNode* node);
 
 
     static void SetClassAccessor(napi_env env, napi_value constructor);
@@ -178,7 +178,7 @@ class MetadataNode {
     static napi_value NullValueOfCallback(napi_env env, napi_callback_info info);
 
 
-    static void RegisterSymbolHasInstanceCallback(napi_env env, const MetadataEntry &entry, napi_value interface);
+    static void RegisterSymbolHasInstanceCallback(napi_env env, const MetadataTreeNode *treeNode, napi_value interface);
 
     static napi_value SymbolHasInstanceCallback(napi_env env, napi_callback_info info);
 
@@ -200,7 +200,7 @@ class MetadataNode {
     static robin_hood::unordered_map<std::string, MetadataNode *> s_name2NodeCache;
     static robin_hood::unordered_map<std::string, MetadataTreeNode *> s_name2TreeNodeCache;
     static robin_hood::unordered_map<MetadataTreeNode *, MetadataNode *> s_treeNode2NodeCache;
-    static robin_hood::unordered_map<napi_env, MetadataNodeCache *> s_metadata_node_cache;
+    static tns::ConcurrentMap<napi_env, MetadataNodeCache *> s_metadata_node_cache;
     static robin_hood::unordered_map<napi_env, napi_ref> s_arrayObjects;
 
     struct CtorCacheData {
@@ -291,10 +291,7 @@ class MetadataNode {
     };
 
     struct MetadataNodeCache {
-//        napi_ref MetadataKey;
-
         robin_hood::unordered_map<MetadataTreeNode *, CtorCacheData> CtorFuncCache;
-
         robin_hood::unordered_map<std::string, MetadataNode::ExtendedClassCacheData> ExtendedCtorFuncCache;
     };
 
diff --git a/test-app/runtime/src/main/cpp/runtime/objectmanager/ObjectManager.cpp b/test-app/runtime/src/main/cpp/runtime/objectmanager/ObjectManager.cpp
@@ -176,6 +176,22 @@ void ObjectManager::Init(napi_env env) {
     this->m_jsObjectProxyCreator = ref;
 }
 
+ObjectManager::~ObjectManager() {
+    if (this->m_jsObjectCtor) napi_delete_reference(m_env, this->m_jsObjectCtor);
+    if (this->m_jsObjectProxyCreator) napi_delete_reference(m_env, this->m_jsObjectProxyCreator);
+    for (auto &entry: m_idToProxy) {
+        if (!entry.second) continue;
+        napi_delete_reference(m_env, entry.second);
+    }
+    m_idToProxy.clear();
+
+    for (auto &entry: m_idToObject) {
+        if (!entry.second) continue;
+        napi_delete_reference(m_env, entry.second);
+    }
+    m_idToObject.clear();
+}
+
 napi_value ObjectManager::GetOrCreateProxy(jint javaObjectID, napi_value instance) {
     napi_value proxy = nullptr;
     auto it = m_idToProxy.find(javaObjectID);
diff --git a/test-app/runtime/src/main/cpp/runtime/objectmanager/ObjectManager.h b/test-app/runtime/src/main/cpp/runtime/objectmanager/ObjectManager.h

Original file line number	Diff line number	Diff line change
`@@ -248,9 +248,15 @@ u16string ArgConverter::ConvertToUtf16String(napi_env env, napi_value s) {`
`248`	`248`	`}`
`249`	`249`	`}`
`250`	`250`
`251`		`-void ArgConverter::onDisposeIsolate(napi_env env) {`
	`251`	`+void ArgConverter::onDisposeEnv(napi_env env) {`
`252`	`252`	`auto itFound = s_type_long_operations_cache.find(env);`
`253`	`253`	`if (itFound != s_type_long_operations_cache.end()) {`
	`254`	`+ if (itFound->second->LongNumberCtorFunc) {`
	`255`	`+ napi_delete_reference(env, itFound->second->LongNumberCtorFunc);`
	`256`	`+ }`
	`257`	`+ if (itFound->second->NanNumberObject) {`
	`258`	`+ napi_delete_reference(env, itFound->second->NanNumberObject);`
	`259`	`+ }`
`254`	`260`	`delete itFound->second;`
`255`	`261`	`s_type_long_operations_cache.erase(itFound);`
`256`	`262`	`}`
Original file line number	Diff line number	Diff line change
`@@ -99,7 +99,7 @@ namespace tns {`
`99`	`99`	`return result;`
`100`	`100`	`}`
`101`	`101`
`102`		`- static void onDisposeIsolate(napi_env env);`
	`102`	`+ static void onDisposeEnv(napi_env env);`
`103`	`103`
`104`	`104`	`private:`
`105`	`105`