Skip to content

fix(ocap-kernel): keep the kernel's account of a vat's life in one piece - #1023

Open
sirtimid wants to merge 9 commits into
sirtimid/gc-delivery-hardeningfrom
sirtimid/vat-lifecycle-consistency-v2
Open

fix(ocap-kernel): keep the kernel's account of a vat's life in one piece#1023
sirtimid wants to merge 9 commits into
sirtimid/gc-delivery-hardeningfrom
sirtimid/vat-lifecycle-consistency-v2

Conversation

@sirtimid

@sirtimid sirtimid commented Aug 13, 2026

Copy link
Copy Markdown
Member

Stacked on #1022, which is stacked on #1021 and #1020. Replaces #1019, rebased onto the split stack.

Last of four. Closes #1015.

The defect

Only kernelStore.deleteVat() removes vatConfig.<vatId>. cleanupTerminatedVat sweeps ${vatId}.-prefixed keys, which never match vatConfig.<vatId>. So the two writes that together mean "this vat is dead" — deleteVat and markVatAsTerminated — both had to land, and nothing made them.

They were spread across four functions with awaits between them: VatManager.stopVat, #endVat's finally, VatHandle.terminate, and a lambda in Kernel.ts. A throw part-way left the vat marked terminated with its config alive. That reads as active again the moment cleanupTerminatedVat calls forgetTerminatedVat, at which point KernelRouter.#resolveEndpoint sees a vat the store calls live and the kernel has no handle for, rethrows, and kills the run loop. initializeAllVats then resurrects the vat on the next process start.

Two more, found alongside it:

  • A vat whose stream failed tore itself down. VatHandle.#init's drain catch called this.terminate(true, …), which did deleteVat but never markVatAsTerminated and never removed the handle from VatManager.#vats. The router went on resolving the handle successfully, the write went nowhere, and since the vat RPC client has no timeout the crank never completed — the run loop hung while getRunLoopStatus() still reported running.
  • A vat reported its dropped imports a bringOutYourDead late. makeGCAndFinalize called gc() without draining the queues first. A pending continuation still holds its closure's objects, so a sweep with work outstanding finds them reachable and the vat reports nothing on the BOYD that provoked it.

Approach

VatManager.#retireVat — the store side of a vat's death, in one synchronous step. Reject the promises it was deciding, unpin its root, deleteVat, markVatAsTerminated, with no await between them, so the half-written state cannot arise. Modelled on SwingSet's terminateVat (kernel.js:345-412) and its comment at :348 about the "synchronous prelude". Killing the worker is deliberately not part of it: that can fail, and a store that says the vat is dead is worth more than a store still waiting to find out.

#endVat and #abandonVat were both partial copies of this and are gone. Kernel.ts's lambda and launchVat lose their trailing markVatAsTerminated. VatHandle.terminate is left with only its own channel to close, and rejects its pending RPCs ahead of ending the stream rather than after, so a stream that will not close does not strand callers on a worker that is already dead.

stopVat(vatId, true, …) tolerates a missing handle. A restart needs a live handle to read its config from; an ending vat does not, and must not — the vat may be one the store still lists while the kernel has lost its handle, which is exactly what SubclusterManager.terminateSubcluster can hand it.

A fatal stream error is routed through the manager, via a new required onCriticalFailure prop on VatHandle. Only the manager can put the vat's death on record and drop the handle.

The run loop carries out a restart itself, as a queued restartVat item, so a vat is never out of the kernel's reach while cranks run.

#1015 is closed here: getImporters now counts remotes, so retiring an object queues a retireImport for a remote importer rather than deleting the object and leaving the remote's c-list entry naming nothing. #1022 supplied the exemption site and the dangling discriminant; this supplies the missing importers.

Two conflicts the rebase surfaced, resolved on the merits

Both were real disagreements between this branch and the stack beneath it, not textual noise. They are in their own commit, 363c3e103.

maybeFreeKrefs was being cleared, not restored. #1021's revertStateBeneathRollback empties the set on rollback; this branch restores the savepoint's snapshot. Restoring is correct, and #1021's own comment admits the caveat ("correct only while every rollback discards the whole delivery"): the set is not per-crank — only collectGarbage empties it — so a candidate added while the run loop was idle, which terminateVat unpinning a root produces, was owed a collection and lost it to an unrelated crank's rollback. This branch introduces exactly that path. #1021's unit test had encoded the clear() behaviour and now distinguishes a pre-savepoint kref from one the abandoned crank added.

