diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8ab62ef0..064b4726 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -325,3 +325,20 @@ add_fixture_test(${_napi_test_prefix}_test_65_unofficial_contextify add_fixture_test(${_napi_test_prefix}_test_allocator src/napi_allocator_test.cc ) + +if(_napi_test_engine STREQUAL "v8") + add_executable(napi_v8_foreground_checkpoint_test src/v8_foreground_checkpoint_test.cc) + target_include_directories(napi_v8_foreground_checkpoint_test PRIVATE + "${NAPI_INCLUDE_ROOT}" + "${${_napi_engine_include_var}}" + ) + target_link_libraries(napi_v8_foreground_checkpoint_test PRIVATE + ${_napi_test_engine_target} + "${${_napi_engine_library_var}}" + ) + if(NAPI_V8_DEFINES) + target_compile_definitions(napi_v8_foreground_checkpoint_test PRIVATE ${NAPI_V8_DEFINES}) + endif() + add_test(NAME napi_v8.foreground_checkpoint_fairness + COMMAND napi_v8_foreground_checkpoint_test) +endif() diff --git a/tests/src/v8_foreground_checkpoint_test.cc b/tests/src/v8_foreground_checkpoint_test.cc new file mode 100644 index 00000000..e2a56645 --- /dev/null +++ b/tests/src/v8_foreground_checkpoint_test.cc @@ -0,0 +1,66 @@ +#include "../../v8/src/edge_v8_platform.h" + +#include +#include +#include + +#include + +namespace { +class RepostingForegroundTask final : public v8::Task { + public: + RepostingForegroundTask(std::shared_ptr runner, int* ran, int remaining) + : runner_(std::move(runner)), ran_(ran), remaining_(remaining) {} + + void Run() override { + ++*ran_; + if (remaining_ > 1) { + runner_->PostTask(std::make_unique(runner_, ran_, remaining_ - 1)); + } + } + + private: + std::shared_ptr runner_; + int* ran_; + int remaining_; +}; +} // namespace + +// Own V8 initialization and disposal in a separate process so this platform +// cannot interfere with the environments of other N-API test fixtures. +int main() { + EdgeV8Platform::SetWorkerThreadCount(1); + auto platform = EdgeV8Platform::Create(true); + if (!platform) return 1; + v8::V8::InitializePlatform(platform.get()); + if (!v8::V8::Initialize()) return 1; + auto allocator = std::unique_ptr( + v8::ArrayBuffer::Allocator::NewDefaultAllocator()); + v8::Isolate::CreateParams params; + params.array_buffer_allocator = allocator.get(); + v8::Isolate* isolate = v8::Isolate::New(params); + if (!isolate) return 1; + bool passed = true; + { + v8::Isolate::Scope isolate_scope(isolate); + v8::HandleScope handle_scope(isolate); + auto runner = platform->GetForegroundTaskRunner(isolate, v8::TaskPriority::kUserVisible); + // Clear bootstrap tasks before introducing the deterministic task chain. + for (int turn = 0; turn < 32; ++turn) platform->PumpPendingForegroundTasks(isolate); + int ran = 0; + runner->PostTask(std::make_unique(runner, &ran, 4)); + for (int turn = 1; turn <= 4; ++turn) { + platform->PumpPendingForegroundTasks(isolate); + if (ran != turn) { + std::fprintf(stderr, "foreground turn %d ran %d tasks\n", turn, ran); + passed = false; + } + } + platform->PumpPendingForegroundTasks(isolate); + passed = passed && ran == 4; + } + platform->DisposeIsolate(isolate); + v8::V8::Dispose(); + v8::V8::DisposePlatform(); + return passed ? 0 : 1; +} diff --git a/v8/src/edge_v8_platform.cc b/v8/src/edge_v8_platform.cc index 3070f6a6..cee8d922 100644 --- a/v8/src/edge_v8_platform.cc +++ b/v8/src/edge_v8_platform.cc @@ -1724,8 +1724,8 @@ void EdgeV8Platform::ClearForegroundTaskTarget(v8::Isolate* isolate, napi_env en state->runner->ClearTarget(env); } -void EdgeV8Platform::PumpPendingForegroundTasks(v8::Isolate* isolate) { - if (fallback_ == nullptr || isolate == nullptr) return; +bool EdgeV8Platform::PumpPendingForegroundTasks(v8::Isolate* isolate) { + if (fallback_ == nullptr || isolate == nullptr) return false; // ForegroundTaskRunner::PostTaskCommon forwards to the guest's bound // enqueue callback when one is set, but falls back to fallback_'s own // GetForegroundTaskRunner() when no guest target is bound (e.g. the guest @@ -1733,9 +1733,10 @@ void EdgeV8Platform::PumpPendingForegroundTasks(v8::Isolate* isolate) { // calls BindForegroundTaskTarget). Nothing else pumps that fallback // runner's queue, so tasks routed there -- including V8-internal work like // Heap::PostFinalizationRegistryCleanupTaskIfNeeded's cleanup task -- would - // otherwise be posted and never run. Drain it explicitly. - while (v8::platform::PumpMessageLoop(fallback_.get(), isolate)) { - } + // otherwise be posted and never run. Admit one task per checkpoint so + // self-rescheduling GC work cannot monopolize the guest's event-loop turn. + // The caller reports progress to keep the loop runnable for the next task. + return v8::platform::PumpMessageLoop(fallback_.get(), isolate); } int EdgeV8Platform::NumberOfWorkerThreads() { diff --git a/v8/src/edge_v8_platform.h b/v8/src/edge_v8_platform.h index e9745857..9d4f80e7 100644 --- a/v8/src/edge_v8_platform.h +++ b/v8/src/edge_v8_platform.h @@ -205,7 +205,9 @@ class EdgeV8Platform final : public v8::Platform { void ClearForegroundTaskTarget(v8::Isolate* isolate, napi_env env); void AddPendingForegroundTask(const std::shared_ptr& state); void CompletePendingForegroundTask(const std::shared_ptr& state); - void PumpPendingForegroundTasks(v8::Isolate* isolate); + // Run at most one ready fallback task. Returns true when a task ran so + // the embedder can admit another loop turn before continuing provider work. + bool PumpPendingForegroundTasks(v8::Isolate* isolate); int NumberOfWorkerThreads() override; std::shared_ptr GetForegroundTaskRunner( diff --git a/v8/src/unofficial_napi.cc b/v8/src/unofficial_napi.cc index c63fa4e3..f5db8d90 100644 --- a/v8/src/unofficial_napi.cc +++ b/v8/src/unofficial_napi.cc @@ -2374,20 +2374,21 @@ napi_status NAPI_CDECL unofficial_napi_set_prepare_stack_trace_callback( // FinalizationRegistry callbacks never fire. Pump it here, at the same point // microtasks are already checkpointed, so this deferred work always gets a // chance to run regardless of whether the guest wired up its own hook. -void PumpPlatformForegroundTasks(napi_env env) { - if (env == nullptr || env->isolate == nullptr) return; +bool PumpPlatformForegroundTasks(napi_env env) { + if (env == nullptr || env->isolate == nullptr) return false; EdgeV8Platform* platform = nullptr; { std::lock_guard lock(g_runtime_mu); platform = g_runtime.platform.get(); } if (platform != nullptr) { - platform->PumpPendingForegroundTasks(env->isolate); + return platform->PumpPendingForegroundTasks(env->isolate); } + return false; } -void DrainMicrotasksForEnv(napi_env env) { - if (env == nullptr || env->isolate == nullptr) return; +bool DrainMicrotasksForEnv(napi_env env) { + if (env == nullptr || env->isolate == nullptr) return false; env->DrainFinalizerQueue(); v8::Local context = env->context(); if (!context.IsEmpty()) { @@ -2395,13 +2396,12 @@ void DrainMicrotasksForEnv(napi_env env) { if (queue != nullptr) { queue->PerformCheckpoint(env->isolate); env->DrainFinalizerQueue(); - PumpPlatformForegroundTasks(env); - return; + return PumpPlatformForegroundTasks(env); } } env->isolate->PerformMicrotaskCheckpoint(); env->DrainFinalizerQueue(); - PumpPlatformForegroundTasks(env); + return PumpPlatformForegroundTasks(env); } napi_status NAPI_CDECL unofficial_napi_event_loop_checkpoint( @@ -2415,11 +2415,17 @@ napi_status NAPI_CDECL unofficial_napi_event_loop_checkpoint( mode != unofficial_napi_event_loop_checkpoint_host_tasks) { return napi_invalid_arg; } - DrainMicrotasksForEnv(env); + const bool host_task_admitted = DrainMicrotasksForEnv(env); if (state_out != nullptr) { *state_out = NapiV8HasPendingProviderWork(env) ? unofficial_napi_event_loop_checkpoint_state_pending_provider_work : unofficial_napi_event_loop_checkpoint_state_none; + if (host_task_admitted) { + // A task may have posted its successor. Conservatively request another + // turn; an empty pump clears this signal without inspecting delayed work. + *state_out |= unofficial_napi_event_loop_checkpoint_state_host_tasks_admitted | + unofficial_napi_event_loop_checkpoint_state_pending_provider_work; + } } return napi_ok; }