fix: drain the platform's fallback foreground-task queue - #50
Merged
Arshia001 merged 1 commit intoAug 7, 2026
Merged
Conversation
…low-up) V8 posts some of its own deferred work -- most importantly Heap::PostFinalizationRegistryCleanupTaskIfNeeded's cleanup task, queued after a GC finds a JSFinalizationRegistry with dead targets -- via v8::TaskRunner::PostNonNestableTask on the runner EdgeV8Platform hands back for the isolate. That runner forwards to the guest's own enqueue callback when one is bound (BindForegroundTaskTarget), but nothing requires the guest to bind one; a guest may drive everything through unofficial_napi_process_microtasks instead, as edgejs does. Tasks posted with no guest target bound fall back to the stock default-platform runner (EdgeV8Platform::ForegroundTaskRunner::PostTaskCommon's fallback branch), and until now nothing ever pumped that runner's queue: the tasks were posted and then silently never ran. GC still correctly collected the dead targets, but the FinalizationRegistry callbacks that were supposed to fire afterward never did -- confirmed via a standalone V8 harness (same prebuilt binary, stock platform + PumpMessageLoop: 95% finalized) against the unpatched bridge (0/120000 finalized under identical GC pressure, edgejs's own WeakRef/FinalizationRegistry-based AbortSignal cleanup leaking as a result). Add EdgeV8Platform::PumpPendingForegroundTasks(isolate), which drains the fallback platform's message loop, and call it from DrainMicrotasksForEnv alongside the existing microtask checkpoint -- the same point already pumped reliably by every guest event-loop tick, regardless of whether that guest ever wires up its own foreground-task hook. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Jul 31, 2026
syrusakbary
approved these changes
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
V8 posts some of its own deferred foreground work -- most importantly
Heap::PostFinalizationRegistryCleanupTaskIfNeeded's cleanup task, queuedafter a GC finds a
JSFinalizationRegistrywith dead targets -- viav8::TaskRunner::PostNonNestableTaskon the runnerEdgeV8Platformhandsback for the isolate. That runner forwards to the guest's own enqueue
callback when one is bound (
BindForegroundTaskTarget), but nothingrequires the guest to bind one; a guest may drive everything through
unofficial_napi_process_microtasksinstead, as edgejs does. Tasks postedwith no guest target bound fall back to the stock default-platform runner
(
ForegroundTaskRunner::PostTaskCommon's fallback branch), and until nownothing ever pumped that runner's queue: the tasks were posted and then
silently never ran.
GC still correctly collected the dead targets, but the FinalizationRegistry
callbacks that were supposed to fire afterward never did. Node's own
AbortSignal/WeakRef-based cleanup (
lib/internal/abort_controller.js)depends on this, so the practical effect was a slow, unbounded per-request
leak in every long-running V8-imports (WASIX) edgejs process.
Fix
Add
EdgeV8Platform::PumpPendingForegroundTasks(isolate), which drains thefallback platform's message loop, and call it from
DrainMicrotasksForEnvalongside the existing microtask checkpoint -- the same point already
pumped reliably by every guest event-loop tick, regardless of whether that
guest ever wires up its own foreground-task hook.
Verification
same prebuilt V8 static lib, using V8's own stock
NewDefaultPlatform()+PumpMessageLoop(no wasmer/edgejs code). ConfirmsV8 itself works fine -- 95% finalized, matching native Node -- so the bug
is in the embedding, not V8.
fr-test.mjs(register 120k objects under GC pressure, standalone):0/120000 finalized before this fix, 118000/120000 (98%) after.
after.
(0/60000) against the pre-fix bridge, passes (60000/60000) after.
Stacked on #49.