Skip to content

fix(runtime): interrupt spinning workers and add an awaitable Worker disposal barrier - #36955

Open
smarcd wants to merge 49 commits into
denoland:mainfrom
smarcd:fix/worker-execution-disposal
Open

smarcd wants to merge 49 commits into
denoland:mainfrom
smarcd:fix/worker-execution-disposal

Conversation

@smarcd

@smarcd smarcd commented Oct 4, 2026

Copy link
Copy Markdown

Problem

Worker.terminate() only sets a flag and wakes the worker's event loop. A
worker that is spinning in JavaScript or WebAssembly never returns to that
loop, so it keeps running, and keeps writing to any SharedArrayBuffer it
shares, indefinitely. There is also no way to wait until a worker has actually
stopped executing, which an embedder needs before reusing memory the worker
could still write.

There is no existing issue for this. A minimal repro: start a worker that
increments a shared counter in a while (true) loop (or in a Wasm loop),
call terminate(), and observe that the counter keeps increasing.

Changes

Interrupting a worker

  • terminate() now also interrupts the worker's isolate (V8
    TerminateExecution), so spinning JS and Wasm stop.
  • New Worker[Symbol.asyncDispose](): requests termination and resolves only
    once the worker's JavaScript, and the native tasks in its runtime's blocking
    pool (nonblocking FFI calls, N-API async work, blocking reads), have stopped.
    While waiting it re-requests the stop every 10ms, because V8 can discard a
    termination (see below). Plain terminate() stays a single, non-blocking
    request.
  • A host stop is recorded in a sticky isolate slot
    (ExternalExecutionTermination), so node:vm timeouts and other local
    cancellations cannot clear it.

Keeping a stop from being lost or caught

  • V8 reports and clears any exception thrown by a promise hook that runs while
    C++ creates a promise (NativeContext::RunPromiseHook), termination
    included. Module evaluation checks the sticky slot afterwards and re-raises
    the stop.
  • V8's ThrowException clears a pending termination, so an op that failed
    because its isolate was terminating turned the stop into a catchable error.
    The #[op2] error path (throw_op_error) and throw_js_error_class now
    leave a pending termination in place.
  • Lazy and synthetic module loading, require(esm) (mod_evaluate_sync) and
    dynamic import() of lazily loaded modules propagate termination instead of
    panicking on unwrap(), reporting it as an ordinary JS error, or dropping it.

Teardown with native code still running

  • A stopped worker is dropped as before, which releases its message port,
    locks, listeners and child workers promptly. Only FFI state that a running
    native call may still use (UnsafeCallback trampolines and libffi Cifs,
    and loaded libraries) is kept until the worker's blocking pool drains.
    Libraries are unloaded before callbacks are freed.
  • Cross-thread callbacks are cancelled during shutdown, while the isolate is
    still alive. A callback invoked afterwards, including from a library's unload
    destructor, returns a zero value without entering V8.
  • FFI callbacks no longer panic across the C boundary when they throw or are
    interrupted with an isize/usize/struct result, or when converting their
    return value fails. The libffi return slot is zeroed instead.
  • N-API threadsafe-function queue slots are freed, and every blocked caller is
    woken, when shutdown discards queued calls. Previously a blocking call on a
    full queue could hang worker teardown.

Tests

Each fix has a regression test that was committed failing before the fix.
Covered: core module-loading interruption cases, VM-timeout and host-stop
interaction, Worker disposal with spinning Wasm, a promise-hook-swallowed stop,
and native writes into shared memory. Also FFI callback lifetime during
disposal, prompt host-state release while a native call runs, library-unload
callbacks in both creation orders, wide and interrupted callback results, and
N-API full-queue disposal.

Local runs on macOS arm64: deno_core (494 passed), deno_ops (126), the
FFI and N-API integration suites, unit::worker_test, the Node vm unit
test, the worker startup spec, plus ./x fmt and ./x lint.

Known limitations

  • Plain terminate() sends one interrupt, so a stop that V8 discards in a
    promise hook can still leave a worker running. Symbol.asyncDispose is the
    reliable stop.
  • Disposal does not cover descendant workers, detached OS threads or threads
    owned by native libraries, and it can wait indefinitely for a blocked native
    call. Every stopped worker's thread now stays alive until such calls return.
  • Native code given a raw pointer into JS memory (N-API buffer info, FFI raw
    pointers) can still write after the isolate is dropped. This is unchanged
    from before.

AI disclosure

This PR was written with AI assistance (Anthropic Claude via Claude Code, with
OpenAI Codex used as an independent reviewer). A human directed the work, and
every change was reviewed and tested as described above.

🤖 Generated with Claude Code

smarcd and others added 30 commits October 3, 2026 20:06
V8 reports and clears any exception thrown by a promise hook that runs while
C++ creates a promise (NativeContext::RunPromiseHook, objects/contexts.cc),
termination included. A local terminate_execution() inside such a hook is
therefore lost by V8 itself. Register the sticky host stop the way WebWorker
does, and check the stop functionally after the loader returns because V8 has
already cleared the thrown termination.