A fix from #1022 was regressed. #1022 marks a vat terminated unconditionally after a failed launch; this branch relied on stopVat reaching #retireVat to record it. But stopVat refuses a vat the kernel has no handle for and the store does not call active — which is what a partial launch looks like. The unconditional mark is asserted again.

Testing

yarn lint clean, yarn build 31/31, changelog:validate clean. @metamask/ocap-kernel and @ocap/kernel-test fully green, with auditRefCounts on for every kernel kernel-test builds.

Note for reviewers of #1021: garbage-collection.test.ts › an object shared by two importers › survives until both importers let go was intermittently flaky on the branches beneath this one. The makeGCAndFinalize fix here is what addresses it.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, README.md, CHANGELOG.md) as appropriate

Note

High Risk
Touches core run-loop delivery, vat termination/restart, and GC rollback invariants; mistakes can deadlock cranks, resurrect vats, or kill the run loop over store/kernel disagreements.

Overview
Vat death is recorded in one synchronous step via VatManager.#retireVat (reject decider promises, unpin root, deleteVat, markVatAsTerminated) so a vat cannot stay marked terminated while vatConfig survives. Fatal stream errors route through onCriticalFailure to the manager instead of VatHandle tearing down store state; terminate only rejects pending RPCs and closes the worker channel.

Restarts run on the run loop as a restartVat queue item (enqueueRestartVat / performVatRestart), with failed relaunch terminating the vat without aborting the crank. provideVat waits out teardown flux so endpoint lookup stays aligned with the store.

KernelRouter uses async #resolveEndpoint: work to vanished endpoints is dropped only when the vat is gone for good (terminated or post-cleanup), not when a live vat is mid-restart or mid-teardown; notify, bringOutYourDead, sends, and GC delivery follow that split.

