From 6363fb24757c16308e3f2f859c65a6fbbe3a0d10 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Fri, 4 Mar 2022 19:32:18 -0300 Subject: [PATCH 1/3] fix(workers): invalidate cached isolates on dispose --- test-app/runtime/CMakeLists.txt | 1 + .../runtime/src/main/cpp/ArgConverter.cpp | 4 ++++ test-app/runtime/src/main/cpp/ArgConverter.h | 2 ++ .../runtime/src/main/cpp/IsolateDisposer.cpp | 20 +++++++++++++++++++ .../runtime/src/main/cpp/IsolateDisposer.h | 13 ++++++++++++ .../runtime/src/main/cpp/MetadataNode.cpp | 8 ++++++++ test-app/runtime/src/main/cpp/MetadataNode.h | 2 ++ test-app/runtime/src/main/cpp/Runtime.cpp | 2 ++ .../runtime/src/main/cpp/V8GlobalHelpers.cpp | 4 ++++ .../runtime/src/main/cpp/V8GlobalHelpers.h | 2 ++ .../runtime/src/main/cpp/console/Console.cpp | 4 ++++ .../runtime/src/main/cpp/console/Console.h | 3 +++ 12 files changed, 65 insertions(+) create mode 100644 test-app/runtime/src/main/cpp/IsolateDisposer.cpp create mode 100644 test-app/runtime/src/main/cpp/IsolateDisposer.h diff --git a/test-app/runtime/CMakeLists.txt b/test-app/runtime/CMakeLists.txt index d2030f74a..250131f9e 100644 --- a/test-app/runtime/CMakeLists.txt +++ b/test-app/runtime/CMakeLists.txt @@ -145,6 +145,7 @@ add_library( src/main/cpp/DirectBuffer.cpp src/main/cpp/FieldAccessor.cpp src/main/cpp/File.cpp + src/main/cpp/IsolateDisposer.cpp src/main/cpp/JEnv.cpp src/main/cpp/DesugaredInterfaceCompanionClassNameResolver.cpp src/main/cpp/JType.cpp diff --git a/test-app/runtime/src/main/cpp/ArgConverter.cpp b/test-app/runtime/src/main/cpp/ArgConverter.cpp index 236862525..5f2415f2f 100644 --- a/test-app/runtime/src/main/cpp/ArgConverter.cpp +++ b/test-app/runtime/src/main/cpp/ArgConverter.cpp @@ -293,4 +293,8 @@ Local ArgConverter::ConvertToV8UTF16String(Isolate* isolate, const u16st return String::NewFromTwoByte(isolate, ((const uint16_t*) utf16string.data())).ToLocalChecked(); } +void ArgConverter::disposeIsolate(Isolate* isolate) { + s_type_long_operations_cache.erase(isolate); +} + std::map ArgConverter::s_type_long_operations_cache; \ No newline at end of file diff --git a/test-app/runtime/src/main/cpp/ArgConverter.h b/test-app/runtime/src/main/cpp/ArgConverter.h index 1d4bf43c1..4533f7f59 100644 --- a/test-app/runtime/src/main/cpp/ArgConverter.h +++ b/test-app/runtime/src/main/cpp/ArgConverter.h @@ -43,6 +43,8 @@ class ArgConverter { static v8::Local ConvertToV8UTF16String(v8::Isolate* isolate, const std::u16string& utf16string); + static void disposeIsolate(v8::Isolate* isolate); + private: // TODO: plamen5kov: rewrite logic for java long number operations in javascript (java long -> javascript number operations check) diff --git a/test-app/runtime/src/main/cpp/IsolateDisposer.cpp b/test-app/runtime/src/main/cpp/IsolateDisposer.cpp new file mode 100644 index 000000000..016cee6f8 --- /dev/null +++ b/test-app/runtime/src/main/cpp/IsolateDisposer.cpp @@ -0,0 +1,20 @@ +// +// Created by Eduardo Speroni on 3/4/22. +// + +#include "IsolateDisposer.h" +#include "ArgConverter.h" +#include "MetadataNode.h" +#include "V8GlobalHelpers.h" +#include + + + +namespace tns { + void disposeIsolate(v8::Isolate* isolate) { + tns::ArgConverter::disposeIsolate(isolate); + tns::MetadataNode::disposeIsolate(isolate); + tns::disposeHelperIsolate(isolate); + Console::disposeIsolate(isolate); + } +} \ No newline at end of file diff --git a/test-app/runtime/src/main/cpp/IsolateDisposer.h b/test-app/runtime/src/main/cpp/IsolateDisposer.h new file mode 100644 index 000000000..7ce85997c --- /dev/null +++ b/test-app/runtime/src/main/cpp/IsolateDisposer.h @@ -0,0 +1,13 @@ +// +// Created by Eduardo Speroni on 3/4/22. +// + +#ifndef TEST_APP_ISOLATEDISPOSER_H +#define TEST_APP_ISOLATEDISPOSER_H +#include "v8.h" + +namespace tns { + void disposeIsolate(v8::Isolate* isolate); +} + +#endif //TEST_APP_ISOLATEDISPOSER_H diff --git a/test-app/runtime/src/main/cpp/MetadataNode.cpp b/test-app/runtime/src/main/cpp/MetadataNode.cpp index 9dc418bf6..2bae387ca 100644 --- a/test-app/runtime/src/main/cpp/MetadataNode.cpp +++ b/test-app/runtime/src/main/cpp/MetadataNode.cpp @@ -2176,6 +2176,14 @@ std::string MetadataNode::GetJniClassName(MetadataEntry entry) { return fullClassName; } +void MetadataNode::disposeIsolate(Isolate* isolate) { + s_metadata_node_cache.erase(isolate); + s_arrayObjectTemplates.erase(isolate); + for (auto it = s_treeNode2NodeCache.begin(); it != s_treeNode2NodeCache.end(); it++) { + it->second->m_poCtorCachePerIsolate.erase(isolate); + } +} + string MetadataNode::TNS_PREFIX = "com/tns/gen/"; MetadataReader MetadataNode::s_metadataReader; std::map MetadataNode::s_name2NodeCache; diff --git a/test-app/runtime/src/main/cpp/MetadataNode.h b/test-app/runtime/src/main/cpp/MetadataNode.h index e233ae533..e6491b303 100644 --- a/test-app/runtime/src/main/cpp/MetadataNode.h +++ b/test-app/runtime/src/main/cpp/MetadataNode.h @@ -58,6 +58,8 @@ class MetadataNode { static MetadataNode* GetOrCreate(const std::string& className); static std::string GetTypeMetadataName(v8::Isolate* isolate, v8::Local& value); + + static void disposeIsolate(v8::Isolate* isolate); private: struct MethodCallbackData; diff --git a/test-app/runtime/src/main/cpp/Runtime.cpp b/test-app/runtime/src/main/cpp/Runtime.cpp index 95271437a..037eb0b19 100644 --- a/test-app/runtime/src/main/cpp/Runtime.cpp +++ b/test-app/runtime/src/main/cpp/Runtime.cpp @@ -29,6 +29,7 @@ #include "sys/system_properties.h" #include "ManualInstrumentation.h" #include +#include "IsolateDisposer.h" #ifdef APPLICATION_IN_DEBUG #include "JsV8InspectorClient.h" @@ -809,6 +810,7 @@ bool Runtime::RunExtraCode(Isolate* isolate, Local context, const char* void Runtime::DestroyRuntime() { s_id2RuntimeCache.erase(m_id); s_isolate2RuntimesCache.erase(m_isolate); + tns::disposeIsolate(m_isolate); } Local Runtime::GetContext() { diff --git a/test-app/runtime/src/main/cpp/V8GlobalHelpers.cpp b/test-app/runtime/src/main/cpp/V8GlobalHelpers.cpp index 17501ae34..b15b2a82e 100644 --- a/test-app/runtime/src/main/cpp/V8GlobalHelpers.cpp +++ b/test-app/runtime/src/main/cpp/V8GlobalHelpers.cpp @@ -141,3 +141,7 @@ bool tns::V8SetPrivateValue(Isolate* isolate, const Local& obj, const Lo return res.FromMaybe(false); } + +void tns::disposeHelperIsolate(Isolate* isolate) { + isolateToPersistentSmartJSONStringify.erase(isolate); +} diff --git a/test-app/runtime/src/main/cpp/V8GlobalHelpers.h b/test-app/runtime/src/main/cpp/V8GlobalHelpers.h index c13e79311..da6520fe5 100644 --- a/test-app/runtime/src/main/cpp/V8GlobalHelpers.h +++ b/test-app/runtime/src/main/cpp/V8GlobalHelpers.h @@ -13,6 +13,8 @@ v8::Local JsonStringifyObject(v8::Isolate* isolate, v8::Handle& obj, const v8::Local& propName, v8::Local& out); bool V8SetPrivateValue(v8::Isolate* isolate, const v8::Local& obj, const v8::Local& propName, const v8::Local& value); + +void disposeHelperIsolate(v8::Isolate* isolate); } #endif /* V8GLOBALHELPERS_H_ */ diff --git a/test-app/runtime/src/main/cpp/console/Console.cpp b/test-app/runtime/src/main/cpp/console/Console.cpp index 5922f4914..4ad5b6fc4 100644 --- a/test-app/runtime/src/main/cpp/console/Console.cpp +++ b/test-app/runtime/src/main/cpp/console/Console.cpp @@ -554,6 +554,10 @@ void Console::timeEndCallback(const v8::FunctionCallbackInfo& info) { } } +void Console::disposeIsolate(v8::Isolate* isolate) { + s_isolateToConsoleTimersMap.erase(isolate); +} + const char* Console::LOG_TAG = "JS"; std::map> Console::s_isolateToConsoleTimersMap; ConsoleCallback Console::m_callback = nullptr; diff --git a/test-app/runtime/src/main/cpp/console/Console.h b/test-app/runtime/src/main/cpp/console/Console.h index 268b93eee..1735e7b8e 100644 --- a/test-app/runtime/src/main/cpp/console/Console.h +++ b/test-app/runtime/src/main/cpp/console/Console.h @@ -8,6 +8,7 @@ #include #include #include +#include namespace tns { @@ -27,6 +28,8 @@ class Console { static void timeCallback(const v8::FunctionCallbackInfo& info); static void timeEndCallback(const v8::FunctionCallbackInfo& info); + static void disposeIsolate(v8::Isolate* isolate); + private: static int m_maxLogcatObjectSize; static bool m_forceLog; From ee26288407eb01023e56010f98dd2e2bfedebe15 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Fri, 4 Mar 2022 20:19:21 -0300 Subject: [PATCH 2/3] fix: delete cache objects associated to isolates --- .../runtime/src/main/cpp/ArgConverter.cpp | 6 ++++- .../runtime/src/main/cpp/IsolateDisposer.cpp | 5 ++-- .../runtime/src/main/cpp/MetadataNode.cpp | 26 ++++++++++++++++--- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/test-app/runtime/src/main/cpp/ArgConverter.cpp b/test-app/runtime/src/main/cpp/ArgConverter.cpp index 5f2415f2f..8b1f359f0 100644 --- a/test-app/runtime/src/main/cpp/ArgConverter.cpp +++ b/test-app/runtime/src/main/cpp/ArgConverter.cpp @@ -294,7 +294,11 @@ Local ArgConverter::ConvertToV8UTF16String(Isolate* isolate, const u16st } void ArgConverter::disposeIsolate(Isolate* isolate) { - s_type_long_operations_cache.erase(isolate); + auto itFound = s_type_long_operations_cache.find(isolate); + if (itFound != s_type_long_operations_cache.end()) { + delete itFound->second; + s_type_long_operations_cache.erase(itFound); + } } std::map ArgConverter::s_type_long_operations_cache; \ No newline at end of file diff --git a/test-app/runtime/src/main/cpp/IsolateDisposer.cpp b/test-app/runtime/src/main/cpp/IsolateDisposer.cpp index 016cee6f8..8e16f3dfb 100644 --- a/test-app/runtime/src/main/cpp/IsolateDisposer.cpp +++ b/test-app/runtime/src/main/cpp/IsolateDisposer.cpp @@ -9,12 +9,11 @@ #include - namespace tns { - void disposeIsolate(v8::Isolate* isolate) { + void disposeIsolate(v8::Isolate *isolate) { tns::ArgConverter::disposeIsolate(isolate); tns::MetadataNode::disposeIsolate(isolate); tns::disposeHelperIsolate(isolate); Console::disposeIsolate(isolate); } -} \ No newline at end of file +} diff --git a/test-app/runtime/src/main/cpp/MetadataNode.cpp b/test-app/runtime/src/main/cpp/MetadataNode.cpp index 2bae387ca..4317e3483 100644 --- a/test-app/runtime/src/main/cpp/MetadataNode.cpp +++ b/test-app/runtime/src/main/cpp/MetadataNode.cpp @@ -2177,10 +2177,28 @@ std::string MetadataNode::GetJniClassName(MetadataEntry entry) { } void MetadataNode::disposeIsolate(Isolate* isolate) { - s_metadata_node_cache.erase(isolate); - s_arrayObjectTemplates.erase(isolate); - for (auto it = s_treeNode2NodeCache.begin(); it != s_treeNode2NodeCache.end(); it++) { - it->second->m_poCtorCachePerIsolate.erase(isolate); + { + auto it = s_metadata_node_cache.find(isolate); + if (it != s_metadata_node_cache.end()) { + delete it->second; + s_metadata_node_cache.erase(it); + } + } + { + auto it = s_arrayObjectTemplates.find(isolate); + if (it != s_arrayObjectTemplates.end()) { + delete it->second; + s_arrayObjectTemplates.erase(it); + } + } + { + for (auto it = s_treeNode2NodeCache.begin(); it != s_treeNode2NodeCache.end(); it++) { + auto it2 = it->second->m_poCtorCachePerIsolate.find(isolate); + if(it2 != it->second->m_poCtorCachePerIsolate.end()) { + delete it2->second; + it->second->m_poCtorCachePerIsolate.erase(it2); + } + } } } From a0316c849e47517130629579f6558a5674598b38 Mon Sep 17 00:00:00 2001 From: Igor Randjelovic Date: Sat, 5 Mar 2022 20:36:12 +0100 Subject: [PATCH 3/3] refactor: disposeIsolate -> onDisposeIsolate --- test-app/runtime/src/main/cpp/ArgConverter.cpp | 2 +- test-app/runtime/src/main/cpp/ArgConverter.h | 2 +- test-app/runtime/src/main/cpp/IsolateDisposer.cpp | 8 ++++---- test-app/runtime/src/main/cpp/MetadataNode.cpp | 2 +- test-app/runtime/src/main/cpp/MetadataNode.h | 2 +- test-app/runtime/src/main/cpp/V8GlobalHelpers.cpp | 2 +- test-app/runtime/src/main/cpp/V8GlobalHelpers.h | 4 +++- test-app/runtime/src/main/cpp/console/Console.cpp | 2 +- test-app/runtime/src/main/cpp/console/Console.h | 2 +- 9 files changed, 14 insertions(+), 12 deletions(-) diff --git a/test-app/runtime/src/main/cpp/ArgConverter.cpp b/test-app/runtime/src/main/cpp/ArgConverter.cpp index 8b1f359f0..75660ac78 100644 --- a/test-app/runtime/src/main/cpp/ArgConverter.cpp +++ b/test-app/runtime/src/main/cpp/ArgConverter.cpp @@ -293,7 +293,7 @@ Local ArgConverter::ConvertToV8UTF16String(Isolate* isolate, const u16st return String::NewFromTwoByte(isolate, ((const uint16_t*) utf16string.data())).ToLocalChecked(); } -void ArgConverter::disposeIsolate(Isolate* isolate) { +void ArgConverter::onDisposeIsolate(Isolate* isolate) { auto itFound = s_type_long_operations_cache.find(isolate); if (itFound != s_type_long_operations_cache.end()) { delete itFound->second; diff --git a/test-app/runtime/src/main/cpp/ArgConverter.h b/test-app/runtime/src/main/cpp/ArgConverter.h index 4533f7f59..39a1738eb 100644 --- a/test-app/runtime/src/main/cpp/ArgConverter.h +++ b/test-app/runtime/src/main/cpp/ArgConverter.h @@ -43,7 +43,7 @@ class ArgConverter { static v8::Local ConvertToV8UTF16String(v8::Isolate* isolate, const std::u16string& utf16string); - static void disposeIsolate(v8::Isolate* isolate); + static void onDisposeIsolate(v8::Isolate* isolate); private: diff --git a/test-app/runtime/src/main/cpp/IsolateDisposer.cpp b/test-app/runtime/src/main/cpp/IsolateDisposer.cpp index 8e16f3dfb..b5453289a 100644 --- a/test-app/runtime/src/main/cpp/IsolateDisposer.cpp +++ b/test-app/runtime/src/main/cpp/IsolateDisposer.cpp @@ -11,9 +11,9 @@ namespace tns { void disposeIsolate(v8::Isolate *isolate) { - tns::ArgConverter::disposeIsolate(isolate); - tns::MetadataNode::disposeIsolate(isolate); - tns::disposeHelperIsolate(isolate); - Console::disposeIsolate(isolate); + tns::ArgConverter::onDisposeIsolate(isolate); + tns::MetadataNode::onDisposeIsolate(isolate); + tns::V8GlobalHelpers::onDisposeIsolate(isolate); + tns::Console::onDisposeIsolate(isolate); } } diff --git a/test-app/runtime/src/main/cpp/MetadataNode.cpp b/test-app/runtime/src/main/cpp/MetadataNode.cpp index 4317e3483..97e92b58e 100644 --- a/test-app/runtime/src/main/cpp/MetadataNode.cpp +++ b/test-app/runtime/src/main/cpp/MetadataNode.cpp @@ -2176,7 +2176,7 @@ std::string MetadataNode::GetJniClassName(MetadataEntry entry) { return fullClassName; } -void MetadataNode::disposeIsolate(Isolate* isolate) { +void MetadataNode::onDisposeIsolate(Isolate* isolate) { { auto it = s_metadata_node_cache.find(isolate); if (it != s_metadata_node_cache.end()) { diff --git a/test-app/runtime/src/main/cpp/MetadataNode.h b/test-app/runtime/src/main/cpp/MetadataNode.h index e6491b303..739594bdd 100644 --- a/test-app/runtime/src/main/cpp/MetadataNode.h +++ b/test-app/runtime/src/main/cpp/MetadataNode.h @@ -59,7 +59,7 @@ class MetadataNode { static std::string GetTypeMetadataName(v8::Isolate* isolate, v8::Local& value); - static void disposeIsolate(v8::Isolate* isolate); + static void onDisposeIsolate(v8::Isolate* isolate); private: struct MethodCallbackData; diff --git a/test-app/runtime/src/main/cpp/V8GlobalHelpers.cpp b/test-app/runtime/src/main/cpp/V8GlobalHelpers.cpp index b15b2a82e..988ca4c09 100644 --- a/test-app/runtime/src/main/cpp/V8GlobalHelpers.cpp +++ b/test-app/runtime/src/main/cpp/V8GlobalHelpers.cpp @@ -142,6 +142,6 @@ bool tns::V8SetPrivateValue(Isolate* isolate, const Local& obj, const Lo return res.FromMaybe(false); } -void tns::disposeHelperIsolate(Isolate* isolate) { +void tns::V8GlobalHelpers::onDisposeIsolate(Isolate* isolate) { isolateToPersistentSmartJSONStringify.erase(isolate); } diff --git a/test-app/runtime/src/main/cpp/V8GlobalHelpers.h b/test-app/runtime/src/main/cpp/V8GlobalHelpers.h index da6520fe5..b91208304 100644 --- a/test-app/runtime/src/main/cpp/V8GlobalHelpers.h +++ b/test-app/runtime/src/main/cpp/V8GlobalHelpers.h @@ -14,7 +14,9 @@ bool V8GetPrivateValue(v8::Isolate* isolate, const v8::Local& obj, c bool V8SetPrivateValue(v8::Isolate* isolate, const v8::Local& obj, const v8::Local& propName, const v8::Local& value); -void disposeHelperIsolate(v8::Isolate* isolate); +namespace V8GlobalHelpers { + void onDisposeIsolate(v8::Isolate* isolate); +} } #endif /* V8GLOBALHELPERS_H_ */ diff --git a/test-app/runtime/src/main/cpp/console/Console.cpp b/test-app/runtime/src/main/cpp/console/Console.cpp index 4ad5b6fc4..ffcd82437 100644 --- a/test-app/runtime/src/main/cpp/console/Console.cpp +++ b/test-app/runtime/src/main/cpp/console/Console.cpp @@ -554,7 +554,7 @@ void Console::timeEndCallback(const v8::FunctionCallbackInfo& info) { } } -void Console::disposeIsolate(v8::Isolate* isolate) { +void Console::onDisposeIsolate(v8::Isolate* isolate) { s_isolateToConsoleTimersMap.erase(isolate); } diff --git a/test-app/runtime/src/main/cpp/console/Console.h b/test-app/runtime/src/main/cpp/console/Console.h index 1735e7b8e..1da0bcb2b 100644 --- a/test-app/runtime/src/main/cpp/console/Console.h +++ b/test-app/runtime/src/main/cpp/console/Console.h @@ -28,7 +28,7 @@ class Console { static void timeCallback(const v8::FunctionCallbackInfo& info); static void timeEndCallback(const v8::FunctionCallbackInfo& info); - static void disposeIsolate(v8::Isolate* isolate); + static void onDisposeIsolate(v8::Isolate* isolate); private: static int m_maxLogcatObjectSize;