fix(ocap-kernel): free retired exports and harden GC delivery

packages/kernel-test/src/refcount-audit.test.ts

@@ -0,0 +1,70 @@
+import { makeSQLKernelDatabase } from '@metamask/kernel-store/sqlite/nodejs';
+import { makeKernelStore } from '@metamask/ocap-kernel';
+import type { KRef, VatId } from '@metamask/ocap-kernel';
+import { expect, describe, it } from 'vitest';
+
+import {
+  getBundleSpec,
+  makeKernel,
+  makeMockLogger,
+  runTestVats,
+  takeRunLoopFailure,
+} from './utils.ts';
+
+/**
+ * The per-crank audit throws from inside the run loop, which nothing restarts.
+ * Unless that failure is reported to whoever is waiting on the kernel, the only
+ * symptom is a test that hangs until its timeout, with no mention of reference
+ * counts anywhere — which would make the audit worthless as a build gate.
+ */
+describe('reference count audit', () => {
+  it('reports a violation to kernel callers rather than hanging', async () => {
+    const kernelDatabase = await makeSQLKernelDatabase({
+      dbFilename: ':memory:',
+    });
+    const kernelStore = makeKernelStore(kernelDatabase);
+    const kernel = await makeKernel(kernelDatabase, true, makeMockLogger());
+    await runTestVats(kernel, {
+      bootstrap: 'exporter',
+      forceReset: true,
+      vats: {
+        exporter: {
+          bundleSpec: getBundleSpec('exporter-vat'),
+          parameters: { name: 'Exporter' },
+        },
+      },
+    });
+
+    const exporterVatId = kernel.getVats()[0]?.id as VatId;
+    const exporterKRef = kernelStore.getRootObject(exporterVatId) as KRef;
+
+    kernelStore.setObjectRefCount(exporterKRef, {
+      reachable: 7,
+      recognizable: 9,
+    });
+
+    // The crank carrying this message settles its result before the
+    // end-of-crank audit runs, so this one may still succeed.
+    await kernel
+      .queueMessage(exporterKRef, 'createObject', ['x'])
+      .catch(() => undefined);
+
+    // What a caller is told directly is that the run loop is gone; the audit
+    // failure that killed it rides along as the `cause`. That chain is the part
+    // that has to survive, since "run loop died" on its own names nothing.
+    const failure = (await kernel
+      .queueMessage(exporterKRef, 'createObject', ['y'])
+      .catch((error) => error)) as Error;
+
+    expect(failure.message).toMatch(/Kernel run loop died/u);
+    expect(String(failure.cause)).toMatch(
+      /reference count invariant violated/u,
+    );
+
+    // This test kills the loop deliberately, so the death is its result and not
+    // a stray one for the shared hooks to report against it.
+    expect(String(takeRunLoopFailure())).toMatch(
+      /reference count invariant violated/u,
+    );
+  }, 30000);
+});

packages/kernel-test/src/utils.ts