GC bookkeeping: crank savepoints snapshot maybeFreeKrefs and rollback restores the snapshot (idle candidates survive unrelated rollbacks). getImporters includes remotes (#1015). gcAndFinalize drains the event loop before sweeping so BOYD reports dropped imports on the provoking reap.

Reviewed by Cursor Bugbot for commit 93efa62. Bugbot is set up for automated code reviews on this repo. Configure here.

sirtimid and others added 7 commits August 13, 2026 17:52
…t as gone

`restartVat` keeps the vat's c-list and takes the vat out of the kernel's vat
table for as long as launching a worker and negotiating with it takes. Absence
from that table was the only signal available, so a crank landing in the window
resolved a live vat as a dead one: a message went splat, a `notify` or
`bringOutYourDead` took the run loop down, and a garbage-collection action
released the kernel's side of entries the returning incarnation still holds.

The vat's flux is now recorded rather than guarded against. `provideVat` waits on
that record, so a crank arriving mid-restart delivers to the new incarnation, and
the kernel's endpoint lookup is asynchronous to let it wait. The crank waits for
the vat, rather than the restart waiting for the run loop — which is the same
direction SwingSet takes it, where a delivery to an evicted vat awaits
`ensureVatOnline` and eviction is routine. Inverted the other way, as a lock the
restart holds while the loop stands still, whatever holds it must never await
anything the loop has to deliver, and `runVat` is exactly that kind of await.

The wait for the crank in flight stays ahead of the record, which is load-bearing:
record first and wait after, and a crank that is already running reaches its
endpoint lookup, finds the record, and waits for a restart that is waiting for
that crank to end. What the ordering leaves open is a crank the run loop starts in
the turn between the wait resolving and the record appearing — it takes the
outgoing handle and can still be mid-delivery when the worker goes down. Closing
that needs the restart to happen inside a crank, the way `processUpgradeVat` does
upstream, where the vat is idle by construction and nothing mutates kernel state
from outside the run loop.

A relaunch that fails now marks the vat terminated. It previously left a vat with
no worker that the store still counted among the living, which nothing revisits:
`cleanupTerminatedVat` only walks vats that are marked.

The GC action guard for a vat that is absent but not terminated stays, now as an
assertion rather than a live path, with its reasoning corrected: aborting the
crank does preserve the action, since `rollbackCrank` restores the cached GC set,
but nothing about the vat changes between cranks, so the action would be
re-selected and re-aborted forever with no delivery to wait on.

Also shortens this PR's CHANGELOG entries, which had grown to carry rationale
that belongs in these messages.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…for endpoints that are gone

Restarting a vat alongside a running run loop cannot be made safe by ordering
alone. The previous approach recorded the vat as mid-flux so a delivery would wait
for the new incarnation, and the record had to be installed *after* waiting out
the crank in flight — install it before, and a crank that is already running
reaches its endpoint lookup, finds the record, and waits for a restart that is
waiting for that crank to end. That ordering left a turn of its own: a crank the
run loop starts between the wait resolving and the record appearing takes the
outgoing handle, and can be mid-delivery when the worker goes down.

So the restart is now the run loop's own work, as a queued `restartVat` item, the
way SwingSet queues `upgrade-vat` for `processUpgradeVat`. In a crank of its own
there is no window to close: the run loop is the only thing that delivers, and it
is here instead, so the vat is idle by construction. `Kernel.restartVat` settles
when the crank has done it, and refuses outright if the run loop is dead, since
nothing would ever carry the request out.

Termination keeps the flux record, because it cannot be queued: `reset` and
`clearStorage` tear vats down on kernels whose run loop has died. Both of its
steps now live inside `#trackFlux`, in the order that does not deadlock, so a
caller does not sequence them and cannot get them wrong — with a test that hangs
if the order is reversed.

Two more, found in review of the previous round:

`#deliverNotify` and `#deliverBringOutYourDead` awaited the endpoint with no
handling for one that has vanished, so a crank landing during a termination took
the rejection into the run loop and killed it. This predates the wait — the lookup
used to throw synchronously in the same case — but the wait is what makes it
routine. All three of notify, reap, and GC-action delivery now go through
`#resolveEndpoint`, which drops the work for an endpoint that is gone for good
(a terminated vat, or a remote) and propagates anything else. The notify resolves
its endpoint before translating, which would otherwise mint c-list entries for an
endpoint with no way to hear about them.

A relaunch that failed marked the vat terminated but left its root pinned:
`stopVat` releases that pin only when it is the one ending the vat, and it had
been told the vat was coming back, while vat cleanup does not touch pins at all.
The pin, and the root's refcount, were held for the life of the kernel. Both
paths now release it through one helper.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The send path caught every endpoint lookup failure and treated it as a splat,
which its own TODO called out: an error that is not "this endpoint is gone"
silently discarded a deliverable message and rejected its result with
ENDPOINT_UNREACHABLE. It is now the last of the four delivery paths to go through
`resolveEndpoint`, so a splat happens where the endpoint will not be back — a
terminated vat, or a remote — and anything else propagates.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nd rollback

Four ways to kill or wedge the kernel, found reviewing this branch.

`rollbackCrank` emptied `maybeFreeKrefs` rather than restoring it. The set is
not per-crank — only `collectGarbage` empties it, at the end of a crank that had
an item — so a candidate created while the run loop was idle, as `terminateVat`
unpinning a root creates one, was owed a collection that any later crank's
rollback silently cancelled. Savepoints now carry the set as it stood when they
were taken. The audit cannot see this one: the counts stay self-consistent at 0.

A restart that could not relaunch its vat threw, and the run loop's catch rolls
back on any throw — undoing the termination records `performVatRestart` had just
written and returning the request to the run queue. Every subsequent process
start dequeued it and failed the same way. It now terminates the vat and reports
through the waiter, so the crank commits and the request is spent. The comment
claiming the throw preserved those records had the causality backwards.

Terminating a vat left a queued restart for it to be carried out against a vat
that no longer existed; `#restartVatWorker` is the one item type that does not
go through `#resolveEndpoint`, so the resulting `VatNotFoundError` propagated.
Restart-then-terminate is reachable from RPC. The waiter is now rejected when
the vat is terminated and the request dropped when the crank reaches it.

`cleanupTerminatedVat` ends by *unmarking* the vat it finished, so work
outliving it — a `bringOutYourDead` scheduled before it died, which nothing
purges from the reap queue — arrived at an endpoint that was neither present nor
terminated, which `#resolveEndpoint` reserves its throw for. It now asks whether
the store has a live record of the vat at all.

Also fixed, from the same review:

- `getImporters` counted only vats, so retiring an object deleted it without
  telling a remote importer, leaving a c-list entry naming nothing — which the
  audit reports as dangling, taking the run loop with it. Adds `getRemoteIds`.
- `#deliverGCAction` computed the live kref set before awaiting the endpoint and
  used it after. A remote re-handshaking in that window clears its c-list
  without waiting for the crank, and `krefsToErefs` throws rather than returning
  short.
- `#endVat` marks the vat terminated in a `finally`. A teardown that threw left
  it unmarked, which is the state above, and falsified `#trackFlux`'s stated
  invariant that waiters can read "gone" as terminated.
- Comments that no longer described the code: `provideVat` waiting on restarts
  (only teardown is recorded), `stopVat` tearing down "only the worker" (it
  releases the root pin, as of this branch), `clearStorage` terminating vats,
  the audit standing in for the disabled `retireExport` assert, and a stale
  `(1, 1)` baseline rationale. `#vatsInFlux` narrows to `Promise<void>`, which
  removes a branch of `provideVat` that could not be reached.

Tests: each fix has a regression test that fails against the code without it.
Closes the two coverage gaps the review named — the splat path charging the run
queue item's own target when routing went through a promise, and `ko6.refCount`
in the control-panel e2e, restored as three per-checkpoint values rather than
dropped as nondeterministic. Full unit suite, kernel-test with auditing on every
crank, and `test:e2e:ci` at 17/17.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Only `deleteVat` removes `vatConfig.<vatId>`, and `cleanupTerminatedVat`
sweeps `${vatId}.`-prefixed keys, which never match it. The writes making up
a vat's death were interleaved with awaits across `VatManager.stopVat`,
`#endVat`'s `finally`, `VatHandle.terminate` and a lambda in `Kernel.ts`, so a
throw part-way left the vat marked terminated with its config alive — which
reads as *active* again as soon as cleanup drops the mark, killing the run
loop over the disagreement and resurrecting the vat on the next process start.

`VatManager.#retireVat` now makes all four writes with no await between them,
modelled on SwingSet's synchronous prelude in `kernel.js` `terminateVat`;
worker teardown follows and is best-effort. `#endVat` and `#abandonVat` go as
duplicates of it, and `VatHandle.terminate` is left with only its own channel
to close.

A vat whose stream fails is retired by the manager, via a new
`onCriticalFailure`, rather than tearing itself down: that left the handle in
the manager and the vat live in the store, so the next delivery went to a
worker that could not answer and, the vat RPC client having no timeout, the
crank never completed while the run loop still reported itself running.

`makeGCAndFinalize` drains the queues before sweeping, since a pending
continuation still holds its closure's objects, so a vat reports its dropped
imports on the `bringOutYourDead` that provoked them rather than a later one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tial launch terminated

Two conflicts the rebase onto the GC-hardening stack surfaced, both real
disagreements rather than textual ones.

`revertStateBeneathRollback` cleared `maybeFreeKrefs` outright. The set is not
per-crank — only `collectGarbage` empties it — so a candidate added while the
run loop was idle, which `terminateVat` unpinning a root produces, was owed a
collection and lost it to an unrelated crank's rollback. It now restores the
savepoint's snapshot, which discards the abandoned crank's additions and keeps
everything that predates it. The unit test had encoded the old behaviour and is
updated to distinguish the two cases.

`launchVat`'s cleanup relied on `stopVat` reaching `#retireVat` to record the
death, but `stopVat` refuses a vat the kernel has no handle for and the store
does not call active — which is what a partial launch looks like. The mark is
asserted directly again, as it was before this branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread packages/ocap-kernel/src/vats/VatManager.ts
Recording the vat's death only saves the deliveries that come after it. The one
in flight when the worker died stays parked on an RPC client with no timeout, so
its crank never completes — the same hang `onCriticalFailure` exists to prevent,
one delivery earlier. The worker was left running too, since nothing else would
stop it once the handle was off the books.

Found by Cursor Bugbot on #1023.

Also reverts this branch's additions to the extension control-panel e2e test.
They asserted `ko6.refCount` directly, and which vat owns `ko6` is not stable:
#983 launches subcluster vats in parallel, so the root krefs fall in completion
order. The behaviour they checked is covered by the refcount audit, which runs
on every kernel `kernel-test` builds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit dfa4e5e. Configure here.

Comment thread packages/ocap-kernel/src/vats/VatManager.ts Outdated
Comment thread packages/ocap-kernel/src/vats/VatManager.ts Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 72.67%
⬆️ +0.62%
9641 / 13265
🔵 Statements 72.51%
⬆️ +0.62%
9797 / 13510
🔵 Functions 73.19%
⬆️ +0.32%
2267 / 3097
🔵 Branches 66.7%
⬆️ +0.90%
3930 / 5892
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/ocap-kernel/src/Kernel.ts 89.84%
⬆️ +0.08%
79.54%
⬆️ +0.97%
85.41%
🟰 ±0%
89.84%
⬆️ +0.08%
331-333, 404, 428, 503-513, 601, 675, 751-754, 767, 777-778, 831, 854
packages/ocap-kernel/src/KernelQueue.ts 98.02%
⬇️ -0.54%
89.47%
⬇️ -0.80%
100%
🟰 ±0%
98.02%
⬇️ -0.54%
152, 202, 582
packages/ocap-kernel/src/KernelRouter.ts 94.73%
⬆️ +0.80%
85.26%
⬆️ +6.80%
100%
🟰 ±0%
94.7%
⬆️ +0.77%
127, 190, 207, 282, 337, 397, 424, 427, 541
packages/ocap-kernel/src/KernelServiceManager.ts 98.52%
⬆️ +2.94%
92.3%
⬆️ +7.69%
100%
🟰 ±0%
98.52%
⬆️ +2.94%
311
packages/ocap-kernel/src/types.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
packages/ocap-kernel/src/garbage-collection/gc-finalize.ts 77.27%
⬆️ +2.27%
75%
🟰 ±0%
100%
🟰 ±0%
77.27%
⬆️ +2.27%
27-31, 72-76
packages/ocap-kernel/src/store/types.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
packages/ocap-kernel/src/store/methods/crank.ts 98%
⬇️ -2.00%
88.88%
⬇️ -4.87%
100%
🟰 ±0%
98%
⬇️ -2.00%
126
packages/ocap-kernel/src/store/methods/remote.ts 98.48%
⬆️ +0.05%
100%
🟰 ±0%
100%
🟰 ±0%
98.48%
⬆️ +0.05%
105-109
packages/ocap-kernel/src/store/methods/vat.ts 98.44%
⬆️ +1.15%
89.47%
⬆️ +7.66%
100%
🟰 ±0%
98.43%
⬆️ +1.16%
298-299
packages/ocap-kernel/src/vats/VatHandle.ts 90.76%
⬆️ +0.62%
86.66%
🟰 ±0%
100%
🟰 ±0%
90.76%
⬆️ +0.62%
379-384, 393-399
packages/ocap-kernel/src/vats/VatManager.ts 97.98%
⬇️ -2.02%
94.73%
⬇️ -5.27%
96.87%
⬇️ -3.13%
97.97%
⬇️ -2.03%
347-349, 379-382, 387-390
Generated in workflow #4637 for commit dfa4e5e by the Vitest Coverage Report Action

… exists

`onCriticalFailure` closed over the binding `VatHandle.make` was still to return, so a stream that broke during `#init` threw on the temporal dead zone — after the vat had been retired, and before anything rejected the pending `initVat` that nothing else will ever settle. The handle is now passed to the callback, and `runVat` refuses to put a handle on the books for a vat retired while it was being made.

The teardown also awaited the worker kill before `terminate`, which is what rejects the vat's pending RPCs. A worker slow to die — or one that never does — kept the parked delivery parked, which is the hang this path exists to clear. The two now run alongside each other, `terminate` first.

Found by Cursor Bugbot on #1023.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant