fix: close four wedge/leak paths that leave a node silently degraded - #195
fix: close four wedge/leak paths that leave a node silently degraded#195luthermonson wants to merge 7 commits into
Conversation
Four independent hardening fixes, all deferred from the adversarial reviews of #187/#189/#190/#194. One theme: a failure path that leaves the daemon looking healthy while it has quietly stopped being able to do its job. 1. buildkit: Rebuild's failure path left the server pointing at a CLOSED core. Rebuild must stop the old core before it can quarantine the data dir (the core holds open bbolt handles; on Windows the rename cannot happen at all while they are open), and GracefulStop is one-way. When newCore then failed, the data dir was restored but s.core still referenced the stopped core: every subsequent build on the node dialed a dead in-process gRPC server and failed with an opaque transport error, forever, while the node passed every health check and kept accepting jobs. Only a daemon restart cleared it. A later Server.Close then double-closed core.stop and panicked the shutdown path. Rebuild now nils s.core up front and only ever installs a core it has just verified. On a newCore failure it restores the quarantined store and makes one attempt to re-init against it (the observed cause is a transient containerd blip, and that store was serving builds a moment ago); if that also fails, s.core stays nil and every build path fails fast with ErrStoreUnavailable naming the original cause and the one instruction that works. Rebuild after Close is rejected. serverCore.close is idempotent (sync.Once) so a double close cannot panic. Also clears the half-built store before the restore rename — newCore MkdirAll's DataDir before failing, so the rename-back was onto an existing directory, which fails outright on Windows. 2. cmd/ephemerd (windows): the Linux VM sidecar start had no retry. vm.StartLinuxVM was a single shot in a background goroutine; one error and it logged "Linux jobs will not be available on this host" and gave up for the daemon's whole uptime. The failures that actually happen here are transient cold-boot ones — vmcompute/Hyper-V still coming up when the service starts after a host reboot — so a node that would have been fine 20s later silently lost all Linux capacity, kept reporting healthy, and kept taking Windows jobs until Linux jobs queued. Reuses the retryInit ladder from #189: 10 attempts x 6s (~1 min), ctx-aware and cancelled by cleanup so a shutdown mid-ladder does not stall. Retrying is safe — StartLinuxVM opens with cleanupStaleVMs and Stops itself on every post-boot failure. Give-up stays FAIL-SOFT by design: the sidecar is extra capacity, and exiting non-zero would turn a missing sidecar into a Windows-CI outage. 3. runtime: teardownContainer had its own unbounded <-exitCh. The exact bug #190 fixed in Destroy, still present in the teardown shared by startup CleanOrphans and the #187 periodic reaper. Worse here than in Destroy: CleanOrphans runs before the scheduler accepts jobs, so one leftover container with a dead shim hung daemon startup indefinitely and the node never came back. Now uses the existing waitTaskExit helper with the existing destroyKillWait bound. The reaper's ctx is already bounded (10m per sweeper pass). 4. cacheprune: `cache clear containerd --all` was silently ignored. pruneContainerd took no `all` parameter, so the flag was dropped for the containerd target: the CLI sent All=true, the daemon ran the ordinary watermark pass, correctly evicted nothing on a node whose thresholds were not tripped, and reported success with 0 records removed. An operator chasing disk on a Windows node had no override and no hint the flag was being ignored. Threads it through to a new imagegc PlanForced / Collector.CollectAll, which evicts every unprotected record regardless of pressure. It forces the POLICY, not the safety: images backing running containers and pinned runner images remain an absolute veto, exactly as in the automatic pass. A forced pass also skips the exhausted-backoff bookkeeping so one operator command cannot silence the automatic collector for 30 minutes on a filling node. Tests: buildkit Rebuild failure-path state coherence + the coreUnavailableErr state machine + close idempotence (incl. concurrent); the Linux VM ladder params, its fail-soft give-up contract and cancellation; PlanForced's protections and its precedence over the watermark policy; the --all routing through Prune -> pruneContainerd. waitTaskExit's bound is already covered by destroy_test.go.
|
The premise is false: So on Windows, every Rebuild now: nils the core → Pre-PR this path returned an error and left a build-dead-but-restartable node. Post-PR it's an unkillable daemon. Linux is reachable too (when Also confirmed (full list in the review): the VM retry ladder sits on the critical startup path ( Explicitly cleared: the new Fixing on the branch now: close the BuildKit |
…re-init
serverCore.close() did close(c.stop) + grpcServ.GracefulStop() and nothing
else. serverCore.controller was assigned in newCore and never read again —
a dead field.
control.Controller.Close (buildkit@v0.25.1 control/control.go:141) is the
only thing that closes HistoryDB (<dataDir>/history.db), the
WorkerController (each worker's Close -> MetadataStore.Close, i.e.
<dataDir>/worker/metadata_v2.db) and the CacheStore (<dataDir>/cache.db).
Nothing in ephemerd called it, so every core we ever tore down left three
exclusively-flock'd bbolt files open under DataDir for the rest of the
daemon's life.
The consequence chain, deterministic on Windows and reachable on Linux:
Rebuild sets s.core = nil, calls old.close() (handles survive), then
os.Rename(DataDir, quarantine) fails "Access is denied" -> the new
reinitAfterFailedRebuild runs WHILE HOLDING s.mu -> newCore ->
metadata.NewStore -> bolt.Open with a nil *bolt.Options.
bbolt reads a zero flock timeout as "retry forever, 50ms apart" rather than
"fail fast" (bbolt@v1.4.3 bolt_windows.go:flock — both the initial
`if timeout != 0` and the deadline check skip the give-up path at zero;
buildkit's cache/metadata/metadata.go:30 passes nil). So the re-init spun
forever under s.mu, and Client/Build/Prune/Close all block on that mutex:
a daemon that can neither build nor shut down. Pre-PR this path returned an
error and left a restartable node, so it was a strict regression.
Fixes, in order of what each is for:
1. close() now closes the controller, AFTER the gRPC server has stopped —
the same ordering buildkitd uses (cmd/buildkitd/main.go registers
`defer controller.Close()` before `server.GracefulStop()`). GracefulStop
is itself bounded at 20s and escalated to Stop(), because a Solve stream
lives as long as its build: waiting it out would hold the bbolt handles
for hours under s.mu, guaranteeing the very rename failure above. Still
idempotent via closeOnce.
2. newCore now unwinds what it has already built on every error path. The
worker controller, cache.db and history.db were all leaked if a later
step failed — and a corrupt history.db is one of the conditions that
sends the heal ladder to Rebuild in the first place, so this was live.
3. Both s.mu-holding newCore calls (Rebuild's own and the re-init) go
through buildCore, which guarantees the caller unblocks within 45s and
reaps an abandoned init if it ever completes. Losing an init attempt
costs the node its solver until a restart — the outcome we already
accept when newCore returns an error. Wedging costs the node everything.
Liveness must not be load-bearing on the correctness of teardown.
4. quarantineDir was fmt.Sprintf("%s%d", prefix, now.Unix()) — one-second
granularity, no collision handling. Two heal keys escalating together
are serialized by s.mu but land in the same second, so the second
Rebuild's rename fails onto the first quarantine and a rebuild that had
nothing wrong with it is pushed into the re-init path. Now nanoseconds
plus a bounded -N suffix loop.
5. The quarantine-path-error branch now attempts a re-init like the
rename-failure branch below it. It is the strictly safer failure (the
store on disk was never touched) and used to have the worse outcome.
Tests: serverCore.controller is held as a one-method interface so the close
contract is testable without a containerd. New tests pin that close() closes
the controller, that a real boltutil-opened file under DataDir can be
renamed away and reopened afterwards (verified to fail both ways with the
controller close removed), and that a Rebuild whose init never returns still
returns and still lets Close() complete. Every lifecycle assertion runs under
a deadline so a regression FAILS instead of hanging the suite.
…p path waitDispatch() was a bare `<-linuxVMDone` called from serve() BEFORE the webhook tunnel and before scheduler.New. On Windows the thing it waited for is a retry ladder, and the ladder's advertised budget was wrong by an order of magnitude: "10 attempts x 6s = 1 minute" counts only the SLEEPS. A single failing vm.StartLinuxVM attempt blocks on its own (linuxvm_windows.go: discoverIP 60s, waitForContainerd ~360s, dispatch wait ~270s, Stop ~11s) for roughly ten to eleven minutes, so ten attempts is closer to 1.5 hours — of a daemon with no scheduler, no webhook receiver, and no ability to run the Windows jobs it was perfectly capable of running. A Linux sidecar is extra capacity; it must never gate the host's primary capacity. Two more holes in the same path: cancelVMStart() was unreachable during the ladder (its only caller is cleanup(), and main was parked inside waitDispatch for the ladder's entire duration), and retryInit checked ctx only BETWEEN attempts while vm.StartLinuxVM takes no ctx — so a SIGTERM or SCM stop mid-ladder still bought one more multi-minute attempt, well past the SCM's 30s hard-kill. What changes: - Startup waits at most linuxDispatchStartupWait (5s) for a dispatcher, then builds the scheduler without one. That 5s is the whole worst-case contribution of the Linux-VM path to time-to-schedulable, on every platform. Linux and macOS return immediately as before. - The scheduler now tolerates a dispatcher that arrives late. It reads cfg.LinuxDispatcher through an atomic pointer with a SetLinuxDispatcher setter, so Linux capacity attaches when the VM comes up instead of being lost for the daemon's uptime. Reading the cfg field directly would have been a data race; handleLinuxJob takes the pointer once so one job's create/wait/destroy always go to the same client. - The ladder is bounded by WALL CLOCK (linuxVMStartBudget, 25m) as a ctx deadline, not by an attempt count that assumes attempts are cheap. - retryInit checks ctx before each attempt as well as between them, and a ctx give-up now carries the last real error so the log still names why the dependency never came up. - Shutdown waits linuxVMShutdownGrace (3s) for the ladder to notice the cancel and otherwise abandons it, rather than blocking daemon stop on an attempt that cannot be interrupted. The timeout branch deliberately does not touch dispatchClient/linuxVM: the start goroutine still owns them and reading them without the linuxVMDone barrier would be a race. Give-up stays fail-soft: a missing sidecar must not become a Windows-CI outage. TestLinuxVMStartLadderParams enshrined the wrong invariant — it bounded (attempts-1) x delay, which is exactly the sleeps-only figure that made the budget look like a minute. It now bounds the wall-clock budget against a worst-case attempt, and new tests pin that startup is not shaped by the ladder, that shutdown can walk away inside the SCM's window, and that an expired budget does not buy one more attempt.
…lectAll A forced pass correctly does not ARM the exhausted-backoff failsafe, but it never CLEARED one that was already armed. The backoff's premise is "everything evictable is gone and we are still over the line"; an operator running `cache clear containerd --all` evicts a SUPERSET of what the automatic pass may touch, so a forced pass that frees records or clears the watermarks has falsified that premise. Leaving the suppression in place meant --all freed the disk and the automatic collector stayed muted for up to 30 more minutes anyway, on a premise a human had just disproved. It is now cleared when the forced pass actually reclaimed something, and still never armed. Two smaller nits on the same path: - The shared "evicting image records" line logged bytes_to_free_gib from plan.BytesToFree, which PlanForced never sets. On a forced pass it always printed 0, reading as "nothing to reclaim" on exactly the pass an operator runs when they believe there is. The field is now logged only for the watermark pass, which is the only one with a byte budget. - PlanEviction was computed and then discarded whenever force was set — a full protected-filter and LRU sort of every candidate on the node, thrown away. Now an if/else. Collector.CollectAll had zero test coverage despite being the operator's disk-recovery command, because every entry point needs a live *containerd.Client. The four containerd-facing calls collect() makes are now overridable seams (nil = use the package function, so the production path is unchanged) and the pass's control flow is covered: the CollectAll -> PlanForced wiring on an unpressured disk, the pinned/live-image veto holding under force, the abort when RunningContainers fails (forcing the policy open must not force the safety open), the backoff not-armed / cleared / left-alone cases, and that a forced pass passes no stop function. cacheprune: pruneContainerd's disabled-collector error now names the pass it would have run. TestPrune_ContainerdTargetThreadsAll claimed to prove the `all` flag survives the Prune -> pruneContainerd hop — the hop where it used to be dropped silently — but a nil ImageGC returns at the disabled guard before any of that, so the test passed identically whether the flag was threaded or hard-coded. The pass name is the only externally visible evidence of what the flag became, and it is also the right thing to tell an operator whose --all did nothing.
|
Blocker fixed — three commits pushed ( The missing close. Two things the fix surfaced beyond the report:
Nothing can wedge under the lock any more. Both Startup path. The scheduler genuinely didn't tolerate a late dispatcher — Verification I ran myself, not just the suites: neutralized only the new Still worth a CI |
|
Re-review of the full four-commit branch: still needs changes. The original BLOCKER is confirmed genuinely fixed ( 1. The 20s GracefulStop escalation is not a bound — reproduced. 2. The Also being fixed: an abandoned (still-running) init can hold handles while Rebuild's Re-verified clear: scheduler |
containerd.NewWorkerOpt (worker/containerd/containerd.go:151) opens
<DataDir>/worker/metadata_v2.db with metadata.NewStore -- a bbolt file
held under an EXCLUSIVE flock -- and hands it back inside the WorkerOpt.
The very next step, base.NewWorker, can fail: cache.NewManager's cm.init
walks the metadata store and errors on a corrupt one, and
newSharableMountPool's MkdirAll on MountPoolRoot fails on a full disk.
base.NewWorker returns those errors bare (worker/base/worker.go:118) and
closes nothing, and neither did newWorkerController. worker.Controller.Add
was the same. So a failed worker init leaked the flock for the daemon's
lifetime.
newCore's own `built []io.Closer` unwind cannot cover this: it only
starts tracking once newWorkerController has RETURNED, so anything opened
and dropped inside it is invisible to it.
That leak is the precondition of the wedge the rest of this branch
removes. With a handle held under DataDir, Rebuild's os.Rename(DataDir,
quarantine) fails with "Access is denied" on Windows every time, and
every later bbolt open spins forever on a zero-timeout flock. And it
fired in the worst possible place: a full disk or a corrupt metadata
store is *when* the repair path runs, so the leak was armed by exactly
the condition that then made it unrecoverable.
The Windows variant leaked more. Its second ctd.New containerd client
(the one used to rebuild the executor with HCN network providers) was
also dropped on every error return below it, on top of the metadata
store.
Both OS variants now funnel their tail through finishWorkerController,
which owns everything on every path:
- error: closes `extra` newest-first, then workerOpt.MetadataStore, so
nothing is left open under DataDir. When wc.Add fails it closes the
*Worker* instead of the store directly, since the worker already owns
the store and the network providers -- closing both would be a double
close of the bbolt handle.
- success: ownership is unchanged. The returned worker.Controller owns
the metadata store (Controller.Close -> Worker.Close ->
MetadataStore.Close) and `extra` lives as long as the controller, as
the Windows client did before.
base.NewWorker is indirected through newWorkerFromOpt so the failure path
is testable: the real one only fails against a live containerd that is
corrupt or out of disk. The test opens a real metadata_v2.db on the real
path, forces the post-NewWorkerOpt step to fail, and then uses
os.Rename(DataDir) as the probe -- the same technique as the premise
test, and the same operation Rebuild performs next. Reverting the unwind
makes it fail with the production symptom verbatim: "Access is denied".
…ores
MAJOR: the 20s "bound" on close() was not one.
case <-time.After(coreCloseGraceTimeout):
c.grpcServ.Stop()
<-stopped // unbounded
Both GracefulStop and Stop funnel into grpc.Server.stop, whose last act
is s.handlersWG.Wait() (grpc@v1.78.0 server.go:1962) -- it waits for
every RPC HANDLER GOROUTINE to return. Stop() closes the listeners and
transports, which cancels each stream's context, but a handler that never
observes its context never returns and the WaitGroup never drains. The
in-flight GracefulStop therefore stays blocked even after Stop(), so
`<-stopped` waits forever. Reproduced directly with a ctx-ignoring
handler: GracefulStop was still blocked ten seconds after Stop().
The realistic producer is not exotic. A Windows RUN step whose containerd
shim has died parks containerdexecutor.runProcess in
`p.Wait(context.Background())` / its `defer io.Wait()` -- neither takes
the stream ctx -- so the Solve handler never returns.
close() runs with s.mu HELD (Rebuild ~:551, Server.Close ~:865) and
inside closeOnce, so one such build step took out builds, prunes, Rebuild
and shutdown for the whole daemon, and a second caller blocked behind
closeOnce as well. That is the same wedge class this branch exists to
remove, relocated into the code that removes it.
WHY NOT JUST BOUND THE `<-stopped`. Because the next statement is
controller.Close(), which closes HistoryDB, the worker's metadata store
and the CacheStore. Doing that while a Solve handler is still live is not
a leaky-but-safe tidy-up; it is a use-after-close of a bbolt DB from a
running goroutine -- a panic or a corrupted store. The handler is exactly
the thing still reading and writing those files.
So on escalation we now: log at Error naming the fact that a handler is
stuck, that the handles (cache.db, history.db, worker/metadata_v2.db) are
being INTENTIONALLY left open, what that costs (the next Rebuild's
quarantine rename will fail on Windows and the node needs an ephemerd
restart to build again) and the likely cause; record it on the core
(closeAbandoned) so Rebuild can say so up front instead of emitting three
unexplained rename errors; SKIP controller.Close(); and RETURN. A live
daemon with a leaked handle beats a wedged daemon, and it is strictly
better than a hang because a restart still fixes it.
Stop() is escalated to on its own goroutine and is never waited on. It
can block indefinitely too: grpc.Server.stop takes s.mu and the parked
GracefulStop holds s.mu for the whole of its handlersWG.Wait() once the
connections have drained, so if the client hung up while the handler
stayed wedged, Stop() deadlocks on that mutex. `stopped` closing is the
only honest evidence the server is down.
s.mu is still held across close(). Dropping it was considered and
rejected: close() is now bounded at grace+hardStop, and the nil-core
invariant that makes the failure paths coherent (s.core dropped to nil
under the SAME lock that closes it, so no reader can ever be handed a
pointer to a core being torn down) depends on the close happening inside
that critical section. Releasing and re-taking the lock around it would
open a window where Rebuild and Close interleave on the same core and
where the quarantine rename could run before the close finished -- more
new state machine than the bound is worth, for a path that is already
bounded.
Also in this commit:
* Rebuild's restore is no longer best-effort-and-hope. The old comment
claimed "everything at this path was created by the newCore call that
just failed", which is false when that call was ABANDONED
(ErrCoreInitTimeout): the init is still running and may still be
creating files under DataDir, so RemoveAll races it and the rename can
lose to its open handles. restoreQuarantinedStore now retries for 5s
(the abandoned init's handles are released as soon as adoptLateCore
closes its core, usually a second or two later), and a genuine failure
logs at Error naming BOTH paths and the hand-recovery steps -- the
store is stranded in quarantine and a restart alone will not bring it
back.
* The reaper no longer discards a late-but-successful core. It installs
it under s.mu when s.core == nil && !s.closed, and closes it otherwise.
coreInitTimeout=45s is an unmeasured guess; discarding a working solver
because the guess was 10% short left the node build-dead until a
restart for no reason. A new dataGen counter gates the adoption: if
Rebuild has renamed or cleared DataDir since the init started, the late
core is bound to a store that is no longer at that path (on Linux, to
unlinked files), so it is closed rather than installed.
* SessionManager() was dead code -- nothing in the tree called it -- and
it was the last user of current(). Both removed.
Tests, all deadline-guarded so a regression FAILS with an attributed
message instead of hanging CI:
- close(), Server.Close() and Rebuild() against a core with a real
served gRPC server holding a real in-flight handler that ignores its
stream ctx. Verified: reinstating the unbounded `<-stopped` makes all
three fail at their deadlines rather than hang.
- the normal path still closes the controller exactly once.
- late-core adoption, plus both rejections (DataDir moved, server
closed) asserting the core is CLOSED and not installed.
- the premise test now opens all three real stores -- cache.db,
history.db and worker/metadata_v2.db -- so a partial fix that
releases only some of them can no longer pass, and asserts the whole
DataDir renames and every store reopens.
Daemon shutdown ABANDONS a Linux-VM start ladder that has not finished within linuxVMShutdownGrace: it cannot touch linuxVM without racing the start goroutine that owns it, so hypervLinuxVM.Stop never runs. Stop is the only thing that deletes the VM's HCN endpoint, and it does so from l.endpointID -- a field only that goroutine may read. The same is true after a crash or an SCM kill. cleanupStaleVMs did not cover it. It enumerates HCS COMPUTE SYSTEMS and terminates the ones owned by "ephemerd"; an HCN endpoint is not a compute system and survives independently. Endpoints are persistent HNS objects, so every abandoned start leaked one permanently, each pinning an IP on the Default Switch. Fixed rather than just documented, because the leak is unbounded and invisible: cleanupStaleVMs now also sweeps endpoints by name. The VM is named "<linuxVMNamePrefix>-<8 hex>" and its endpoint is that plus "-ep", so the pair identifies exactly the resources one ephemerd created for its sidecar. The sweep runs AFTER the compute systems are terminated (so no endpoint being deleted is still attached to a live adapter) and BEFORE this daemon creates its own endpoint (so nothing matched can be ours). That is the same one-daemon-per-host assumption cleanupStaleVMs already makes when it terminates every "ephemerd"-owned compute system. The prefix and suffix are now constants shared by the creator and the sweeper so they cannot drift apart. The abandon branch's comment in cmd/ephemerd claimed the next daemon start cleans a half-started VM up. That was only half true -- the VM, yes; the endpoint, no. Corrected, and it now says which sweep covers which resource.
|
Round 3 pushed ( MAJOR 1 — the bound is now a real bound. The fix went further than the report: calling MAJOR 2 — worker init now unwinds. New shared Minors: the abandoned-init/restore race now retries for 5s and, on genuine failure, logs the quarantine path plus hand-recovery steps; the reaper adopts a late-but-successful core (gated by a new Regression proofs I re-ran myself (not taking the agent's word):
Restored and re-verified after both experiments: vet clean, buildkit/imagegc/cacheprune/runtime/dind/scheduler/cmd suites green, tree clean, no embed artifacts. Still outstanding: no |
|
Round 3 review: the round-2 wedge is genuinely fixed — verified with five throwaway probes against vendored grpc v1.78.0, including the client-gone shape that broke round 2 ( Still needs changes — three MAJORs: 1. The composition was never bounded. Every individual step has a bound; 2. The MAJOR-1 regression test uses the one wedge shape a synchronous 3. Nothing tests what Minors worth noting: Given this is the third round, I'd suggest splitting: fixes 3 and 4 (bounded |
|
Split per the owner's call. The two fixes that reviewed clean in all three rounds — This PR keeps the cluster where every finding across three rounds has landed: the buildkit Not closing it — the work is real and mostly right. But it shouldn't gate a release, and it needs a round-4 pass on the composition bound plus tests that actually protect the fix before it goes near the fleet. |
1 participant
Four independent fixes, one theme: a node that keeps reporting healthy while it has quietly lost a capability. Each traces to a specific production incident or a review finding deferred from #187/#189/#190/#194.
1.
Rebuild's failure path left the server pointing at a CLOSED core (the node-bricking one)Found reviewing #194.
Rebuildclosed the old core, and ifnewCorethen failed,s.corestill referenced the closed core — every later build dialed a stopped gRPC server and failed until a daemon restart, while the node looked healthy.Server.Close()afterwards double-closed and panicked.The old core can't be kept as a fallback (it holds bbolt handles inside
DataDir, so the quarantine rename can't proceed — impossible on Windows — andGracefulStopis one-way). So:s.coreis nil'd before the close, under the same write lock, and the failure path attempts one re-init over the restored store. Builds now get a clearErrStoreUnavailable(wrapping the original cause) orErrServerClosedinstead of dialing a corpse. Closes are idempotent (sync.Onceat both levels).Adjacent bug found and fixed on the same path:
newCoreMkdirAllsDataDirbefore the step that fails, so the restore was renaming onto an existing directory — fails outright on Windows, fails on Linux once non-empty. The restore now clears the half-built store first (everything there was created by the failed attempt; the real store is in quarantine).2. Linux VM sidecar start had no retry and failed soft forever
Found reviewing #189. A transient cold-boot Hyper-V/vmcompute hiccup made
StartLinuxVMfail once and give up permanently — the node silently loses all Linux capacity ("Linux jobs will not be available on this host") while serving Windows jobs happily. Now retried with the existingretryInithelper (10 × 6s ≈ 1 min; longer cadence than #189's networking ladder because each attempt is an expensive blocking WSL import). Give-up stays fail-soft — a missing Linux sidecar must never take down Windows CI — but logs at Error. Retry is safe:StartLinuxVMopens withcleanupStaleVMs()andStop()s on every post-boot failure. Shutdown cancels the ladder before waiting on it, so a stop landing mid-retry can't block daemon shutdown.3.
teardownContainer's own unbounded<-exitChThe startup-path cousin of the exact bug #190 fixed in
Destroy: a dead shim at boot would hangCleanOrphansand thus daemon startup. Now uses the existingwaitTaskExit(ctx, exitCh, destroyKillWait). No bare receive remains inpkg/runtime.4.
cache clear containerd --allwas silently ignoredpruneContainerdtook noallparam, so an operator had no override when the watermarks weren't tripped (the collector correctly evicts nothing, and the CLI's promise that "'clear' means clear" was false).--allnow reaches a newCollector.CollectAll→ purePlanForced. Every protection still applies: live-container image refs and pinned runner images remain an absolute veto, aRunningContainerserror still aborts the pass rather than risk evicting a live image, and BuildKit job-record name prefixes stay protected. A forced pass also skips the exhausted-backoff bookkeeping so one operator clear can't silence the automatic collector on a filling node.Testing
New: Rebuild failure-path state coherence (bogus containerd address as the
newCoreseam — asserts nil core, recorded cause,ErrStoreUnavailablefrom Client/Build/Prune, store restored, no quarantine left,Closedoesn't panic), rebuild-after-close, idempotent + concurrent closes, VM ladder params/fail-soft/cancel-unblocks-shutdown,PlanForced(LRU order, running-job veto, pinned veto, all-protected, forced-beats-watermark), and theallthreading. Fix 3 is covered by #190's existingwaitTaskExittests.go vet ./pkg/... ./cmd/ephemerd/...clean; buildkit/runtime/cacheprune/imagegc/dind/cmd suites green.-raceunavailable on the dev box (no cgo) — the concurrent-close test would benefit from a CI race run.Known, deliberately out of scope
Bare
<-exitChreceives remain inpkg/dind/cleanup.goandpkg/dind/containers.go— same class, next pass.