@@ -41,6 +41,18 @@ function assertRunLoopAlive(): void {
 afterEach(assertRunLoopAlive);
 afterAll(assertRunLoopAlive);
 
+/**
+ * Claim a run loop death the running test caused on purpose, so the hooks above
+ * do not report it as the test's own failure.
+ *
+ * @returns The failure, if the loop has died since the last check.
+ */
+export function takeRunLoopFailure(): Error | undefined {
+  const failure = runLoopFailure;
+  runLoopFailure = undefined;
+  return failure;
+}
+
 /**
  * Kernel options under which reference count drift fails the test run.
  *

packages/ocap-kernel/CHANGELOG.md

@@ -39,11 +39,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Reports drift in both directions: counts too low, which lets a live capability be collected, and counts too high, which keeps a dead one alive. A holder that should have been torn down but wasn't is not detectable this way, since it justifies its own count
   - Only references visible in the kernel's own state are checkable, so a holder that keeps a kref outside them has to take a pin to be counted at all
   - `recomputeRefCounts` is a repair tool for a drifted store, offered to embedders and never run automatically: opening an existing store does not migrate it. Reach it by calling `makeKernelStore` over the kernel's own database
-  - Exports the `RefCountViolation` type
+  - Exports the `RefCountViolation` type, a union over `kind: 'mismatch' | 'dangling'`
 - Add `setReachableFlag` to the kernel store, the counterpart to `clearReachableFlag` ([#1020](https://github.com/MetaMask/ocap-kernel/pull/1020))
 - Add `getOcapURLObjects`, `getOcapURLIssuanceCount`, `retainForOcapURL`, `undoOcapURLRetention` and `releaseOcapURLRetentions` to the kernel store, and `VatManager.releaseVatRootPin` ([#1020](https://github.com/MetaMask/ocap-kernel/pull/1020))
   - `undoOcapURLRetention` unwinds one issuance whose URL was never minted; `releaseOcapURLRetentions` drops a target's whole retention, for disavowing every URL naming it at once
 - Add `getPinCount` to the kernel store, which reports how many pins are held on an object ([#1020](https://github.com/MetaMask/ocap-kernel/pull/1020))
+- Add `orphanKernelObject` to the kernel store, which drops an object's owner mapping and hands it to the collector ([#1022](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1022))
 
 ### Changed
 
@@ -102,6 +103,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - A root is addressable while its vat lives whether or not anyone imports it; the old `(1, 1)` birth baseline was standing in for this
 - Garbage-collection action delivery now moves the kernel's own c-list: `dropExports` clears the owner's reachable flag and `retireExports`/`retireImports` tear the entry down ([#1020](https://github.com/MetaMask/ocap-kernel/pull/1020))
   - Previously the owner's flag never cleared, so the same action could be re-derived, and retired entries outlived the objects they named
+- Orphan a kernel object when its owner stops naming it (a delivered `retireExport`, or a `retireExports`/`abandonExports` syscall), so its `owner` and `refCount` records no longer outlive the c-list entry they were reachable through ([#1022](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1022))
+  - They leaked, and the next collection to visit such a kref read a c-list entry that was no longer there and killed the run loop. Reproduces on `main`, so it predates this stack
+- Reject a `retireExports`/`abandonExports` syscall for an object the calling endpoint does not own, instead of letting it erase another endpoint's claim to an object it is still exporting ([#1022](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1022))
+  - Nothing upstream of `performExportCleanup` checked that the vref it was handed is even an export, and the audit could not see the damage, because an export entry carries no count
+- Garbage-collection action delivery releases the kernel's side even when the endpoint has vanished, and rolls back rather than committing a release the endpoint was never told about ([#1022](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1022))
+  - It releases only where the endpoint is genuinely gone: a terminated vat, whose cleanup tears the whole c-list down anyway, or a remote, which reconciles on its next incarnation. A vat that is absent yet not terminated is one `restartVat` has taken out of the kernel's reach while keeping its c-list, so the crank fails there instead of committing a release the returning incarnation would disagree with
+- A failed garbage-collection delivery to a remote is logged and survived rather than escaping the crank and stopping the run loop ([#1022](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1022))
+- Tear down and mark for cleanup a vat whose worker launched but whose kernel-side registration then failed ([#1022](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1022))
 - Charge a delivered message's target reference against the run-queue item's own target rather than the routed target, so a message routed through a resolved promise no longer decrements an object nobody charged while leaking the promise ([#1020](https://github.com/MetaMask/ocap-kernel/pull/1020))
 - Release a queued notification's reference before the paths that decide there is nothing to deliver, and stop decrementing references on promises retired alongside it that nobody had taken ([#1020](https://github.com/MetaMask/ocap-kernel/pull/1020))
 - Transfer, rather than duplicate, the references a message carries when it is queued on an unresolved promise and later re-enqueued on resolution ([#1020](https://github.com/MetaMask/ocap-kernel/pull/1020))

packages/ocap-kernel/src/Kernel.ts

@@ -111,8 +111,12 @@ export class Kernel {
    * @param options.onRunLoopFailure - Optional handler called if the run loop dies.
    * @param options.auditRefCounts - If true, verify every kref's reference
    * counts against the references the kernel actually holds at the end of each
-   * crank, and throw on any mismatch. Intended for tests and debugging; the
-   * audit walks the whole store.
+   * crank, and throw on any mismatch. This is the check standing in for the
+   * accounting invariant `collectGarbage` still cannot assert (see the comment
+   * on its `retireExport` branch), so it is not optional
+   * instrumentation: it is off by default only because it walks the whole store
+   * every crank. Any kernel whose accounting is under test wants it on, and
+   * every kernel `kernel-test` builds enables it.
    */
   // eslint-disable-next-line no-restricted-syntax
   private constructor(

packages/ocap-kernel/src/KernelRouter.test.ts

@@ -66,6 +66,9 @@ describe('KernelRouter', () => {
       clearReachableFlag: vi.fn(),
       deleteCListEntry: vi.fn(),
       forgetKref: vi.fn(),
+      orphanKernelObject: vi.fn(),
+      hasCListEntry: vi.fn().mockReturnValue(true),
+      isVatTerminated: vi.fn().mockReturnValue(false),
       createCrankSavepoint: vi.fn(),
     } as unknown as KernelStore;
 
@@ -782,6 +785,168 @@ describe('KernelRouter', () => {
           ]);
         },
       );
+
+      it('orphans the object when delivering retireExports', async () => {
+        await kernelRouter.deliver({
+          type: 'retireExports',
+          endpointId: 'v1',
+          krefs: ['ko1', 'ko2'],
+        });
+
+        // The owner has given up the last name for the object, so the kernel's
+        // record of who owns it must go too or it outlives every reference.
+        expect(
+          (kernelStore.orphanKernelObject as unknown as MockInstance).mock
+            .calls,
+        ).toStrictEqual([
+          ['ko1', 'v1'],
+          ['ko2', 'v1'],
+        ]);
+      });
+
+      it('leaves ownership alone when delivering retireImports', async () => {
+        await kernelRouter.deliver({
+          type: 'retireImports',
+          endpointId: 'v1',
+          krefs: ['ko1'],
+        });
+
+        expect(kernelStore.orphanKernelObject).not.toHaveBeenCalled();
+      });
+
+      it('still releases the kernel side when a terminated vat has vanished', async () => {
+        getEndpoint.mockImplementationOnce(() => {
+          throw Error('vat v1 not found');
+        });
+        (
+          kernelStore.isVatTerminated as unknown as MockInstance
+        ).mockReturnValue(true);
+
+        const result = await kernelRouter.deliver({
+          type: 'retireImports',
+          endpointId: 'v1',
+          krefs: ['ko1'],
+        });
+
+        expect(result).toStrictEqual({ didDelivery: 'v1' });
+        // The action has already been consumed, so skipping the teardown would
+        // lose it and leave the entry behind for good
+        expect(kernelStore.deleteCListEntry).toHaveBeenCalledWith(
+          'v1',
+          'ko1',
+          'translated-ko1',
+        );
+      });
+
+      it('still releases the kernel side when a remote has vanished', async () => {
+        getEndpoint.mockImplementationOnce(() => {
+          throw Error('remote r1 not found');
+        });
+
+        const result = await kernelRouter.deliver({
+          type: 'retireImports',
+          endpointId: 'r1',
+          krefs: ['ko1'],
+        });
+
+        expect(result).toStrictEqual({ didDelivery: 'r1' });
+        expect(kernelStore.deleteCListEntry).toHaveBeenCalledWith(
+          'r1',
+          'ko1',
+          'translated-ko1',
+        );
+      });
+
+      it.each(['dropExports', 'retireExports', 'retireImports'] as const)(
+        'refuses to release %s for a vat that is absent but not terminated',
+        async (actionType) => {
+          // A vat between incarnations still holds every one of these krefs, so
+          // committing the kernel's release would leave the two disagreeing.
+          getEndpoint.mockImplementationOnce(() => {
+            throw Error('vat v1 not found');
+          });
+
+          await expect(
+            kernelRouter.deliver({
+              type: actionType,
+              endpointId: 'v1',
+              krefs: ['ko1'],
+            }),
+          ).rejects.toThrow('vat v1 not found');
+
+          expect(kernelStore.clearReachableFlag).not.toHaveBeenCalled();
+          expect(kernelStore.deleteCListEntry).not.toHaveBeenCalled();
+          expect(kernelStore.orphanKernelObject).not.toHaveBeenCalled();
+        },
+      );
+
+      it('skips krefs already cleaned up before delivery', async () => {
+        (
+          kernelStore.hasCListEntry as unknown as MockInstance
+        ).mockImplementation(
+          (_endpointId: string, kref: string) => kref === 'ko1',
+        );
+
+        await kernelRouter.deliver({
+          type: 'retireImports',
+          endpointId: 'v1',
+          krefs: ['ko1', 'ko2'],
+        });
+
+        expect(
+          (kernelStore.deleteCListEntry as unknown as MockInstance).mock.calls,
+        ).toStrictEqual([['v1', 'ko1', 'translated-ko1']]);
+      });
+
+      it('does nothing when every kref is already gone', async () => {
+        (kernelStore.hasCListEntry as unknown as MockInstance).mockReturnValue(
+          false,
+        );
+
+        const result = await kernelRouter.deliver({
+          type: 'retireImports',
+          endpointId: 'v1',
+          krefs: ['ko1'],
+        });
+
+        expect(result).toStrictEqual({ didDelivery: 'v1' });
+        expect(kernelStore.deleteCListEntry).not.toHaveBeenCalled();
+        expect(endpointHandle.deliverRetireImports).not.toHaveBeenCalled();
+      });
+
+      it('rolls back and terminates the vat when delivery fails', async () => {
+        (
+          endpointHandle.deliverRetireImports as unknown as MockInstance
+        ).mockRejectedValueOnce(Error('endpoint went away mid-delivery'));
+
+        const result = await kernelRouter.deliver({
+          type: 'retireImports',
+          endpointId: 'v1',
+          krefs: ['ko1'],
+        });
+
+        // Committing the release while v1 still holds the eref would leave the
+        // two disagreeing, and v1 would mint a fresh kref for the same object
+        expect(result?.abort).toBe(true);
+        expect(result?.terminate?.vatId).toBe('v1');
+      });
+
+      it('does not retry a remote that refuses the delivery', async () => {
+        (
+          endpointHandle.deliverRetireImports as unknown as MockInstance
+        ).mockRejectedValueOnce(Error('remote queue full'));
+
+        const result = await kernelRouter.deliver({
+          type: 'retireImports',
+          endpointId: 'r1',
+          krefs: ['ko1'],
+        });
+
+        // Aborting would restore the action, and GC actions are selected ahead
+        // of all other work, so a remote that keeps refusing would be handed
+        // this same item every crank and nothing else would ever run
+        expect(result).toStrictEqual({ didDelivery: 'r1' });
+      });
     });
 
     describe('bringOutYourDead', () => {