Drain deferred V8 N-API finalizers after dispatch

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;

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.

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)
+    {
+    }
 }

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)
+    {
+    }
 }

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)
+    {
+    }
 }

Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp

@@ -28,4 +28,8 @@ namespace Babylon
     {
         // JavaScriptCore drains microtasks automatically at script boundaries.
     }
+
+    void AppRuntime::DrainPostDispatchWork(Napi::Env)
+    {
+    }
 }

Core/AppRuntime/Source/AppRuntime_QuickJS.cpp

@@ -64,4 +64,8 @@ namespace Babylon
         {
         }
     }
+
+    void AppRuntime::DrainPostDispatchWork(Napi::Env)
+    {
+    }
 }

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);
     }
 }

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<v8::Context> GetContext(Napi::Env);

Core/Node-API/Source/env_v8.cc

@@ -2,8 +2,15 @@
 #include <napi/js_native_api_types.h>
 #include "js_native_api_v8.h"
 
+#include <chrono>
+
 namespace Napi
 {
+  namespace
+  {
+    constexpr auto FinalizerDrainBudget{std::chrono::milliseconds{8}};
+  }
+
   Env Attach(v8::Local<v8::Context> 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<v8::Context> GetContext(Env env)
   {
     napi_env env_ptr{env};

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})

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

Tests/UnitTests/Shared/Shared.cpp

@@ -22,6 +22,10 @@
 #include <iostream>
 #include <thread>
 
+#if defined(JSRUNTIMEHOST_NAPI_ENGINE_V8)
+#include <napi/env.h>
+#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<size_t> finalized{};
+    std::promise<void> created;
+    std::promise<void> collectionRequested;
+    std::promise<size_t> observed;
+
+    Babylon::AppRuntime runtime{};
+    runtime.Dispatch([&finalized, &created](Napi::Env env) {
+        for (size_t index{}; index < ExternalCount; ++index)
+        {
+            Napi::External<std::atomic<size_t>>::New(
+                env,
+                &finalized,
+                [](Napi::Env, std::atomic<size_t>* 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<size_t> finalized{};
+    std::promise<void> created;
+    std::promise<void> collectionRequested;
+
+    Babylon::AppRuntime runtime{};
+    runtime.Dispatch([&](Napi::Env env) {
+        for (size_t index{}; index < ExternalCount; ++index)
+        {
+            Napi::External<std::atomic<size_t>>::New(
+                env,
+                &finalized,
+                [](Napi::Env, std::atomic<size_t>* 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<size_t> 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