diff --git a/Core/AppRuntime/Include/Babylon/AppRuntime.h b/Core/AppRuntime/Include/Babylon/AppRuntime.h index e001fa32..a7371e6b 100644 --- a/Core/AppRuntime/Include/Babylon/AppRuntime.h +++ b/Core/AppRuntime/Include/Babylon/AppRuntime.h @@ -76,6 +76,10 @@ namespace Babylon // queue explicitly (Napi::DrainJobs / JS_ExecutePendingJob). void DrainMicrotasks(Napi::Env env); + // Engine-specific maintenance that should run once after a complete + // dispatcher turn, rather than after each callback in that turn. + void DrainPostDispatchWork(Napi::Env env); + Options m_options; class Impl; diff --git a/Core/AppRuntime/Source/AppRuntime.cpp b/Core/AppRuntime/Source/AppRuntime.cpp index 176bc849..9a8b49a5 100644 --- a/Core/AppRuntime/Source/AppRuntime.cpp +++ b/Core/AppRuntime/Source/AppRuntime.cpp @@ -84,7 +84,10 @@ namespace Babylon while (!m_impl->m_cancelSource.cancelled()) { - m_impl->m_dispatcher.blocking_tick(m_impl->m_cancelSource); + if (m_impl->m_dispatcher.blocking_tick(m_impl->m_cancelSource)) + { + DrainPostDispatchWork(env); + } } // The dispatcher can be non-empty if something is dispatched after cancellation. diff --git a/Core/AppRuntime/Source/AppRuntime_Chakra.cpp b/Core/AppRuntime/Source/AppRuntime_Chakra.cpp index 315954c2..bf36a3b7 100644 --- a/Core/AppRuntime/Source/AppRuntime_Chakra.cpp +++ b/Core/AppRuntime/Source/AppRuntime_Chakra.cpp @@ -78,4 +78,8 @@ namespace Babylon // JsSetPromiseContinuationCallback hook (see RunEnvironmentTier). // No explicit pump needed here. } + + void AppRuntime::DrainPostDispatchWork(Napi::Env) + { + } } diff --git a/Core/AppRuntime/Source/AppRuntime_Hermes.cpp b/Core/AppRuntime/Source/AppRuntime_Hermes.cpp index 12debfcb..02b5b941 100644 --- a/Core/AppRuntime/Source/AppRuntime_Hermes.cpp +++ b/Core/AppRuntime/Source/AppRuntime_Hermes.cpp @@ -25,4 +25,8 @@ namespace Babylon // observes the same "between turns" semantics it gets on V8/Chakra. Napi::DrainJobs(env); } + + void AppRuntime::DrainPostDispatchWork(Napi::Env) + { + } } diff --git a/Core/AppRuntime/Source/AppRuntime_JSI.cpp b/Core/AppRuntime/Source/AppRuntime_JSI.cpp index a7c809c7..f35fa314 100644 --- a/Core/AppRuntime/Source/AppRuntime_JSI.cpp +++ b/Core/AppRuntime/Source/AppRuntime_JSI.cpp @@ -50,4 +50,8 @@ namespace Babylon { // JSI/V8 backed JSI auto-drains microtasks per scope. } + + void AppRuntime::DrainPostDispatchWork(Napi::Env) + { + } } diff --git a/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp b/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp index b1334c22..1d8f1c25 100644 --- a/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp +++ b/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp @@ -28,4 +28,8 @@ namespace Babylon { // JavaScriptCore drains microtasks automatically at script boundaries. } + + void AppRuntime::DrainPostDispatchWork(Napi::Env) + { + } } diff --git a/Core/AppRuntime/Source/AppRuntime_QuickJS.cpp b/Core/AppRuntime/Source/AppRuntime_QuickJS.cpp index a3bf75da..a7ffb621 100644 --- a/Core/AppRuntime/Source/AppRuntime_QuickJS.cpp +++ b/Core/AppRuntime/Source/AppRuntime_QuickJS.cpp @@ -64,4 +64,8 @@ namespace Babylon { } } + + void AppRuntime::DrainPostDispatchWork(Napi::Env) + { + } } diff --git a/Core/AppRuntime/Source/AppRuntime_V8.cpp b/Core/AppRuntime/Source/AppRuntime_V8.cpp index 89928dcf..933ccd1c 100644 --- a/Core/AppRuntime/Source/AppRuntime_V8.cpp +++ b/Core/AppRuntime/Source/AppRuntime_V8.cpp @@ -115,7 +115,13 @@ namespace Babylon void AppRuntime::DrainMicrotasks(Napi::Env) { - // V8 auto-drains microtasks at the end of each script/callback when - // using the default MicrotasksPolicy. No explicit pump needed. + // V8 auto-drains microtasks at the end of each script/callback. + } + + void AppRuntime::DrainPostDispatchWork(Napi::Env env) + { + // N-API finalizers deferred by a V8 garbage collection require a safe + // host boundary, but should not monopolize a dispatcher turn. + Napi::DrainFinalizers(env); } } diff --git a/Core/Node-API/Include/Engine/V8/napi/env.h b/Core/Node-API/Include/Engine/V8/napi/env.h index 22334ec5..65406498 100644 --- a/Core/Node-API/Include/Engine/V8/napi/env.h +++ b/Core/Node-API/Include/Engine/V8/napi/env.h @@ -52,6 +52,8 @@ namespace Napi void Detach(Napi::Env); + void DrainFinalizers(Napi::Env); + Napi::Value Eval(Napi::Env env, const char* source, const char* sourceUrl); v8::Local GetContext(Napi::Env); diff --git a/Core/Node-API/Source/env_v8.cc b/Core/Node-API/Source/env_v8.cc index 1fbd7d30..fd2575d9 100644 --- a/Core/Node-API/Source/env_v8.cc +++ b/Core/Node-API/Source/env_v8.cc @@ -2,8 +2,15 @@ #include #include "js_native_api_v8.h" +#include + namespace Napi { + namespace + { + constexpr auto FinalizerDrainBudget{std::chrono::milliseconds{8}}; + } + Env Attach(v8::Local isolate) { // second argument is module version @@ -16,6 +23,26 @@ namespace Napi env_ptr->DeleteMe(); } + void DrainFinalizers(Env env) + { + napi_env env_ptr{env}; + const auto start = std::chrono::steady_clock::now(); + + // Finalizers can mutate this queue. Bound each dispatcher turn so a large + // batch cannot monopolize the runtime thread. + while (!env_ptr->pending_finalizers.empty()) + { + auto* finalizer = *env_ptr->pending_finalizers.begin(); + env_ptr->pending_finalizers.erase(finalizer); + finalizer->Finalize(); + + if (std::chrono::steady_clock::now() - start >= FinalizerDrainBudget) + { + break; + } + } + } + v8::Local GetContext(Env env) { napi_env env_ptr{env}; diff --git a/Tests/UnitTests/Android/app/src/main/cpp/CMakeLists.txt b/Tests/UnitTests/Android/app/src/main/cpp/CMakeLists.txt index 0af5caa8..0946623d 100644 --- a/Tests/UnitTests/Android/app/src/main/cpp/CMakeLists.txt +++ b/Tests/UnitTests/Android/app/src/main/cpp/CMakeLists.txt @@ -23,6 +23,9 @@ add_library(UnitTestsJNI SHARED target_compile_definitions(UnitTestsJNI PRIVATE JSRUNTIMEHOST_PLATFORM="${JSRUNTIMEHOST_PLATFORM}") target_compile_definitions(UnitTestsJNI PRIVATE ARCANA_TEST_HOOKS) +if(NAPI_JAVASCRIPT_ENGINE STREQUAL "V8") + target_compile_definitions(UnitTestsJNI PRIVATE JSRUNTIMEHOST_NAPI_ENGINE_V8) +endif() target_include_directories(UnitTestsJNI PRIVATE ${UNIT_TESTS_DIR}) diff --git a/Tests/UnitTests/CMakeLists.txt b/Tests/UnitTests/CMakeLists.txt index 2dbc7619..2de2122d 100644 --- a/Tests/UnitTests/CMakeLists.txt +++ b/Tests/UnitTests/CMakeLists.txt @@ -51,6 +51,9 @@ target_compile_definitions(UnitTests PRIVATE JSRUNTIMEHOST_PLATFORM="${JSRUNTIME if(NAPI_JAVASCRIPT_ENGINE STREQUAL "JSI") target_compile_definitions(UnitTests PRIVATE JSRUNTIMEHOST_NAPI_ENGINE_JSI) endif() +if(NAPI_JAVASCRIPT_ENGINE STREQUAL "V8") + target_compile_definitions(UnitTests PRIVATE JSRUNTIMEHOST_NAPI_ENGINE_V8) +endif() target_link_libraries(UnitTests PRIVATE AppRuntime diff --git a/Tests/UnitTests/Shared/Shared.cpp b/Tests/UnitTests/Shared/Shared.cpp index a920fa1f..34e7bf26 100644 --- a/Tests/UnitTests/Shared/Shared.cpp +++ b/Tests/UnitTests/Shared/Shared.cpp @@ -22,6 +22,10 @@ #include #include +#if defined(JSRUNTIMEHOST_NAPI_ENGINE_V8) +#include +#endif + namespace { const char* EnumToString(Babylon::Polyfills::Console::LogLevel logLevel) @@ -279,6 +283,93 @@ TEST(AppRuntime, DestroyDoesNotDeadlock) testThread.join(); } +#if defined(JSRUNTIMEHOST_NAPI_ENGINE_V8) +TEST(AppRuntime, V8FinalizersDrainAfterDispatch) +{ + constexpr size_t ExternalCount{32}; + std::atomic finalized{}; + std::promise created; + std::promise collectionRequested; + std::promise observed; + + Babylon::AppRuntime runtime{}; + runtime.Dispatch([&finalized, &created](Napi::Env env) { + for (size_t index{}; index < ExternalCount; ++index) + { + Napi::External>::New( + env, + &finalized, + [](Napi::Env, std::atomic* count) { + count->fetch_add(1, std::memory_order_relaxed); + }); + } + created.set_value(); + }); + created.get_future().wait(); + + runtime.Dispatch([&collectionRequested](Napi::Env env) { + Napi::GetContext(env)->GetIsolate()->LowMemoryNotification(); + collectionRequested.set_value(); + }); + collectionRequested.get_future().wait(); + + runtime.Dispatch([&finalized, &observed](Napi::Env) { + observed.set_value(finalized.load(std::memory_order_relaxed)); + }); + + EXPECT_EQ(observed.get_future().get(), ExternalCount); +} + +TEST(AppRuntime, V8FinalizerDrainYieldsBetweenDispatcherTurns) +{ + constexpr size_t ExternalCount{16}; + std::atomic finalized{}; + std::promise created; + std::promise collectionRequested; + + Babylon::AppRuntime runtime{}; + runtime.Dispatch([&](Napi::Env env) { + for (size_t index{}; index < ExternalCount; ++index) + { + Napi::External>::New( + env, + &finalized, + [](Napi::Env, std::atomic* count) { + std::this_thread::sleep_for(std::chrono::milliseconds{2}); + count->fetch_add(1, std::memory_order_relaxed); + }); + } + created.set_value(); + }); + created.get_future().wait(); + + runtime.Dispatch([&](Napi::Env env) { + Napi::GetContext(env)->GetIsolate()->LowMemoryNotification(); + collectionRequested.set_value(); + }); + collectionRequested.get_future().wait(); + + const auto observeFinalized = [&]() { + std::promise observed; + auto future = observed.get_future(); + runtime.Dispatch([&](Napi::Env) { + observed.set_value(finalized.load(std::memory_order_relaxed)); + }); + return future.get(); + }; + + auto observed = observeFinalized(); + EXPECT_GT(observed, 0u); + EXPECT_LT(observed, ExternalCount); + + for (size_t turn{}; turn < ExternalCount && observed < ExternalCount; ++turn) + { + observed = observeFinalized(); + } + EXPECT_EQ(observed, ExternalCount); +} +#endif + // The V8JSI Node-API shim does not implement napi_create_dataview / // napi_get_dataview_info (its DataView::New throws "TODO"), so this native test // only builds on the Chakra, V8, and JavaScriptCore backends. The size_t-width