diff --git a/Core/AppRuntime/Include/Babylon/AppRuntime.h b/Core/AppRuntime/Include/Babylon/AppRuntime.h index d6d024e5..48bc3552 100644 --- a/Core/AppRuntime/Include/Babylon/AppRuntime.h +++ b/Core/AppRuntime/Include/Babylon/AppRuntime.h @@ -85,6 +85,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); + Internal::DelayedTaskScheduler& GetDelayedTaskScheduler(); Options m_options; diff --git a/Core/AppRuntime/Source/AppRuntime.cpp b/Core/AppRuntime/Source/AppRuntime.cpp index 78ebe2d9..ea141d65 100644 --- a/Core/AppRuntime/Source/AppRuntime.cpp +++ b/Core/AppRuntime/Source/AppRuntime.cpp @@ -90,7 +90,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); + } } Napi::HandleScope scope{env}; diff --git a/Core/AppRuntime/Source/AppRuntime_Chakra.cpp b/Core/AppRuntime/Source/AppRuntime_Chakra.cpp index 8fe33493..8589e0f5 100644 --- a/Core/AppRuntime/Source/AppRuntime_Chakra.cpp +++ b/Core/AppRuntime/Source/AppRuntime_Chakra.cpp @@ -82,4 +82,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 f9dc3d5a..c78755f4 100644 --- a/Core/AppRuntime/Source/AppRuntime_Hermes.cpp +++ b/Core/AppRuntime/Source/AppRuntime_Hermes.cpp @@ -29,4 +29,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 a81a3900..5c83053b 100644 --- a/Core/AppRuntime/Source/AppRuntime_JSI.cpp +++ b/Core/AppRuntime/Source/AppRuntime_JSI.cpp @@ -54,4 +54,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 f2483f41..70672a6d 100644 --- a/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp +++ b/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp @@ -32,4 +32,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 10505fea..36538b68 100644 --- a/Core/AppRuntime/Source/AppRuntime_QuickJS.cpp +++ b/Core/AppRuntime/Source/AppRuntime_QuickJS.cpp @@ -68,4 +68,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 83da9550..efe3f234 100644 --- a/Core/AppRuntime/Source/AppRuntime_V8.cpp +++ b/Core/AppRuntime/Source/AppRuntime_V8.cpp @@ -126,4 +126,11 @@ namespace Babylon { // V8 auto-drains microtasks. Foreground tasks run on AppRuntime's dispatcher. } + + 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 2e31e0fb..941643e1 100644 --- a/Tests/UnitTests/Android/app/src/main/cpp/CMakeLists.txt +++ b/Tests/UnitTests/Android/app/src/main/cpp/CMakeLists.txt @@ -32,6 +32,9 @@ endif() target_compile_definitions(UnitTestsJNI PRIVATE JSRUNTIMEHOST_PLATFORM="${JSRUNTIMEHOST_PLATFORM}") target_compile_definitions(UnitTestsJNI PRIVATE NAPI_JAVASCRIPT_ENGINE="${NAPI_JAVASCRIPT_ENGINE}") 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 f3676d7d..0028a3e8 100644 --- a/Tests/UnitTests/CMakeLists.txt +++ b/Tests/UnitTests/CMakeLists.txt @@ -59,6 +59,9 @@ target_compile_definitions(UnitTests PRIVATE NAPI_JAVASCRIPT_ENGINE="${NAPI_JAVA 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 1c7e9ff7..0a226f96 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) @@ -334,6 +338,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