ReviewOS

home-lang/home

Worker teardown: quiesce in-flight native work-pool jobs before VM destruction

#468
Closed chris opened this 8 days ago ·
8 days ago

Follow-up to #66, #461, and #465. The native task-cancellation audit found a separate lifetime family: webcrypto/PhonyWorkQueue.cpp creates EventLoopTaskNoContext, and Home ConcurrentCppTask.runFromWorkpool reads its raw creating-VM pointer, runs the native work, then calls vm.event_loop.unrefConcurrently(). The event-loop liveness reference prevents a natural drain but does not join work during explicit Worker termination. Home worker shutdown currently tears down JSC, the event loop, and its arena without waiting for these per-VM jobs. Removing the ScriptExecutionContext from the contexts map only rejects result callbacks; it does not prove the work-pool job has stopped accessing the VM.

Implement per-VM native-job lifetime accounting and a shutdown quiescence barrier with clear admission rules. Wait outside callback/contexts-map locks, prevent new jobs after shutdown admission closes, preserve normal completion and error behavior, and do not execute user callbacks in a stopped VM. Audit interactions with FFI/Node-API cleanup and worker self-exit. Add bounded termination-during-WebCrypto tests and instrumented completion/ownership checks where feasible. This is a source-audited gap, not a claimed reproduced use-after-free or passing feature. Generic queued CppTask cancellation remains #465.

8 days ago

#565 is complete in 71c8ff953aab47c918a4f79ebf87592c736c0508 and supplies the identifier-registry admission barrier plus owner-thread cancellation for already-admitted ScriptExecutionContext C++ tasks. That boundary deliberately stops short of this issue.

EventLoopTaskNoContext work-pool jobs can still hold a raw creating-VM pointer, run outside the ScriptExecutionContext contexts map, and call back into vm.event_loop after native work completes. Removing the context and draining its queues does not join those jobs. Keep #468 open for per-VM job admission/accounting and a teardown quiescence barrier; no work-pool UAF fix or passing WebCrypto termination claim is made by #565.

8 days ago

Completed in a1dcd6977 (fix(runtime): quiesce native worker jobs).

The source audit against pinned Bun 4982b91e3702094330f3be3883354c52b8c01323 confirmed that PhonyWorkQueue routes WebCrypto work through EventLoopTaskNoContext and ConcurrentCppTask. Home now gives each VM a mutex/condition-backed admission counter: accepted jobs increment before scheduling, completion unreferences the event loop before decrementing, and jobs arriving after admission closes are destroyed without running. Worker teardown and destructing main-VM teardown close admission and wait for all accepted jobs before closing the ScriptExecutionContext, cancelling result callbacks, or destroying JSC/event-loop state.

The Home-owned native-module generator now owns the mirrored EventLoopTaskNoContext.cpp, which supplies the cancellation bridge and deterministic work-pool probe used by the regression.

Validation on the rebuilt debug runtime:

  • zig build debug: passed.
  • zig build test -Dfilter=home_rt --summary all: 1,815 passed, 19 skipped, 0 failed; 22/22 build steps.
  • native-worker-workpool-shutdown.test.mjs: passed four controlled blocked-job cycles (termination could not pass the barrier before release) plus four real 64 MiB crypto.subtle.digest() termination cycles with the per-VM counter proving native admission.
  • Neighboring C++ task, inbox-shutdown, and nested-worker regressions: passed.
  • HOME_BIN="$PWD/zig-out/bin/home-debug" scripts/runtime-regression.sh: 39 passed, 0 failed.
  • Zig format checks, targeted Pickier checks, and git diff --check: passed.

This closes the EventLoopTaskNoContext/WebCrypto lifetime family. #465 remains the separate audit for other raw, non-C++, or late task producers.

Sign in to comment on this issue.