fix(ocap-kernel): finish retiring a vat that is not running - #1030
Draft
grypez wants to merge 2 commits into
Draft
fix(ocap-kernel): finish retiring a vat that is not running#1030grypez wants to merge 2 commits into
grypez wants to merge 2 commits into
Conversation
4 tasks
grypez
force-pushed
the
grypez/retire-persisted-vat
branch
from
August 25, 2026 16:57
f8ac84d to
76b5822
Compare
Contributor
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
grypez
force-pushed
the
grypez/retire-persisted-vat
branch
3 times, most recently
from
August 26, 2026 16:27
4b29d4c to
4926948
Compare
grypez
force-pushed
the
grypez/retire-persisted-vat
branch
from
August 26, 2026 19:00
4926948 to
a3e80cc
Compare
`restartVat` stops a vat and then runs it again. A relaunch that fails —
the bundle has moved since the vat was launched, the worker will not spawn
— leaves the vat gone from the running map with its record, its vat store
and its root pin all still in place.
Nothing retires a vat in that state. `terminateVat` goes through `stopVat`
→ `getVat` and throws `VatNotFoundError`, so the only way to be rid of one
is to discard the whole store, and `terminateSubcluster` — which walks
persisted membership rather than the running map — rejects part way through
on reaching one, after deleting the system-subcluster mapping and before
removing the subcluster record.
Retiring one has to do what `stopVat` does besides stopping a worker, and
none of it is visible from `terminateVat` alone:
`deleteVat` takes the vat's config, its own store, and its subcluster
membership. `markVatAsTerminated` does not: the cleanup it schedules walks
keys prefixed `${vatId}.`, which never matches `vatConfig.${vatId}`. A
record left behind restores the vat at the next boot whose code is
reachable.
The promises the vat was deciding are rejected. `cleanupTerminatedVat`
deletes their c-list entries and drops the decider's refcount on the stated
understanding that its caller has already rejected them.
The pin `launchVat` took on the vat's root is released, or the root stays
pinned and uncollectable for the life of the store.
`removeVatFromSubcluster` reports a vat that is in no subcluster, which
`deleteVat` reaches while discarding a vat — the one moment a failure
cannot be retried past.
Co-Authored-By: Claude Opus 5 <[email protected]>
`terminateVat` went through `stopVat`, so a vat that is persisted but not
running threw `VatNotFoundError` and could not be retired at all. It now
does everything `stopVat` does apart from stopping a worker.
The record goes. `markVatAsTerminated` alone does not retire a vat: the
cleanup it schedules walks keys prefixed `${vatId}.`, which never matches
`vatConfig.${vatId}`, and that record is what the next boot restores from.
The decider promises are rejected, carrying the termination reason.
`cleanupTerminatedVat` deletes their c-list entries and drops the decider's
refcount on the stated understanding that its caller rejected them first;
nothing did, so they were left unresolved with a decider that no longer
existed and no way to ever settle.
The root pin goes. `launchVat` pins a vat's root for its lifetime and
`stopVat` releases it when terminating, so a vat retired without passing
through `stopVat` left its root pinned, and so uncollectable, for the life
of the store.
`removeVatFromSubcluster` no longer reports a vat that is in no subcluster.
It is reached from `deleteVat` while a vat is being discarded — the one
moment a failure cannot be retried past — and a vat that is in no
subcluster is already in the state it asks for.
Co-Authored-By: Claude Opus 5 <[email protected]>
grypez
force-pushed
the
grypez/retire-persisted-vat
branch
from
August 26, 2026 19:37
a3e80cc to
6819b5d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second of three, stacked on #1029. Splits #1025.
The defect
restartVatstops a vat and then runs it again. A relaunch that fails — thebundle has moved since the vat was launched, the worker will not spawn — leaves
the vat gone from the running map with its record, its own store and its root
pin all still in place.
stopVat(vatId, false)deletes the handle and does nottouch persistence, and nothing puts it back.
Nothing retires a vat in that state.
terminateVatgoes throughstopVat→getVatand throwsVatNotFoundError, so the only way to be rid of one is todiscard the whole store — and
terminateSubcluster, which walks persistedmembership rather than the running map, rejects part-way through on reaching
one, after deleting the system-subcluster mapping and before removing the
subcluster record.
Approach
Retiring such a vat does everything
stopVatdoes apart from stopping a worker.None of it is visible from
terminateVatalone, which is why the state was easyto leave half-done:
markVatAsTerminatedalone does not retire a vat: thecleanup it schedules walks keys prefixed
${vatId}., which never matchesvatConfig.${vatId}. OnlydeleteVattakes that, along with the vat's ownstore and its subcluster membership. A record left behind restores the vat at
the next boot whose code is reachable.
cleanupTerminatedVatdeletes their c-list entries and drops the decider'srefcount on the stated understanding that its caller rejected them first.
Nothing did, so they were left unresolved with a decider that no longer
existed and no way to ever settle — their waiters hang for good.
launchVatpin a vat's root for itslifetime and
stopVatrelease it when terminating, so a vat retired withoutpassing through
stopVatleft its root pinned, and uncollectable, for thelife of the store.
A vat that is neither running nor persisted still throws.
removeVatFromSubclusterno longer reports a vat that is in no subcluster. Itis reached from
deleteVatwhile a vat is being discarded — the one moment afailure cannot be retried past — and a vat in no subcluster is already in the
state it asks for.
Relationship to #1023
#1023's
#retireVatdoes the same three things and additionally makes thematomic, which mine is not, and its
stopVatgains the sameisVatActivetolerance. Its version is the better one. If #1023 lands first this PR should be
dropped in favour of it; what may be worth keeping is the exemplary test below,
which drives the state through
restartVatrather than constructing it, and theremoveVatFromSubclustertolerance. Flagging rather than resolving here, sincethe two were developed in parallel.
Testing
First commit is the failing repro on its own: 6 tests. The exemplary one,
can be terminated after a failed restart, produces the state the wayproduction does — run a vat, fail its relaunch, then terminate it — rather than
asserting against a hand-built mock. The rest pin each of the three things a
retire has to do, plus the unknown-vat case as a control.
@metamask/ocap-kerneland@ocap/kernel-testgreen, withauditRefCountsonfor every kernel
kernel-testbuilds.lintandbuildclean.Checklist