From 0f7f109a6fffa2f10adc14c480a35da82596725d Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Mon, 24 Aug 2026 12:33:41 -0400 Subject: [PATCH 1/6] test(ocap-kernel): reproduce a housekeeping delivery killing the run loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A vat's ownership entries outlive it. `deleteVat` takes its config and subcluster membership when it is terminated, but its c-lists and reachable flags stay in the store until `cleanupTerminatedVat` gets to it — and that runs one vat per crank, so terminating a subcluster of N vats leaves a window N cranks wide in which the kernel still addresses a vat that has no handle. `#deliverNotify`, `#deliverGCAction` and `#deliverBringOutYourDead` look their endpoint up bare. In that window the lookup throws `VatNotFoundError` from inside the crank, which escapes it and kills the run loop for good. Because the crank is rolled back the item is restored to the queue, so the next boot dequeues it and dies too. Reachable as soon as a refcount for one of the vat's exports hits zero, a promise it subscribed to resolves, or a reap queued for it comes round. The remaining cases are what a skip has to get right, and neither is obvious from the delivery site alone: Releasing the kernel's own half of a GC action does not depend on the endpoint being there to be told. Skip it and a dropped export stays flagged reachable, so the same action is derived again on the next sweep — the comment already in `#deliverGCAction` says as much. `#deliverNotify` translates the resolution and its slots before it looks the endpoint up, and both translations import if needed. Committing those mints c-list rows and takes references in an endpoint that can never release them; today the throw and its rollback are the only reason they don't survive. Co-Authored-By: Claude Opus 5 --- packages/ocap-kernel/src/KernelRouter.test.ts | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) diff --git a/packages/ocap-kernel/src/KernelRouter.test.ts b/packages/ocap-kernel/src/KernelRouter.test.ts index eddf28a6a..e2fd87de8 100644 --- a/packages/ocap-kernel/src/KernelRouter.test.ts +++ b/packages/ocap-kernel/src/KernelRouter.test.ts @@ -1,3 +1,4 @@ +import { Logger } from '@metamask/logger'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { MockInstance } from 'vitest'; @@ -808,6 +809,177 @@ describe('KernelRouter', () => { }); }); + describe('an endpoint named by persisted state that is not running', () => { + // A vat's ownership entries outlive it. `deleteVat` takes its config and + // subcluster membership at termination, but its c-lists and reachable + // flags stay until `cleanupTerminatedVat` gets to it — and that runs one + // vat per crank, so terminating a subcluster of N leaves a window N + // cranks wide in which the kernel still addresses a vat with no handle. + // + // Unlike a send, none of these deliveries has a caller to reject. + const endpointId = 'v2'; + + beforeEach(() => { + (getEndpoint as unknown as MockInstance).mockImplementation( + (requested: EndpointId) => { + if (requested === endpointId) { + throw new Error(`Vat not found: ${requested}`); + } + return endpointHandle; + }, + ); + }); + + /** + * Set up a notify whose promise is resolved and still in the endpoint's + * c-list, so delivery is reached rather than short-circuited. + * + * @returns The notify item to deliver. + */ + const makeLiveNotify = (): RunQueueItemNotify => { + const kpid = 'kp123'; + ( + kernelStore.getKernelPromise as unknown as MockInstance + ).mockReturnValue({ + state: 'fulfilled', + value: { body: JSON.stringify({ value: 'v' }), slots: [] }, + }); + (kernelStore.krefToEref as unknown as MockInstance).mockReturnValue( + 'p+123', + ); + ( + kernelStore.getKpidsToRetire as unknown as MockInstance + ).mockReturnValue([kpid]); + return { type: 'notify', endpointId, kpid }; + }; + + it.each([ + [ + 'notify', + (): RunQueueItem => makeLiveNotify(), + 'deliverNotify' as const, + ], + [ + 'dropExports', + (): RunQueueItem => ({ + type: 'dropExports' as GCRunQueueType, + endpointId, + krefs: ['ko1'], + }), + 'deliverDropExports' as const, + ], + [ + 'bringOutYourDead', + (): RunQueueItem => ({ type: 'bringOutYourDead', endpointId }), + 'deliverBringOutYourDead' as const, + ], + ])( + 'skips a %s addressed to it instead of throwing out of the crank', + async (_what, makeItem, deliverMethod) => { + // Throwing here escapes the crank and kills the run loop for good — + // and because the crank is rolled back, the same item is re-dequeued + // on the next boot and kills that one too. + const result = await kernelRouter.deliver(makeItem()); + + expect(result).toStrictEqual({ didDelivery: endpointId }); + expect( + endpointHandle[deliverMethod as keyof EndpointHandle], + ).not.toHaveBeenCalled(); + }, + ); + + it('still releases the kernel side of a skipped dropExports', async () => { + await kernelRouter.deliver({ + type: 'dropExports', + endpointId, + krefs: ['ko1'], + }); + + // Telling an endpoint to let go is also the kernel letting go, and that + // half does not depend on the endpoint being there to be told. Skip it + // and the export stays flagged reachable, so the same action is derived + // again on the next sweep, forever. + expect(kernelStore.clearReachableFlag).toHaveBeenCalledWith( + endpointId, + 'ko1', + ); + }); + + it.each(['retireExports', 'retireImports'] as const)( + 'still tears down the c-list entry of a skipped %s', + async (type) => { + await kernelRouter.deliver({ type, endpointId, krefs: ['ko1'] }); + + expect(kernelStore.deleteCListEntry).toHaveBeenCalledWith( + endpointId, + 'ko1', + 'translated-ko1', + ); + }, + ); + + it('allocates nothing in the c-list of an endpoint it is skipping', async () => { + await kernelRouter.deliver(makeLiveNotify()); + + // Both translations import if needed, minting a c-list entry and taking + // a reference on every slot. Doing that for an endpoint nobody will + // tell writes rows only that endpoint could release, and it cannot. + // While the lookup threw, the rollback undid them; once it is skipped + // the crank commits. + expect(kernelStore.translateRefKtoE).not.toHaveBeenCalled(); + expect(kernelStore.translateCapDataKtoE).not.toHaveBeenCalled(); + }); + + it('throws for an endpoint id that is neither a vat nor a remote', async () => { + (getEndpoint as unknown as MockInstance).mockImplementation( + (requested: EndpointId) => { + throw new Error(`invalid endpoint ID ${requested}`); + }, + ); + + // A missing vat and a missing remote are ordinary; an id that is + // neither is corrupt state or a kernel bug, and GC actions are parsed + // through `insistEndpointId` before they are ever queued. + await expect( + kernelRouter.deliver({ + type: 'bringOutYourDead', + endpointId: 'bogus' as EndpointId, + }), + ).rejects.toThrow('invalid endpoint ID bogus'); + }); + + it('reports the skip above the per-delivery trace level', async () => { + const logger = new Logger('test'); + const warnSpy = vi.spyOn(logger, 'warn'); + const router = new KernelRouter( + kernelStore, + kernelQueue, + getEndpoint, + vi.fn(), + logger, + ); + + await router.deliver({ type: 'bringOutYourDead', endpointId }); + + // A delivery dropped on the floor is not routine traffic, and it is the + // only trace of a vat that has quietly stopped doing anything. + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining(endpointId), + expect.anything(), + ); + }); + + it('still delivers to endpoints that are running', async () => { + const result = await kernelRouter.deliver({ + type: 'bringOutYourDead', + endpointId: 'v1', + }); + + expect(endpointHandle.deliverBringOutYourDead).toHaveBeenCalled(); + expect(result).toStrictEqual({ didDelivery: 'v1' }); + }); + }); + it('throws on unknown run queue item type', async () => { // @ts-expect-error - deliberately using an invalid type const invalidItem: RunQueueItem = { type: 'invalid' }; From c2e1c8cbded954d19ca5dd74b0d7eaf7c4134b89 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Mon, 24 Aug 2026 12:38:30 -0400 Subject: [PATCH 2/6] fix(ocap-kernel): keep an absent endpoint from killing the run loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `#deliverNotify`, `#deliverGCAction` and `#deliverBringOutYourDead` now tolerate an endpoint that is named by persisted state but not running, instead of throwing `VatNotFoundError` out of the crank and killing the run loop for good. `send` already did, rejecting its caller with `ENDPOINT_UNREACHABLE`; none of these three has a caller to reject, so an absent endpoint is something to skip. Two halves of a skip are not interchangeable, and getting either wrong undoes the point of the fix. A GC action still performs the kernel's own release — clearing the reachable flag, or tearing the c-list entry down. That half does not depend on the endpoint being there to be told, and an endpoint that cannot hear the action is precisely the case where a re-derived action would repeat on every sweep forever. `#deliverNotify` looks its endpoint up before translating the resolution rather than after. Both translations import if needed, so committing them mints c-list rows and takes references in an endpoint that can never release them. The throw and its rollback were the only reason they did not already survive; skipping without moving the lookup would have made them permanent. The lookup does not swallow an id that names neither a vat nor a remote. A missing vat and a missing remote are ordinary; that is corrupt state or a kernel bug, and GC actions pass `insistEndpointId` before they are ever queued. Skips are reported at warn rather than on the per-delivery trace channel, since a delivery dropped on the floor is not routine traffic and is the only trace of a vat that has quietly stopped doing anything. Co-Authored-By: Claude Opus 5 --- packages/ocap-kernel/CHANGELOG.md | 6 ++ packages/ocap-kernel/src/KernelRouter.ts | 71 +++++++++++++++++++++++- 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index 5d162a591..720b1cadc 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -61,6 +61,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A `notify`, GC action, or `bringOutYourDead` addressed to an endpoint that is not running is skipped instead of killing the run loop ([#1027](https://github.com/MetaMask/ocap-kernel/pull/1027)) + - These deliveries looked up their endpoint unguarded, so an endpoint named by persisted state but absent from the running kernel threw `VatNotFoundError` from inside the crank, which killed the run loop permanently. Because the crank was rolled back the item was restored to the queue, so the next boot dequeued it and died too. Reachable whenever ownership entries outlive their vat: `deleteVat` takes a terminated vat's config and subcluster membership, but its c-lists and reachable flags stay until `cleanupTerminatedVat` gets to it, one vat per crank + - Unlike a `send`, none of these has a caller to reject; `send` already tolerated a vanished endpoint by rejecting the caller with `ENDPOINT_UNREACHABLE` + - A skipped GC action still performs the kernel's own half — clearing the reachable flag, or tearing the c-list entry down — since that does not depend on the endpoint being there to be told. Skipping it would leave a dropped export flagged reachable, so the same action would be derived again on every sweep + - A skipped `notify` no longer translates the resolution first. Those translations import if needed, which would mint c-list entries and take references in an endpoint that will never be told and so can never release them + - An endpoint id that names neither a vat nor a remote still throws, since that is corrupt state rather than an endpoint that has gone away - A message delivered to a kernel-owned kref with no registered service now rejects the caller with `ENDPOINT_UNREACHABLE` instead of throwing, which escaped the crank and killed the run loop — turning one unreachable reference into a dead kernel ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Reachable without any kernel bug: an anonymous kernel object hosts something that cannot outlive the process, such as an accepted socket connection, so a vat holding one across a restart or a message to one still queued from the previous incarnation lands here. That surviving reference is exactly what stops the init sweep deleting the object, so its `kernel` owner survives with it - Matches what `KernelRouter` already does for a delivery whose endpoint has vanished. A message sent with no result promise has nobody to report to, so it is logged instead diff --git a/packages/ocap-kernel/src/KernelRouter.ts b/packages/ocap-kernel/src/KernelRouter.ts index 6bd080e7c..89049987c 100644 --- a/packages/ocap-kernel/src/KernelRouter.ts +++ b/packages/ocap-kernel/src/KernelRouter.ts @@ -21,6 +21,7 @@ import type { RunQueueItemGCAction, CrankResult, } from './types.ts'; +import { isRemoteId, isVatId } from './types.ts'; import { assert, Fail } from './utils/assert.ts'; type MessageRoute = { @@ -356,6 +357,51 @@ export class KernelRouter { return { didDelivery: 'kernel' }; } + /** + * Look up an endpoint for a housekeeping delivery — a notify, a GC action, or + * a reap — tolerating one that is not running. + * + * An endpoint can be named by persisted state without being live. A + * terminated vat's c-lists and reachable flags outlive it until + * `cleanupTerminatedVat` gets to it, one vat per crank, and the kernel goes + * on addressing it in the meantime. + * + * Unlike a `send`, these deliveries have no caller to reject: nobody is + * waiting on a `dropExport`. Throwing here instead escapes the crank and + * kills the run loop for good — and since the crank is rolled back, the same + * item is restored to the queue and kills the next boot too. An endpoint that + * isn't there is something to skip, not a kernel fault. + * + * @param endpointId - The endpoint the item is addressed to. + * @param what - What was being delivered, for the log. + * @returns The endpoint's handle, or undefined if it is not running. + */ + #getEndpointIfRunning( + endpointId: EndpointId, + what: string, + ): EndpointHandle | undefined { + // An id that names neither a vat nor a remote is not an endpoint that has + // gone away; it is corrupt state or a kernel bug, and GC actions pass + // `insistEndpointId` before they are ever queued. Let it through. + if (!isVatId(endpointId) && !isRemoteId(endpointId)) { + return this.#getEndpoint(endpointId); + } + try { + return this.#getEndpoint(endpointId); + } catch (error) { + // Deliberately broad over the rest: `#getEndpoint` reports a missing vat + // and a missing remote as different error types, and neither is worth + // killing the kernel over. Reported above the per-delivery trace channel, + // since a delivery dropped on the floor is not routine traffic — it is + // the only trace of a vat that has quietly stopped doing anything. + this.#logger?.warn( + `Skipped ${what} for endpoint ${endpointId}, which is not running:`, + error, + ); + return undefined; + } + } + /** * Deliver a 'notify' run queue item. * @@ -390,6 +436,15 @@ export class KernelRouter { // no kpids to retire, already done return { didDelivery: endpointId }; } + // Looked up before the translations below, not after: those import if + // needed, minting c-list entries and taking a reference on every slot. + // Doing that for an endpoint that will never be told writes rows only that + // endpoint could release, and it cannot. While an absent endpoint threw, + // the rollback undid them; once it is skipped, the crank commits. + const endpoint = this.#getEndpointIfRunning(endpointId, `notify ${kpid}`); + if (!endpoint) { + return { didDelivery: endpointId }; + } const resolutions: VatOneResolution[] = []; for (const toResolve of targets) { const tPromise = this.#kernelStore.getKernelPromise(toResolve); @@ -409,7 +464,6 @@ export class KernelRouter { // promise in the batch here, since the endpoint can never refer to a // settled promise by that eref again. Left alone for now because the // debug UI discovers exported ocap URLs by scanning these entries. - const endpoint = this.#getEndpoint(endpointId); return await endpoint.deliverNotify(resolutions); } @@ -424,11 +478,16 @@ export class KernelRouter { this.#logger?.log( `@@@@ deliver ${endpointId} ${type} ${JSON.stringify(krefs)}`, ); - const endpoint = this.#getEndpoint(endpointId); + const endpoint = this.#getEndpointIfRunning(endpointId, type); const erefs = this.#kernelStore.krefsToErefs(endpointId, krefs); // Telling an endpoint to let go is also the kernel letting go. Otherwise a // dropped export stays flagged reachable, so the same action gets derived // again, and retired entries outlive the objects they name. + // + // The kernel's half does not depend on the endpoint being there to be told, + // so it runs even when the delivery is skipped — an endpoint that cannot + // hear the action is the case where a re-derived action would repeat + // forever. krefs.forEach((kref, index) => { if (type === 'dropExports') { this.#kernelStore.clearReachableFlag(endpointId, kref); @@ -440,6 +499,9 @@ export class KernelRouter { ); } }); + if (!endpoint) { + return { didDelivery: endpointId }; + } const method = `deliver${(type[0] as string).toUpperCase()}${type.slice(1)}` as | 'deliverDropExports' @@ -460,7 +522,10 @@ export class KernelRouter { ): Promise { const { endpointId } = item; this.#logger?.log(`@@@@ deliver ${endpointId} bringOutYourDead`); - const endpoint = this.#getEndpoint(endpointId); + const endpoint = this.#getEndpointIfRunning(endpointId, 'bringOutYourDead'); + if (!endpoint) { + return { didDelivery: endpointId }; + } const crankResult = await endpoint.deliverBringOutYourDead(); return crankResult; } From 660cacf1723f4dec2a1a8828e3e86b54234553e8 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:51:29 -0400 Subject: [PATCH 3/6] docs(ocap-kernel): use the real PR number in the changelog --- packages/ocap-kernel/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index 720b1cadc..afe6b6829 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -61,7 +61,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- A `notify`, GC action, or `bringOutYourDead` addressed to an endpoint that is not running is skipped instead of killing the run loop ([#1027](https://github.com/MetaMask/ocap-kernel/pull/1027)) +- A `notify`, GC action, or `bringOutYourDead` addressed to an endpoint that is not running is skipped instead of killing the run loop ([#1029](https://github.com/MetaMask/ocap-kernel/pull/1029)) - These deliveries looked up their endpoint unguarded, so an endpoint named by persisted state but absent from the running kernel threw `VatNotFoundError` from inside the crank, which killed the run loop permanently. Because the crank was rolled back the item was restored to the queue, so the next boot dequeued it and died too. Reachable whenever ownership entries outlive their vat: `deleteVat` takes a terminated vat's config and subcluster membership, but its c-lists and reachable flags stay until `cleanupTerminatedVat` gets to it, one vat per crank - Unlike a `send`, none of these has a caller to reject; `send` already tolerated a vanished endpoint by rejecting the caller with `ENDPOINT_UNREACHABLE` - A skipped GC action still performs the kernel's own half — clearing the reachable flag, or tearing the c-list entry down — since that does not depend on the endpoint being there to be told. Skipping it would leave a dropped export flagged reachable, so the same action would be derived again on every sweep From 6b381904502e0dd23f4b004f25108d05217d180b Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:21:12 -0400 Subject: [PATCH 4/6] test(ocap-kernel): pin the reap that outlives its vat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reachability this guard rests on, which the changelog and the JSDoc had attributed to the wrong caller. `terminateSubcluster` does not leave a terminated vat addressable: it calls `collectGarbage` after each `terminateVat`, and that drains every pending cleanup in a loop, so the c-lists are gone before the run loop sees another crank — and `shouldProcessAction` then filters out any GC action naming that vat. The reap queue is not filtered that way, and nothing purges it when a vat dies. `nextReapAction` shifts an endpoint off and hands back a `bringOutYourDead` with no liveness check at all, so a reap scheduled by `reapVats` before a vat was terminated arrives after it — through `terminateSubcluster` as readily as any other route. Reaps are taken ahead of the run queue, so it blocks everything behind it too. The window does exist for the callers that mark a vat terminated without draining — the boot-time orphan prune, and the run loop's own termination callback — which is what the corrected wording now says. Purging the queue on termination is worth doing on its own, and is tracked separately; the delivery-side guard is needed either way. Co-Authored-By: Claude Opus 5 --- packages/ocap-kernel/CHANGELOG.md | 4 ++- packages/ocap-kernel/src/KernelRouter.ts | 11 +++--- .../ocap-kernel/src/store/methods/gc.test.ts | 36 +++++++++++++++++++ 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index afe6b6829..ce9c295be 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -62,7 +62,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - A `notify`, GC action, or `bringOutYourDead` addressed to an endpoint that is not running is skipped instead of killing the run loop ([#1029](https://github.com/MetaMask/ocap-kernel/pull/1029)) - - These deliveries looked up their endpoint unguarded, so an endpoint named by persisted state but absent from the running kernel threw `VatNotFoundError` from inside the crank, which killed the run loop permanently. Because the crank was rolled back the item was restored to the queue, so the next boot dequeued it and died too. Reachable whenever ownership entries outlive their vat: `deleteVat` takes a terminated vat's config and subcluster membership, but its c-lists and reachable flags stay until `cleanupTerminatedVat` gets to it, one vat per crank + - These deliveries looked up their endpoint unguarded, so an endpoint named by persisted state but absent from the running kernel threw `VatNotFoundError` from inside the crank, which killed the run loop permanently. Because the crank was rolled back the item was restored to the queue, so the next boot dequeued it and died too + - Reachable from `reapVats` alone: nothing purges the reap queue when a vat dies, and unlike a GC action — which `shouldProcessAction` filters on whether the endpoint still has a c-list entry — a reap is handed back with no liveness check at all. Schedule one, terminate that vat, and the run loop dequeues a `bringOutYourDead` addressed to a vat that no longer exists. Reaps are taken ahead of the run queue, so it also blocks everything behind it + - Also reachable while a terminated vat awaits cleanup, which happens one vat per crank, since `deleteVat` takes its config and subcluster membership but leaves its c-lists and reachable flags in place. Not via `terminateSubcluster`, which drains every pending cleanup after each vat it terminates - Unlike a `send`, none of these has a caller to reject; `send` already tolerated a vanished endpoint by rejecting the caller with `ENDPOINT_UNREACHABLE` - A skipped GC action still performs the kernel's own half — clearing the reachable flag, or tearing the c-list entry down — since that does not depend on the endpoint being there to be told. Skipping it would leave a dropped export flagged reachable, so the same action would be derived again on every sweep - A skipped `notify` no longer translates the resolution first. Those translations import if needed, which would mint c-list entries and take references in an endpoint that will never be told and so can never release them diff --git a/packages/ocap-kernel/src/KernelRouter.ts b/packages/ocap-kernel/src/KernelRouter.ts index 89049987c..f734c2a65 100644 --- a/packages/ocap-kernel/src/KernelRouter.ts +++ b/packages/ocap-kernel/src/KernelRouter.ts @@ -361,10 +361,13 @@ export class KernelRouter { * Look up an endpoint for a housekeeping delivery — a notify, a GC action, or * a reap — tolerating one that is not running. * - * An endpoint can be named by persisted state without being live. A - * terminated vat's c-lists and reachable flags outlive it until - * `cleanupTerminatedVat` gets to it, one vat per crank, and the kernel goes - * on addressing it in the meantime. + * An endpoint can be named by persisted state without being live. Nothing + * purges the reap queue when a vat dies, and unlike a GC action — which + * `shouldProcessAction` filters on whether the endpoint still has a c-list + * entry — a reap is handed back with no liveness check at all, so a reap + * scheduled before a vat was terminated arrives after it. A terminated vat's + * c-lists also outlive it until `cleanupTerminatedVat` gets to it, one vat + * per crank, and the kernel goes on addressing it in the meantime. * * Unlike a `send`, these deliveries have no caller to reject: nobody is * waiting on a `dropExport`. Throwing here instead escapes the crank and diff --git a/packages/ocap-kernel/src/store/methods/gc.test.ts b/packages/ocap-kernel/src/store/methods/gc.test.ts index e91fb29bb..d18ec317b 100644 --- a/packages/ocap-kernel/src/store/methods/gc.test.ts +++ b/packages/ocap-kernel/src/store/methods/gc.test.ts @@ -165,6 +165,42 @@ describe('GC methods', () => { expect(kernelStore.nextReapAction()).toBeUndefined(); }); + + it('yields a reap scheduled for a vat that has since been terminated', () => { + // Why a housekeeping delivery has to tolerate an endpoint that is not + // running. Nothing purges the reap queue when a vat dies — not + // `terminateVat`, not `deleteVat`, not `cleanupTerminatedVat` — and unlike + // a GC action, which `shouldProcessAction` filters on `hasCListEntry`, + // a reap is handed back with no liveness check at all. So the run loop + // dequeues a `bringOutYourDead` addressed to a vat that no longer exists + // anywhere else in the store, and looks its endpoint up. + // + // Purging the queue instead is worth doing on its own; this only pins the + // reachability that makes the delivery-side guard necessary either way. + kernelStore.setVatConfig('v1', { bundleSpec: 'file:///gone.bundle' }); + kernelStore.initEndpoint('v1'); + const subclusterId = kernelStore.addSubcluster({ + bootstrap: 'a', + vats: { a: 'v1' }, + }); + kernelStore.addSubclusterVat(subclusterId, 'a', 'v1'); + kernelStore.scheduleReap('v1'); + + // Exactly what `terminateSubcluster` does: terminate the vat, then drain + // every pending cleanup, which is why that path leaves no c-list behind. + kernelStore.deleteVat('v1'); + kernelStore.markVatAsTerminated('v1'); + while (kernelStore.nextTerminatedVatCleanup()) { + // drain + } + kernelStore.collectGarbage(); + + expect(kernelStore.isVatActive('v1')).toBe(false); + expect(kernelStore.nextReapAction()).toStrictEqual({ + type: 'bringOutYourDead', + endpointId: 'v1', + }); + }); }); describe('retireKernelObjects', () => { From 18126874b9ba9bf0873ba75f63d51b6eeb6305a4 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:22:06 -0400 Subject: [PATCH 5/6] fix(ocap-kernel): tolerate a c-list cleaned out from under a GC action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported by Bugbot on #1029. Guarding only the endpoint lookup left the same crash one line further on. `processGCActionSet` selects an action while the endpoint still has a c-list entry for each of its krefs, but the run loop calls `nextTerminatedVatCleanup` between that selection and the delivery, and cleaning a vat takes its whole c-list. An action selected in that crank therefore arrives after its entries are gone, and `krefsToErefs` reports an unmapped kref by throwing — out of the crank, killing the run loop exactly as the unguarded lookup did, and re-dequeued on the next boot to kill that one too. Nothing is owed in that case: the cleanup performed the kernel's half on its way past. So the krefs are filtered to those the endpoint still holds, and an action left with none is skipped whole. The filter applies only when the endpoint is gone. Cleanup runs for a terminated vat and a terminated vat has no handle, so a running endpoint cannot be in this state; one that is missing a c-list entry is a real disagreement and still throws. Co-Authored-By: Claude Opus 5 --- packages/ocap-kernel/CHANGELOG.md | 1 + packages/ocap-kernel/src/KernelRouter.test.ts | 31 +++++++++++++++++++ packages/ocap-kernel/src/KernelRouter.ts | 24 ++++++++++++-- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index ce9c295be..59412eb8f 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -67,6 +67,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Also reachable while a terminated vat awaits cleanup, which happens one vat per crank, since `deleteVat` takes its config and subcluster membership but leaves its c-lists and reachable flags in place. Not via `terminateSubcluster`, which drains every pending cleanup after each vat it terminates - Unlike a `send`, none of these has a caller to reject; `send` already tolerated a vanished endpoint by rejecting the caller with `ENDPOINT_UNREACHABLE` - A skipped GC action still performs the kernel's own half — clearing the reachable flag, or tearing the c-list entry down — since that does not depend on the endpoint being there to be told. Skipping it would leave a dropped export flagged reachable, so the same action would be derived again on every sweep + - Unless the c-list entries have gone in the meantime: an action is selected only while they exist, but the run loop cleans one terminated vat between that selection and the delivery, and cleaning a vat takes its whole c-list. The cleanup has done the kernel's half in that case, and translating the krefs anyway would report an unmapped kref by throwing — out of the crank, killing the run loop exactly as the unguarded lookup did - A skipped `notify` no longer translates the resolution first. Those translations import if needed, which would mint c-list entries and take references in an endpoint that will never be told and so can never release them - An endpoint id that names neither a vat nor a remote still throws, since that is corrupt state rather than an endpoint that has gone away - A message delivered to a kernel-owned kref with no registered service now rejects the caller with `ENDPOINT_UNREACHABLE` instead of throwing, which escaped the crank and killed the run loop — turning one unreachable reference into a dead kernel ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) diff --git a/packages/ocap-kernel/src/KernelRouter.test.ts b/packages/ocap-kernel/src/KernelRouter.test.ts index e2fd87de8..e4b46ac7d 100644 --- a/packages/ocap-kernel/src/KernelRouter.test.ts +++ b/packages/ocap-kernel/src/KernelRouter.test.ts @@ -64,6 +64,7 @@ describe('KernelRouter', () => { krefsToErefs: vi.fn((_endpointId: string, krefs: string[]) => krefs.map((kref: string) => `translated-${kref}`), ) as unknown as MockInstance, + hasCListEntry: vi.fn().mockReturnValue(true), clearReachableFlag: vi.fn(), deleteCListEntry: vi.fn(), forgetKref: vi.fn(), @@ -918,6 +919,36 @@ describe('KernelRouter', () => { }, ); + it('skips a GC action whose c-list entries went in the same crank', async () => { + // `processGCActionSet` selects an action only while the endpoint still + // has a c-list entry for its krefs, but the run loop then calls + // `nextTerminatedVatCleanup` before delivering it — and that takes the + // whole c-list of the vat it cleans. So the entries can be gone by the + // time this runs, and `krefsToErefs` reports an unmapped kref by + // throwing, which would leave the crank and kill the run loop just as + // the unguarded lookup used to. + (kernelStore.hasCListEntry as unknown as MockInstance).mockReturnValue( + false, + ); + ( + kernelStore.krefsToErefs as unknown as MockInstance + ).mockImplementation(() => { + throw new Error(`unmapped kref ko1 in ${endpointId} c-list`); + }); + + const result = await kernelRouter.deliver({ + type: 'dropExports', + endpointId, + krefs: ['ko1'], + }); + + expect(result).toStrictEqual({ didDelivery: endpointId }); + // The cleanup performed the kernel's half already; there is nothing + // left for this delivery to release. + expect(kernelStore.clearReachableFlag).not.toHaveBeenCalled(); + expect(kernelStore.deleteCListEntry).not.toHaveBeenCalled(); + }); + it('allocates nothing in the c-list of an endpoint it is skipping', async () => { await kernelRouter.deliver(makeLiveNotify()); diff --git a/packages/ocap-kernel/src/KernelRouter.ts b/packages/ocap-kernel/src/KernelRouter.ts index f734c2a65..8e32a95f5 100644 --- a/packages/ocap-kernel/src/KernelRouter.ts +++ b/packages/ocap-kernel/src/KernelRouter.ts @@ -482,7 +482,27 @@ export class KernelRouter { `@@@@ deliver ${endpointId} ${type} ${JSON.stringify(krefs)}`, ); const endpoint = this.#getEndpointIfRunning(endpointId, type); - const erefs = this.#kernelStore.krefsToErefs(endpointId, krefs); + // `processGCActionSet` selects an action only while the endpoint still has + // a c-list entry for each of its krefs, but the run loop calls + // `nextTerminatedVatCleanup` between that selection and this delivery, and + // cleaning a vat takes its whole c-list. So an action can arrive after its + // entries are gone — and `krefsToErefs` reports an unmapped kref by + // throwing, which would leave the crank and kill the run loop just as the + // unguarded lookup did. The cleanup has already done the kernel's half in + // that case, so there is nothing left to do. + // + // Only an endpoint that is gone can be in this state: cleanup runs for a + // terminated vat, and a terminated vat has no handle. A running endpoint + // missing a c-list entry is a real disagreement, and still throws. + const toRelease = endpoint + ? krefs + : krefs.filter((kref) => + this.#kernelStore.hasCListEntry(endpointId, kref), + ); + if (toRelease.length === 0) { + return { didDelivery: endpointId }; + } + const erefs = this.#kernelStore.krefsToErefs(endpointId, toRelease); // Telling an endpoint to let go is also the kernel letting go. Otherwise a // dropped export stays flagged reachable, so the same action gets derived // again, and retired entries outlive the objects they name. @@ -491,7 +511,7 @@ export class KernelRouter { // so it runs even when the delivery is skipped — an endpoint that cannot // hear the action is the case where a re-derived action would repeat // forever. - krefs.forEach((kref, index) => { + toRelease.forEach((kref, index) => { if (type === 'dropExports') { this.#kernelStore.clearReachableFlag(endpointId, kref); } else { From 5d33bc37c8b5c2054b88bfe38fd0fb011e289d4f Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:55:35 -0400 Subject: [PATCH 6/6] test(kernel-test): brick a kernel with one message from a peer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bringOutYourDead` is an ordinary arm of the remote protocol; any peer can send one, unsolicited, and it needs no authority beyond being connected. The kernel answers by scheduling a reap against the remote it came from, in the persisted reap queue. `scheduleReap` does not wake a parked run loop, so an idle kernel holds that reap indefinitely, and carries it into its next incarnation. That incarnation starts its run loop inside `Kernel.make` — before an embedder can call `initRemoteComms`, which is what restores remote handles — and reaps are taken ahead of the run queue, so the first thing the loop does is deliver one addressed to a remote that does not exist yet. So one message from a peer is enough to stop a kernel ever booting again, assuming only that it restarts at some point. On main: expected [ { state: 'failed', ... }, ... ] to strictly equal [ { state: 'running' }, ... ] Error: Remote not found: r1 at RemoteManager.getRemote → #getEndpoint → #deliverBringOutYourDead → deliver → #runLoop → KernelQueue.run at #init (Kernel.ts) → Kernel.make Both boots are asserted together so the failure reports both, because the point is that the second is no better than the first: the crank that dies is rolled back, which puts the reap back on the queue. Two things the test pins rather than assumes. The peer is given local work so its own loop cranks and sends the request, and nothing touches the victim afterwards — a victim that cranks eats its own reap while the remote still exists, and the scenario evaporates. And the reap queue is read from the database after shutdown, so a run that loses that race fails as a setup failure rather than passing. Only the victim needs a file-backed database, since only it restarts. Co-Authored-By: Claude Opus 5 --- packages/kernel-test/src/remote-comms.test.ts | 91 ++++++++++++++++++- packages/ocap-kernel/CHANGELOG.md | 3 +- packages/ocap-kernel/src/KernelRouter.ts | 16 ++-- 3 files changed, 101 insertions(+), 9 deletions(-) diff --git a/packages/kernel-test/src/remote-comms.test.ts b/packages/kernel-test/src/remote-comms.test.ts index 00b69bfaa..5eeb22cec 100644 --- a/packages/kernel-test/src/remote-comms.test.ts +++ b/packages/kernel-test/src/remote-comms.test.ts @@ -3,7 +3,7 @@ import { peerIdFromPrivateKey } from '@libp2p/peer-id'; import { NodejsPlatformServices } from '@metamask/kernel-node-runtime'; import type { KernelDatabase } from '@metamask/kernel-store'; import { makeSQLKernelDatabase } from '@metamask/kernel-store/sqlite/nodejs'; -import { fromHex } from '@metamask/kernel-utils'; +import { fromHex, waitUntilQuiescent } from '@metamask/kernel-utils'; import { makeKernelStore, kunser, Kernel } from '@metamask/ocap-kernel'; import type { KernelStore, @@ -494,6 +494,95 @@ describe('Remote Communications (Integration Tests)', () => { await rm(tempDir, { recursive: true, force: true }); } }); + + it('is not bricked by a peer asking it to bring out its dead', async () => { + // `bringOutYourDead` is an ordinary arm of the remote protocol: any peer can + // send one, unsolicited. The kernel answers by scheduling a reap against the + // remote it came from, in the persisted reap queue. + // + // `scheduleReap` does not wake a parked run loop, so an idle kernel holds + // that reap indefinitely — and carries it into its next incarnation, which + // starts its run loop inside `Kernel.make`, before an embedder can call + // `initRemoteComms` to restore any remote to deliver it to. One message from + // a peer is therefore enough to stop a kernel ever booting again, given only + // that it restarts at some point. + const tempDir = await mkdtemp(join(tmpdir(), 'kernel-test-rc-reap-')); + const dbFile = join(tempDir, 'victim.db'); + try { + // Only the victim needs to survive a restart, so only it needs a file. + await kernel1.stop(); + const victimStore = makeKernelStore( + await makeSQLKernelDatabase({ dbFilename: dbFile }), + ); + let victim = await makeTestKernel( + 'victim', + await makeSQLKernelDatabase({ dbFilename: dbFile }), + directNetwork, + true, + 'kernel1-peer', + '01', + ); + + // One exchange, so each kernel holds a remote for the other. + await runTestVats(victim, makeSenderSubclusterConfig('Sender')); + const receiver = (await runTestVats( + kernel2, + makeReceiverSubclusterConfig('Receiver'), + )) as BootstrapResult; + await victim.queueMessage( + victimStore.getRootObject('v1') as KRef, + 'sendMessage', + [receiver.ocapURL, 'hello', ['once']], + ); + + // The attack, in one message. The peer is given local work purely so its + // own loop cranks and sends the request; nothing touches the victim + // afterwards, so the victim's loop stays parked and never delivers the + // reap it just queued. + kernel2.reapRemotes(); + await kernel2.queueMessage( + makeKernelStore(kernelDatabase2).getRootObject('v1') as KRef, + 'hello', + ['probe'], + ); + await waitUntilQuiescent(); + await victim.stop(); + + // Asserted, not assumed: if the victim had cranked it would have eaten its + // own reap while the remote still existed, and the rest would prove nothing. + const armed = await makeSQLKernelDatabase({ dbFilename: dbFile }); + expect( + JSON.parse(armed.kernelKVStore.get('reapQueue') ?? '[]'), + ).not.toStrictEqual([]); + + // Twice, because it is unrecoverable rather than merely fatal: the crank + // that dies is rolled back, which puts the reap back on the queue for the + // boot after this one. + let database = armed; + const bootStates = []; + for (const boot of [1, 2]) { + victim = await makeTestKernel( + `victim-boot${boot}`, + database, + directNetwork, + false, + 'kernel1-peer', + '01', + ); + bootStates.push((await victim.getStatus()).runLoop); + await victim.stop(); + database = await makeSQLKernelDatabase({ dbFilename: dbFile }); + } + // Asserted together rather than per boot, so a failure reports both: the + // point is that the second is no better than the first. + expect(bootStates).toStrictEqual([ + { state: 'running' }, + { state: 'running' }, + ]); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } + }); }); /** diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index 59412eb8f..726948e97 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -63,7 +63,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - A `notify`, GC action, or `bringOutYourDead` addressed to an endpoint that is not running is skipped instead of killing the run loop ([#1029](https://github.com/MetaMask/ocap-kernel/pull/1029)) - These deliveries looked up their endpoint unguarded, so an endpoint named by persisted state but absent from the running kernel threw `VatNotFoundError` from inside the crank, which killed the run loop permanently. Because the crank was rolled back the item was restored to the queue, so the next boot dequeued it and died too - - Reachable from `reapVats` alone: nothing purges the reap queue when a vat dies, and unlike a GC action — which `shouldProcessAction` filters on whether the endpoint still has a c-list entry — a reap is handed back with no liveness check at all. Schedule one, terminate that vat, and the run loop dequeues a `bringOutYourDead` addressed to a vat that no longer exists. Reaps are taken ahead of the run queue, so it also blocks everything behind it + - Reached by a peer's routine remote GC. A kernel answers a peer's `bringOutYourDead` by scheduling a reap against the remote it came from, and the reap queue is persisted. `scheduleReap` does not wake a parked run loop, so on an otherwise idle kernel that reap sits in the queue until something else gives the loop work — and a kernel shut down in the meantime carries it into the next incarnation. That incarnation starts its run loop inside `Kernel.make`, before an embedder can call `initRemoteComms`, which is what restores remote handles; reaps are taken ahead of the run queue, so the loop's first act is to deliver one addressed to a remote that does not exist yet. The kernel is dead before `Kernel.make` returns, and stays dead on every boot after that + - A reap is the delivery that reaches this most easily, because nothing filters it: a GC action is dropped by `shouldProcessAction` once the endpoint has no c-list entry, and a `notify` short-circuits on the same check, but a reap carries no kref and is handed back with no liveness check at all. Nothing purges the reap queue when its endpoint goes away - Also reachable while a terminated vat awaits cleanup, which happens one vat per crank, since `deleteVat` takes its config and subcluster membership but leaves its c-lists and reachable flags in place. Not via `terminateSubcluster`, which drains every pending cleanup after each vat it terminates - Unlike a `send`, none of these has a caller to reject; `send` already tolerated a vanished endpoint by rejecting the caller with `ENDPOINT_UNREACHABLE` - A skipped GC action still performs the kernel's own half — clearing the reachable flag, or tearing the c-list entry down — since that does not depend on the endpoint being there to be told. Skipping it would leave a dropped export flagged reachable, so the same action would be derived again on every sweep diff --git a/packages/ocap-kernel/src/KernelRouter.ts b/packages/ocap-kernel/src/KernelRouter.ts index 8e32a95f5..f9de97bba 100644 --- a/packages/ocap-kernel/src/KernelRouter.ts +++ b/packages/ocap-kernel/src/KernelRouter.ts @@ -361,13 +361,15 @@ export class KernelRouter { * Look up an endpoint for a housekeeping delivery — a notify, a GC action, or * a reap — tolerating one that is not running. * - * An endpoint can be named by persisted state without being live. Nothing - * purges the reap queue when a vat dies, and unlike a GC action — which - * `shouldProcessAction` filters on whether the endpoint still has a c-list - * entry — a reap is handed back with no liveness check at all, so a reap - * scheduled before a vat was terminated arrives after it. A terminated vat's - * c-lists also outlive it until `cleanupTerminatedVat` gets to it, one vat - * per crank, and the kernel goes on addressing it in the meantime. + * An endpoint can be named by persisted state without being live, and a reap + * reaches that state most easily because nothing filters it: a GC action is + * dropped once the endpoint has no c-list entry and a notify short-circuits + * on the same check, but a reap carries no kref and is handed back with no + * liveness check at all. Nothing purges the reap queue either, so one a peer + * asked for survives a shutdown — and the next incarnation starts this loop + * inside `Kernel.make`, before `initRemoteComms` restores any remote to + * deliver it to. A terminated vat's c-lists likewise outlive it until + * `cleanupTerminatedVat` gets to it, one vat per crank. * * Unlike a `send`, these deliveries have no caller to reject: nobody is * waiting on a `dropExport`. Throwing here instead escapes the crank and