Also probe the JS-created promise path; its Torque hook caller does not catch
termination, and the probe passes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
V8 reports and clears a termination thrown by a promise hook that runs while
C++ creates a promise, so the lazy-module evaluator saw neither a caught nor a
terminating state and completed a module the host had asked to stop. Consult
the sticky host request after evaluation and re-request termination.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A promise hook that runs while V8 creates a promise in C++ has its
termination reported and cleared. A Worker whose stop lands in such a hook
then keeps running, so Symbol.asyncDispose never resolves (times out at 5s).

A throwing callback whose declared result is isize, usize or a struct reaches
an unreachable!() arm in do_ffi_callback. Immediate interruption makes this
path reachable from a Worker stop as well as from a thrown error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Worker teardown dropped the JsRuntime, and with it the resource table that
owns UnsafeCallback closures and their libffi Cif, while a managed nonblocking
call could still be returning through that callback. Close callback admission
while the runtime is alive, drop the worker's Tokio runtime (which waits for
blocking tasks), and only then drop the WebWorker inside a retained Tokio
context. run_web_worker now borrows the worker so its caller owns that order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
V8 reports and clears a termination thrown by a promise hook that runs while
C++ creates a promise, so a single interrupt can be lost and the worker keeps
running. While Symbol.asyncDispose waits for the execution-stop
acknowledgment, re-request the stop every 10ms as long as it is still
requested. This re-requests interruption; it does not assume the worker has
stopped. A plain terminate() without disposal still sends a single request.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The failure path wrote a per-type zero and hit unreachable!() for isize,
usize and struct results, panicking across the C callback boundary. Zero the
whole return slot libffi supplies instead: the struct's size, or at least an
ffi_arg for scalars, which libffi expects small integers to be widened into.
The cancelled-dispatch fallback uses the same helper.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Retaining the whole WebWorker until the worker's blocking pool drains keeps
its message port, locks and other resources alive for as long as a managed
native call runs. With a 5s nonblocking sleep left running, a Web worker's
close(), a Node worker's process.exit() and terminate() of a Web worker
holding a lock are all observed only after the call returns.

Also guard the library lifetime: only the worker opens the library, so its
managed native call must return into a still-loaded library.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Retaining the WebWorker until its blocking pool drained held its message
port, locks, listeners and child workers for as long as any managed native
call ran. Drop the worker inside its future again, as before, after closing
callback admission, and carry only what such calls can still use across the
join: UnsafeCallback closures (libffi Cif and trampoline) and loaded
libraries. A callback invoked after the runtime is gone fails to dispatch and
returns zero through its retained Cif; its CallbackInfo is never freed on
resource-table teardown.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An async work's execute makes a second blocking call on a threadsafe
function whose one-slot queue the spinning worker never drains. Shutdown
discards the queued call without freeing its slot, so the execute thread
stays blocked and Symbol.asyncDispose, which joins the blocking pool, never
resolves (times out at 5s).

Drop the library-lifetime guard added in b2b6526: with libraries left out
of the keepalive it still passed, because the test library is not unmapped
by dlclose here, so it could not fail.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The queue slot was freed, and a caller blocked on a full queue woken, only
when the queued call ran on the JS thread. Runtime shutdown discards queued
calls, so a blocking caller in the worker's blocking pool waited forever and
worker disposal, which joins that pool, never completed. Tie the slot to a
guard whose drop frees it and wakes the next caller, whether the call runs or
is discarded. A woken caller's own call is then discarded too, freeing its
slot in turn.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
op_lazy_load_esm returns ExecutionTerminated after the loader rethrows the
termination, and the op error path throws a JS error for it. V8's
ThrowException clears the pending termination first, so the caller's catch
block runs and the script completes normally.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
V8's ThrowException clears a pending termination before throwing, so an op
that failed because its isolate was terminating (for example a lazy load
that rethrew the stop) handed JavaScript a catchable error and execution
continued. Skip throwing while the isolate is terminating, both in the
#[op2] error path (now deno_core::error::throw_op_error) and in
throw_js_error_class, which fast calls and the module loader use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mod_evaluate_sync, used by require(esm), reports an interrupted module as an
ordinary "Uncaught null" JS error and does not rethrow the termination.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mod_evaluate_sync turned a caught termination into an ordinary JS error and
did not rethrow it, so require(esm) of an interrupted module let execution
continue. Rethrow and report ExecutionTerminated, as the lazy evaluator does,
including a host stop swallowed by a promise hook.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
smarcd and others added 17 commits October 4, 2026 11:09
Node's terminate() resolves without waiting for the worker, so the case only
caught a spurious startup error. Count from the spinning worker into shared
memory and require the count to stop after terminate(). With the immediate
interrupt removed from WebWorkerHandle::terminate this case prints "still
running" and fails.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A worker's first import() of a lazily loaded built-in evaluates it inside
V8's dynamic-import host callback. When that evaluation is interrupted, the
loader returns ExecutionTerminated with the termination pending, but the
callback's own TryCatch drops it and the importing script completes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When the lazy fast path of import() was interrupted, it settled the import
promise through V8 calls while terminating, and the host callback's TryCatch
then swallowed the termination. Leave the promise alone on
ExecutionTerminated and rethrow from the callback, returning no promise, so
V8 propagates the stop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two async works block on a queue of two while the worker spins; disposal
must release both. This passes before and after the following change: in
the observed schedule each woken caller refills the queue and its discarded
call wakes the next one. It is coverage, not a reproduction.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Freeing a slot only woke one caller, and only on the full-to-not-full
transition. If shutdown discards several queued calls before a woken caller
runs, that caller does not refill the queue, so its own discarded call frees
a slot without waking anyone and a remaining caller blocks forever, hanging
disposal. Wake all waiters on every freed slot. The failing schedule could
not be forced locally; this is a robustness fix without a reproduction.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Worker teardown now drops loaded libraries after the JsRuntime. A library
whose unload destructor calls a stored same-thread UnsafeCallback runs it on
the worker thread, where the callback takes the same-thread path and builds
a V8 scope from the disposed isolate (a null-scope precondition panic here).
The fixture compiles a small C library, since a Rust test library is not
unmapped by dlclose on macOS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Worker teardown drops loaded libraries after the JsRuntime so managed native
calls can finish. A library's unload destructor that calls a same-thread
UnsafeCallback then took the same-thread path into the disposed isolate.
Check whether the runtime's task spawner has shut down first, and return a
zero value without entering V8 if so.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A callback created before its library is freed first, because the keepalive
drops resources in id order; the library's unload destructor then calls a
freed trampoline (SIGSEGV here). The unload test now alternates creation
order.

Interrupting a callback's return conversion (a spinning valueOf on an i32
result) makes int32_value() fail and the expect() panics across the C
callback boundary. The unchecked conversions predate this branch; immediate
interruption makes them reachable from a worker stop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The keepalive dropped resources in id order, so a callback created before
its library was freed first and the library's unload destructor could call
a freed trampoline. Hold libraries and callbacks separately and drop the
libraries first; a callback reached from an unload destructor then finds a
live trampoline that refuses to enter the disposed isolate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Converting a callback's return value can run JavaScript (valueOf), which
can throw or be interrupted by a worker stop. The numeric conversions then
returned None and expect() panicked across the C callback boundary. Use a
zero value instead, as for a callback that throws.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… calls

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When the lazy loader notices a host stop that a promise hook swallowed, it
only queues a new interrupt before returning ExecutionTerminated. Nothing is
pending yet, so the op throws an ordinary error, the caller's catch block
runs and the script completes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After noticing a host stop that a promise hook swallowed, the lazy and
synchronous module evaluators queued a new interrupt and returned
ExecutionTerminated while nothing was pending, so the op threw an ordinary,
catchable error (and the queued interrupt could fire inside the error
builder's TryCatch and be lost). Enter a trivial script so V8 throws the
termination now, rethrow it, and queue the request again so the host stop
also stops later entries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…inate()

WebWorkerHandle::terminate() now always interrupts the isolate, and an
interrupted lazy require no longer panics, so the extra interrupt for
workers holding a Web Lock and its rationale are obsolete. Also name
Deno.stdin reads among the blocking-pool work disposal waits for.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s locks

WebWorkerHandle::terminate() interrupts only on the first stop request. When
a worker whose earlier stop a promise hook swallowed is dropped with its
parent's op state, no new interrupt was sent before its Web Locks were
re-granted. Request the stop again there. Also fix a stray line break in the
disposal docs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Gate the task-spawner test that creates a JsRuntime with cfg(not(miri)), as
deno_core does for other V8-backed tests. In the host-release FFI test, use
import.meta.filename instead of a file URL pathname (wrong on Windows) and
widen its timing margins: a 10s native call against 5s/8s prompt limits.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@smarcd

smarcd commented Oct 5, 2026

Copy link
Copy Markdown
Author

This is still a draft. I pushed CI-readiness fixes in 5908f5a (a Miri gate for a V8-backed test, plus Windows-safe paths and wider timing margins in a new FFI test). The ci and pr workflows show action required: they need a maintainer to approve the workflow run for a first-time contributor before they'll execute. Locally, the PR title lint, format, lint and jsdoc_checker all pass, along with the deno_core, deno_ops, FFI, N-API and Worker suites and the deno_core doc tests.

@CLAassistant

CLAassistant commented Oct 8, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@smarcd
smarcd marked this pull request as ready for review October 9, 2026 12:04

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants