ReviewOS

home-lang/home

Shell WorkPool graphs: quiesce recursive and parked tasks on VM shutdown

#570
Closed chris opened this 9 days ago ·
9 days ago

Scope

Complete VM shutdown ownership for Bun shell builtins whose work spans a graph of native worker-pool jobs rather than one self-contained job.

Tracked from #66 and the native WorkPool lifetime audit in #465.

Affected task graphs

  • ShellCpTask starts on the shared WorkPool, then parks while ShellAsyncCpTask runs a directory scan and may spawn many SingleTask workers. The shell variant currently bypasses the VM native-job barrier.
  • ShellLsTask recursively creates more ShellLsTask workers while walking directories.
  • ShellRmTask recursively creates DirTask workers and can enqueue multiple owner-thread verbose-output completions before its final completion.

Each graph retains shell/interpreter state and an EventLoopHandle. Waiting only for the first worker callback is insufficient: a child worker can still touch the parent after the VM is destroyed. Holding one admission across a parked graph without a graph-level terminal signal can also deadlock shutdown.

Required ownership contract

  • Admit the whole graph, or every child edge, into the creating VM's native-work barrier.
  • Reject new child work after the barrier closes without leaving parent counters permanently pending.
  • Publish every terminal owner-thread completion before releasing the last barrier admission.
  • Add cancellation for every queued completion, including ShellRmTask.DirTask verbose completions, without calling shell state or JavaScript.
  • Release copied paths, errors, output buffers, file descriptors, keep-alive refs, arenas, and task graph allocations exactly once on normal completion, error, admission rejection, and VM shutdown.
  • Preserve mini-event-loop behavior.

Regression coverage

Use the existing native WorkPool saturation probes from bun:internal-for-testing to force each graph to remain queued during Worker.terminate(). Assert teardown blocks until release, then terminates cleanly with exact cancellation/resource counters. Also run the exact pinned Bun cp, ls, and rm shell suites from /Users/chris/Code/bun at 4982b91e3702094330f3be3883354c52b8c01323.

9 days ago

The prerequisite one-shot shell shutdown checkpoint landed on main in 5848c3a58fe074612706daf7cdbbf04c68784140.

That commit establishes the shared EventLoopHandle admission/completion helpers, queued shell-task cancellation dispatch, an internal exact cancellation counter, and deterministic pool-saturation coverage for mkdir, touch, glob, conditional stat, and both mv worker phases.

This issue remains scoped to the graph-shaped cases: parked ShellCpTask → ShellAsyncCpTask/SingleTask, recursive ShellLsTask, and recursive/multi-completion ShellRmTask/DirTask. Their child admission and terminal ownership must be solved before adding them to the same barrier.

9 days ago

Checkpoint 71a25f41dce29d77cd223614b5b9aef8cacb8d9d (fix(runtime): quiesce shell copy work on shutdown) completes the cp portion of this issue.

  • admits the parent shell copy task into the VM native-work barrier before WorkPool scheduling
  • keeps that admission active across the recursive ShellAsyncCpTask graph
  • destroys the nested graph before publishing owner-loop completion and releasing the barrier
  • adds idempotent shutdown cancellation/cleanup for parent and Windows EBUSY state
  • adds a deterministic saturated-pool worker teardown regression, repeated six times
  • fixes regular-file shell capture discovered by the exact corpus (IOReader now starts non-pollable files correctly)
  • honors BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS=1 in the shell test helper so the POSIX cp corpus actually runs

Verification:

  • pinned Bun 4982b91e3702094330f3be3883354c52b8c01323 cp.test.ts: 30 passed, 0 failed, 111 assertions; Home and pinned Bun files are byte-identical (b35f33efe5ffce08a37759a212c5a70e76483e1237508e149da2bfb4b9a67429)
  • runtime regression aggregate: 53/53 passed
  • zig build test -Dfilter=home_rt -j1: 1,816 passed, 19 skipped, 0 failed
  • debug build, focused AST/Pickier, and diff checks pass

The first native-suite link attempt stopped before tests with NoSpaceLeft; after capacity recovered without clearing the shared Zig cache, the full authoritative rerun passed with the counts above.

Remaining scope here: audit and quiesce the recursive/parked ls and rm WorkPool graphs.

9 days ago

Checkpoint e0612cfd850718d27e37375af690c1cefc7f541b (fix(runtime): quiesce shell listing work on shutdown) completes the ls portion of this issue.

  • every root and recursively spawned listing node now acquires its own VM native-work admission before WorkPool scheduling
  • each node publishes its owner-loop completion before releasing admission
  • closed admission cleans the task and records shutdown cancellation
  • queued completions are explicitly canceled through Task.cancelForShutdown
  • the deterministic saturated-pool regression covers the path-expansion → listing producer chain and passed six fresh-process repetitions

Verification:

  • real-runtime aggregate: 54/54 passed
  • zig build test -Dfilter=home_rt -j1: 1,816 passed, 19 skipped, 0 failed
  • debug build, focused AST/Pickier, and diff checks pass
  • byte-identical pinned Bun ls.test.ts (ca7a0becacc193e337e839c217283702e0777e28bc630b77acccf61e4c1e7d52): 26/27 pass, with every ls assertion green

The remaining exact test times out during its nested dependency install before the final listing comparison; that separate package-manager/test-timeout boundary is tracked in #571.

Remaining scope here: rm WorkPool graph shutdown ownership.

9 days ago

Final checkpoint 1611693c74f997e437a2512dd05f45bd2a0c1300 (fix(runtime): quiesce shell removal work on shutdown) completes the remaining rm scope.

  • each root removal owns one VM native-work admission for its entire recursive DirTask graph
  • final owner-loop completion is published before releasing that admission
  • queued owner and verbose-directory callbacks cancel through the existing pending-callback refcount, preserving exactly-once graph cleanup
  • closed root admission cleans the task immediately
  • deterministic saturated-pool worker teardown passed six fresh-process repetitions

The pinned Bun rm.test.ts mirror was also updated byte for byte to 72f9085c5b7401e381ffe43f8120fce849525137d11c5f4e83c9279d0da5d98a, adding Bun's directory-to-symlink TOCTOU race. The four rm tests that do not enter the known nested install fixture pass (4 passed, 1 filtered, 45 assertions), including all ten TOCTOU iterations.

Verification:

  • real-runtime aggregate: 55/55 passed
  • current native home_rt: 1,816 passed, 19 skipped, 0 failed
  • debug build, focused AST/Pickier, and diff checks pass

The initial native launcher artifact was malformed after the host ran low on disk. A clean test-only relink from the same current sources and native object list produced the full green result above; no shared cache was cleared.

With the earlier cp and ls checkpoints, all WorkPool graphs scoped by this issue are now quiesced. Closing as complete. The shared exact-corpus nested install timeout remains tracked in #571.

Sign in to comment on this issue.