From 1604b8accc35f45a980899998aa307a17f320d0e Mon Sep 17 00:00:00 2001 From: Ammar Ahmed Date: Mon, 9 Nov 2026 12:13:35 +0500 Subject: [PATCH] fix(android): guard MetadataNode's process-wide node caches --- .../ffi/jni/jsi/metadata/MetadataNode.cpp | 100 +++++++++--------- .../ffi/jni/jsi/metadata/MetadataNode.h | 3 + .../ffi/jni/napi/metadata/MetadataNode.cpp | 100 +++++++++--------- .../ffi/jni/napi/metadata/MetadataNode.h | 3 + 4 files changed, 108 insertions(+), 98 deletions(-) diff --git a/NativeScript/ffi/jni/jsi/metadata/MetadataNode.cpp b/NativeScript/ffi/jni/jsi/metadata/MetadataNode.cpp index 04c6b50a..de813870 100644 --- a/NativeScript/ffi/jni/jsi/metadata/MetadataNode.cpp +++ b/NativeScript/ffi/jni/jsi/metadata/MetadataNode.cpp @@ -900,19 +900,18 @@ void MetadataNode::CreateTopLevelNamespaces(JsRuntime &rt) { } MetadataTreeNode *MetadataNode::GetOrCreateTreeNodeByName(const string &className) { - MetadataTreeNode *result = nullptr; - - auto itFound = s_name2TreeNodeCache.find(className); - - if (itFound != s_name2TreeNodeCache.end()) { - result = itFound->second; - } else { - result = s_metadataReader.GetOrCreateTreeNodeByName(className); - - s_name2TreeNodeCache.emplace(className, result); + { + std::lock_guard lock(s_nodeCacheMutex); + auto itFound = s_name2TreeNodeCache.find(className); + if (itFound != s_name2TreeNodeCache.end()) { + return itFound->second; + } } - return result; + MetadataTreeNode *created = s_metadataReader.GetOrCreateTreeNodeByName(className); + + std::lock_guard lock(s_nodeCacheMutex); + return s_name2TreeNodeCache.emplace(className, created).first->second; } string MetadataNode::GetName() { @@ -920,54 +919,53 @@ string MetadataNode::GetName() { } MetadataNode *MetadataNode::GetOrCreate(const string &className) { - MetadataNode *node = nullptr; - - auto it = s_name2NodeCache.find(className); - - if (it == s_name2NodeCache.end()) { - MetadataTreeNode *treeNode = GetOrCreateTreeNodeByName(className); + { + std::lock_guard lock(s_nodeCacheMutex); + auto it = s_name2NodeCache.find(className); + if (it != s_name2NodeCache.end()) { + return it->second; + } + } - node = GetOrCreateInternal(treeNode); + MetadataTreeNode *treeNode = GetOrCreateTreeNodeByName(className); - s_name2NodeCache.emplace(className, node); - } else { - node = it->second; - } + MetadataNode *node = GetOrCreateInternal(treeNode); - return node; + std::lock_guard lock(s_nodeCacheMutex); + return s_name2NodeCache.emplace(className, node).first->second; } MetadataNode *MetadataNode::GetOrCreateInternal(MetadataTreeNode *treeNode) { - MetadataNode *result = nullptr; + { + std::lock_guard lock(s_nodeCacheMutex); + auto it = s_treeNode2NodeCache.find(treeNode); + if (it != s_treeNode2NodeCache.end()) { + return it->second; + } + } - auto it = s_treeNode2NodeCache.find(treeNode); + auto name = GetJniClassName(treeNode); + if (!name.empty()) { + std::lock_guard lock(s_nodeCacheMutex); + auto it = s_name2NodeCache.find(name); + if (it != s_name2NodeCache.end()) { + s_treeNode2NodeCache.emplace(treeNode, it->second); + return it->second; + } + } - if (it != s_treeNode2NodeCache.end()) { - result = it->second; - } else { - auto name = GetJniClassName(treeNode); - if (!name.empty()) { - auto it2 = s_name2NodeCache.find(name); - if ( it2 != s_name2NodeCache.end()) { - result = it2->second; - } - } + auto *created = new MetadataNode(treeNode); - if (!result) { - result = new MetadataNode(treeNode); - s_treeNode2NodeCache.emplace(treeNode, result); - if (!result->m_name.empty()) { - s_name2NodeCache.emplace(result->m_name, result); - } - } + std::lock_guard lock(s_nodeCacheMutex); + auto inserted = s_treeNode2NodeCache.emplace(treeNode, created); + if (!inserted.second) { + delete created; + return inserted.first->second; } - - auto found = s_treeNode2NodeCache.find(treeNode); - if (found == s_treeNode2NodeCache.end()) { - s_treeNode2NodeCache.emplace(treeNode, result); + if (!created->m_name.empty()) { + s_name2NodeCache.emplace(created->m_name, created); } - - return result; + return created; } MetadataEntry MetadataNode::GetChildMetadataForPackage(MetadataNode *node, const char *propName) { @@ -2125,7 +2123,10 @@ JsValue MetadataNode::ExtendMethodCallback(JsRuntime &rt, const JsValue &thisVal extendFuncCtor.setProperty(rt, PRIVATE_TYPE_NAME, ArgConverter::convertToJsString(rt, fullExtendedName)); - s_name2NodeCache.emplace(fullExtendedName, node); + { + std::lock_guard lock(s_nodeCacheMutex); + s_name2NodeCache.emplace(fullExtendedName, node); + } auto cache = GetMetadataNodeCache(rt); @@ -2711,6 +2712,7 @@ MetadataReader MetadataNode::s_metadataReader; robin_hood::unordered_map MetadataNode::s_name2NodeCache; robin_hood::unordered_map MetadataNode::s_name2TreeNodeCache; robin_hood::unordered_map MetadataNode::s_treeNode2NodeCache; +std::mutex MetadataNode::s_nodeCacheMutex; tns::ConcurrentMap MetadataNode::s_metadata_node_cache; robin_hood::unordered_map MetadataNode::s_arrayObjects; diff --git a/NativeScript/ffi/jni/jsi/metadata/MetadataNode.h b/NativeScript/ffi/jni/jsi/metadata/MetadataNode.h index 826cdf0b..e6ba09b6 100644 --- a/NativeScript/ffi/jni/jsi/metadata/MetadataNode.h +++ b/NativeScript/ffi/jni/jsi/metadata/MetadataNode.h @@ -1,6 +1,7 @@ #ifndef METADATA_NODE_H #define METADATA_NODE_H +#include #include #include "MetadataTreeNode.h" #include "MetadataEntry.h" @@ -269,6 +270,8 @@ class MetadataNode { static robin_hood::unordered_map s_name2NodeCache; static robin_hood::unordered_map s_name2TreeNodeCache; static robin_hood::unordered_map s_treeNode2NodeCache; + // Guards the three caches above. Held only for map access, never across the metadata reader. + static std::mutex s_nodeCacheMutex; // Both keyed by JsRuntime::identity(); &rt is not stable across callbacks. static tns::ConcurrentMap s_metadata_node_cache; static robin_hood::unordered_map s_arrayObjects; diff --git a/NativeScript/ffi/jni/napi/metadata/MetadataNode.cpp b/NativeScript/ffi/jni/napi/metadata/MetadataNode.cpp index d1d56ffb..cd123bd5 100644 --- a/NativeScript/ffi/jni/napi/metadata/MetadataNode.cpp +++ b/NativeScript/ffi/jni/napi/metadata/MetadataNode.cpp @@ -1136,19 +1136,18 @@ void MetadataNode::CreateTopLevelNamespaces(napi_env env) { } MetadataTreeNode *MetadataNode::GetOrCreateTreeNodeByName(const string &className) { - MetadataTreeNode *result = nullptr; - - auto itFound = s_name2TreeNodeCache.find(className); - - if (itFound != s_name2TreeNodeCache.end()) { - result = itFound->second; - } else { - result = s_metadataReader.GetOrCreateTreeNodeByName(className); - - s_name2TreeNodeCache.emplace(className, result); + { + std::lock_guard lock(s_nodeCacheMutex); + auto itFound = s_name2TreeNodeCache.find(className); + if (itFound != s_name2TreeNodeCache.end()) { + return itFound->second; + } } - return result; + MetadataTreeNode *created = s_metadataReader.GetOrCreateTreeNodeByName(className); + + std::lock_guard lock(s_nodeCacheMutex); + return s_name2TreeNodeCache.emplace(className, created).first->second; } string MetadataNode::GetName() { @@ -1156,54 +1155,53 @@ string MetadataNode::GetName() { } MetadataNode *MetadataNode::GetOrCreate(const string &className) { - MetadataNode *node = nullptr; - - auto it = s_name2NodeCache.find(className); - - if (it == s_name2NodeCache.end()) { - MetadataTreeNode *treeNode = GetOrCreateTreeNodeByName(className); + { + std::lock_guard lock(s_nodeCacheMutex); + auto it = s_name2NodeCache.find(className); + if (it != s_name2NodeCache.end()) { + return it->second; + } + } - node = GetOrCreateInternal(treeNode); + MetadataTreeNode *treeNode = GetOrCreateTreeNodeByName(className); - s_name2NodeCache.emplace(className, node); - } else { - node = it->second; - } + MetadataNode *node = GetOrCreateInternal(treeNode); - return node; + std::lock_guard lock(s_nodeCacheMutex); + return s_name2NodeCache.emplace(className, node).first->second; } MetadataNode *MetadataNode::GetOrCreateInternal(MetadataTreeNode *treeNode) { - MetadataNode *result = nullptr; + { + std::lock_guard lock(s_nodeCacheMutex); + auto it = s_treeNode2NodeCache.find(treeNode); + if (it != s_treeNode2NodeCache.end()) { + return it->second; + } + } - auto it = s_treeNode2NodeCache.find(treeNode); + auto name = GetJniClassName(treeNode); + if (!name.empty()) { + std::lock_guard lock(s_nodeCacheMutex); + auto it = s_name2NodeCache.find(name); + if (it != s_name2NodeCache.end()) { + s_treeNode2NodeCache.emplace(treeNode, it->second); + return it->second; + } + } - if (it != s_treeNode2NodeCache.end()) { - result = it->second; - } else { - auto name = GetJniClassName(treeNode); - if (!name.empty()) { - auto it2 = s_name2NodeCache.find(name); - if ( it2 != s_name2NodeCache.end()) { - result = it2->second; - } - } + auto *created = new MetadataNode(treeNode); - if (!result) { - result = new MetadataNode(treeNode); - s_treeNode2NodeCache.emplace(treeNode, result); - if (!result->m_name.empty()) { - s_name2NodeCache.emplace(result->m_name, result); - } - } + std::lock_guard lock(s_nodeCacheMutex); + auto inserted = s_treeNode2NodeCache.emplace(treeNode, created); + if (!inserted.second) { + delete created; + return inserted.first->second; } - - auto found = s_treeNode2NodeCache.find(treeNode); - if (found == s_treeNode2NodeCache.end()) { - s_treeNode2NodeCache.emplace(treeNode, result); + if (!created->m_name.empty()) { + s_name2NodeCache.emplace(created->m_name, created); } - - return result; + return created; } MetadataEntry MetadataNode::GetChildMetadataForPackage(MetadataNode *node, const char *propName) { @@ -2502,7 +2500,10 @@ napi_value MetadataNode::ExtendMethodCallback(napi_env env, napi_callback_info i NAPI_GUARD(napi_set_named_property(env, extendFuncCtor, PRIVATE_TYPE_NAME, ArgConverter::convertToJsString(env, fullExtendedName))) {} - s_name2NodeCache.emplace(fullExtendedName, node); + { + std::lock_guard lock(s_nodeCacheMutex); + s_name2NodeCache.emplace(fullExtendedName, node); + } auto cache = GetMetadataNodeCache(env); @@ -3224,6 +3225,7 @@ MetadataReader MetadataNode::s_metadataReader; robin_hood::unordered_map MetadataNode::s_name2NodeCache; robin_hood::unordered_map MetadataNode::s_name2TreeNodeCache; robin_hood::unordered_map MetadataNode::s_treeNode2NodeCache; +std::mutex MetadataNode::s_nodeCacheMutex; tns::ConcurrentMap MetadataNode::s_metadata_node_cache; robin_hood::unordered_map MetadataNode::s_arrayObjects; diff --git a/NativeScript/ffi/jni/napi/metadata/MetadataNode.h b/NativeScript/ffi/jni/napi/metadata/MetadataNode.h index f0f9aeef..7aaf11e7 100644 --- a/NativeScript/ffi/jni/napi/metadata/MetadataNode.h +++ b/NativeScript/ffi/jni/napi/metadata/MetadataNode.h @@ -1,6 +1,7 @@ #ifndef METADATA_NODE_H #define METADATA_NODE_H +#include #include #include "MetadataTreeNode.h" #include "MetadataEntry.h" @@ -250,6 +251,8 @@ class MetadataNode { static robin_hood::unordered_map s_name2NodeCache; static robin_hood::unordered_map s_name2TreeNodeCache; static robin_hood::unordered_map s_treeNode2NodeCache; + // Guards the three caches above. Held only for map access, never across the metadata reader. + static std::mutex s_nodeCacheMutex; static tns::ConcurrentMap s_metadata_node_cache; static robin_hood::unordered_map s_arrayObjects;