diff --git a/.github/workflows/npm_release.yml b/.github/workflows/npm_release.yml index ac56f2dd1..022861902 100644 --- a/.github/workflows/npm_release.yml +++ b/.github/workflows/npm_release.yml @@ -52,6 +52,10 @@ jobs: cache: gradle - name: Setup Android SDK uses: android-actions/setup-android@651bceb6f9ca583f16b8d75b62c36ded2ae6fc9c # v4.0.0 + with: + # The action's default also asks for 'tools', which the SDK repository + # only lists as obsolete and current sdkmanager versions refuse. + packages: platform-tools - name: Setup NDK run: | echo "y" | sdkmanager "cmake;$CMAKE_VERSION" @@ -151,6 +155,10 @@ jobs: cache: gradle - name: Setup Android SDK uses: android-actions/setup-android@651bceb6f9ca583f16b8d75b62c36ded2ae6fc9c # v4.0.0 + with: + # The action's default also asks for 'tools', which the SDK repository + # only lists as obsolete and current sdkmanager versions refuse. + packages: platform-tools - name: Setup NDK run: | echo "y" | sdkmanager "cmake;$CMAKE_VERSION" diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 455b99fb7..7e4d330a6 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -41,6 +41,10 @@ jobs: cache: gradle - name: Setup Android SDK uses: android-actions/setup-android@651bceb6f9ca583f16b8d75b62c36ded2ae6fc9c # v4.0.0 + with: + # The action's default also asks for 'tools', which the SDK repository + # only lists as obsolete and current sdkmanager versions refuse. + packages: platform-tools - name: Setup NDK run: | echo "y" | sdkmanager "cmake;$CMAKE_VERSION" @@ -123,6 +127,10 @@ jobs: cache: gradle - name: Setup Android SDK uses: android-actions/setup-android@651bceb6f9ca583f16b8d75b62c36ded2ae6fc9c # v4.0.0 + with: + # The action's default also asks for 'tools', which the SDK repository + # only lists as obsolete and current sdkmanager versions refuse. + packages: platform-tools - name: Setup NDK run: | echo "y" | sdkmanager "cmake;3.6.4111459" diff --git a/test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp b/test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp index e553bec6e..ea957448f 100644 --- a/test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp +++ b/test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp @@ -909,7 +909,11 @@ JsV8InspectorClient* JsV8InspectorClient::GetInstance() { // handleMessageOnSocketThread also calls this from the socket thread, so a // concurrent first call is possible: construct, then publish with a CAS and // discard our copy if another thread won the race. - auto* created = new JsV8InspectorClient(Runtime::GetRuntime(0)->GetIsolate()); + Runtime* mainRuntime = Runtime::GetMainRuntime(); + if (mainRuntime == nullptr) { + throw NativeScriptException("Cannot create the inspector: the main runtime is not initialized"); + } + auto* created = new JsV8InspectorClient(mainRuntime->GetIsolate()); JsV8InspectorClient* expected = nullptr; if (!instance.compare_exchange_strong(expected, created, std::memory_order_acq_rel, std::memory_order_acquire)) { diff --git a/test-app/runtime/src/main/cpp/MetadataNode.cpp b/test-app/runtime/src/main/cpp/MetadataNode.cpp index 83e77fa34..f8660949d 100644 --- a/test-app/runtime/src/main/cpp/MetadataNode.cpp +++ b/test-app/runtime/src/main/cpp/MetadataNode.cpp @@ -2042,6 +2042,8 @@ void MetadataNode::BuildMetadata(const string& filesPath) { throw NativeScriptException(ss.str()); } } + // Only opened to tell a missing folder from a missing file. + closedir(dir); string nodesFile = baseDir + "/treeNodeStream.dat"; string namesFile = baseDir + "/treeStringsStream.dat"; @@ -2068,9 +2070,11 @@ void MetadataNode::BuildMetadata(const string& filesPath) { << "-byte records. The metadata is truncated or corrupt."; throw NativeScriptException(ss.str()); } - char* nodes = new char[lenNodes]; + // Owned until the reader takes them, so a file that fails to open further + // down does not strand the buffers already read. + std::unique_ptr nodes(new char[lenNodes]); rewind(f); - fread(nodes, 1, lenNodes, f); + fread(nodes.get(), 1, lenNodes, f); fclose(f); const int _512KB = 524288; @@ -2085,9 +2089,9 @@ void MetadataNode::BuildMetadata(const string& filesPath) { } fseek(f, 0, SEEK_END); int lenNames = ftell(f); - char* names = new char[lenNames + _512KB]; + std::unique_ptr names(new char[lenNames + _512KB]); rewind(f); - fread(names, 1, lenNames, f); + fread(names.get(), 1, lenNames, f); fclose(f); f = fopen(valuesFile.c_str(), "rb"); @@ -2115,11 +2119,9 @@ void MetadataNode::BuildMetadata(const string& filesPath) { DEBUG_WRITE("time=%ld", (millis2 - millis1)); - BuildMetadata(lenNodes, reinterpret_cast(nodes), lenNames, reinterpret_cast(names), lenValues, reinterpret_cast(values)); - - delete[] nodes; - //delete[] names; - //delete[] values; + // The reader keeps the names and values buffers for the life of the + // process and only reads the nodes buffer while it builds the tree. + BuildMetadata(lenNodes, reinterpret_cast(nodes.get()), lenNames, reinterpret_cast(names.release()), lenValues, reinterpret_cast(values)); } void MetadataNode::BuildMetadata(uint32_t nodesLength, uint8_t* nodeData, uint32_t nameLength, uint8_t* nameData, uint32_t valueLength, uint8_t* valueData) { diff --git a/test-app/runtime/src/main/cpp/ObjectManager.h b/test-app/runtime/src/main/cpp/ObjectManager.h index 23194133a..ae4003dc4 100644 --- a/test-app/runtime/src/main/cpp/ObjectManager.h +++ b/test-app/runtime/src/main/cpp/ObjectManager.h @@ -260,7 +260,7 @@ class ObjectManager { static jmethodID CHECK_WEAK_OBJECTS_ARE_ALIVE_METHOD_ID; - v8::Persistent* m_poJsWrapperFunc; + v8::Persistent* m_poJsWrapperFunc = nullptr; }; } // namespace tns diff --git a/test-app/runtime/src/main/cpp/Runtime.cpp b/test-app/runtime/src/main/cpp/Runtime.cpp index d7ab015e8..527096186 100644 --- a/test-app/runtime/src/main/cpp/Runtime.cpp +++ b/test-app/runtime/src/main/cpp/Runtime.cpp @@ -313,6 +313,9 @@ Runtime::~Runtime() { s_isolate2RuntimesCache.erase(it); } } + // Same backstop for the breadcrumb slot Init took: the table is small and + // fixed, so slots lost to failed bootstraps would crowd out live runtimes. + CrashBreadcrumbs::UnregisterRuntime(m_id); delete this->m_objectManager; // idempotent backstop for the matched erase WorkerWrapper does right after @@ -712,9 +715,11 @@ void Runtime::ElectMainRuntime() { s_mainRuntimeElected = true; s_mainRuntimeFailed = false; m_isMainThread = true; - // Once per process: V8::Initialize freezes the flag list, and setting a - // flag afterwards aborts. - InitializeV8(); + // Once per process, not once per election: a main runtime that failed + // hands the election back, and V8 aborts both on a second + // InitializePlatform and on a flag set after V8::Initialize froze the list. + static std::once_flag v8Initialized; + std::call_once(v8Initialized, InitializeV8); return; } @@ -760,6 +765,12 @@ void Runtime::UnwindFailedInit() { DestroyRuntime(); } m_isolate->Dispose(); + // The ~Runtime backstop keys on m_isolate, which is cleared below, so the + // platform's loop entry has to go here. Left behind, it would hand the + // stopped loop to the next isolate allocated at this address. + if (m_eventLoop != nullptr) { + NativeScriptPlatform::Instance()->IsolateDisposed(m_isolate, m_eventLoop); + } m_isolate = nullptr; } @@ -1071,7 +1082,15 @@ Isolate* Runtime::PrepareV8Runtime(const string& filesPath, // Do not build metadata (which should be static for the process) for non-main // threads if (m_isMainThread) { - MetadataNode::BuildMetadata(filesPath); + // Once per process, like V8 itself: the tree is process-wide state that + // outlives the runtime that built it, so a main runtime elected after an + // earlier one failed past this point reads the tree already there. Only + // the elected main runtime gets here, one at a time. + static bool metadataBuilt = false; + if (!metadataBuilt) { + MetadataNode::BuildMetadata(filesPath); + metadataBuilt = true; + } } auto enableProfiler = !profilerOutputDir.empty(); @@ -1089,6 +1108,7 @@ Isolate* Runtime::PrepareV8Runtime(const string& filesPath, s_currentRuntime = this; if (m_isMainThread) { + s_mainRuntime.store(this, std::memory_order_release); // Releases any runtime waiting in ElectMainRuntime: the metadata tree and // the main event loop they depend on are published by now. SignalMainRuntimeReady(false /* failed */); @@ -1164,6 +1184,9 @@ void Runtime::DestroyRuntime() { if (s_currentRuntime == this) { s_currentRuntime = nullptr; } + Runtime* self = this; + s_mainRuntime.compare_exchange_strong(self, nullptr, + std::memory_order_acq_rel); // The events state holds v8::Global handles (backing event target, dispatch // closures and tracked promise rejections) - reset them while the isolate // is still alive. @@ -1251,6 +1274,7 @@ bool Runtime::s_mainRuntimeFailed = false; v8::Platform* Runtime::platform = nullptr; int Runtime::m_androidVersion = Runtime::GetAndroidVersion(); std::shared_ptr Runtime::s_mainEventLoop; +std::atomic Runtime::s_mainRuntime{nullptr}; thread_local Runtime* Runtime::s_currentRuntime = nullptr; thread_local PendingIsolateSetup Runtime::s_pendingIsolateSetup; diff --git a/test-app/runtime/src/main/cpp/Runtime.h b/test-app/runtime/src/main/cpp/Runtime.h index 22f0436fd..40a5ff3d4 100644 --- a/test-app/runtime/src/main/cpp/Runtime.h +++ b/test-app/runtime/src/main/cpp/Runtime.h @@ -202,6 +202,16 @@ class Runtime { static std::shared_ptr GetMainEventLoop() { return s_mainEventLoop; } + + /* + * The main runtime, or null while there is none: before it finishes + * initializing and after it is destroyed. Its id is whatever its + * bootstrap attempt was handed, which is 0 only when the first attempt + * succeeded. + */ + static Runtime* GetMainRuntime() { + return s_mainRuntime.load(std::memory_order_acquire); + } static JavaVM* GetJVM() { return s_jvm; } @@ -351,7 +361,7 @@ class Runtime { v8::Persistent* m_gcFunc; volatile bool m_runGC; - v8::Persistent* m_context; + v8::Persistent* m_context = nullptr; // Decided by ElectMainRuntime, before anything can read it. bool m_isMainThread = false; @@ -422,6 +432,7 @@ class Runtime { static bool s_mainRuntimeFailed; static std::shared_ptr s_mainEventLoop; + static std::atomic s_mainRuntime; static thread_local Runtime* s_currentRuntime; static thread_local PendingIsolateSetup s_pendingIsolateSetup;