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

packages/kernel-test/src/crank-rollback.test.ts

@@ -205,6 +205,36 @@ describe('crank rollback against a real database', () => {
     kernelStore.endCrank();
   });
 
+  // The set is not per-crank: only `collectGarbage` empties it, and that runs at
+  // the end of a crank that had an item. So a candidate created while the run
+  // loop was idle — `terminateVat` unpinning a root is the real path — is still
+  // owed a collection, and an unrelated crank's rollback must not cancel it.
+  it('keeps GC candidates that predate the crank it rolled back', async () => {
+    const { kernelStore } = await makeStore();
+    const idle = kernelStore.initKernelPromise()[0];
+    kernelStore.decrementRefCount(idle, 'test');
+
+    kernelStore.startCrank();
+    kernelStore.createCrankSavepoint('start');
+    const abandoned = kernelStore.initKernelPromise()[0];
+    kernelStore.decrementRefCount(abandoned, 'test');
+    kernelStore.rollbackCrank('start');
+    kernelStore.endCrank();
+
+    kernelStore.startCrank();
+    kernelStore.createCrankSavepoint('start');
+    kernelStore.collectGarbage();
+    kernelStore.endCrank();
+
+    // Collected, because it was owed before the abandoned crank began.
+    expect(() => kernelStore.getKernelPromise(idle)).toThrow(
+      'unknown kernel promise',
+    );
+  });
+
+  // `createCrankSavepoint` records the name only once the database has the
+  // savepoint. Asking to roll back one that was never created must therefore say
+  // so, rather than releasing someone else's savepoint.
   it('refuses to roll back a savepoint that was never created', async () => {
     const { kernelStore } = await makeStore();
 

packages/kernel-test/src/garbage-collection.test.ts

@@ -256,23 +256,40 @@ describe('Garbage Collection', () => {
     /**
      * Give an importer a chance to notice a dropped object and tell the kernel.
      *
+     * Waits for `done` as well as for an empty action set, because an empty set
+     * is also what "the vat has not told us anything yet" looks like. A vat
+     * reports a dropped import only once the engine has actually collected it,
+     * and `gcAndFinalize` can only provoke that, not guarantee it on the first
+     * try — so a round that reports nothing has to be retried rather than read
+     * as the end of the story. Reaped afresh each round for the same reason:
+     * the report rides on a `bringOutYourDead`.
+     *
      * @param vatId - The vat to reap.
      * @param rootKRef - That vat's root, to poke with cranks afterwards.
-     * @param settled - Whether the state under test has arrived yet.
+     * @param done - The outcome being waited for.
      */
     async function reapAndSettle(
       vatId: VatId,
       rootKRef: KRef,
-      settled: () => boolean,
+      done: () => boolean,
     ): Promise<void> {
-      // Reap until the vat's GC is visible rather than a fixed number of times:
-      // three was enough on an idle machine and not under a loaded one, which
-      // made this the last flake in the file.
-      for (let attempt = 0; attempt < 5 && !settled(); attempt += 1) {
+      const maxRounds = 10;
+      for (let round = 0; round < maxRounds; round++) {
         kernel.reapVats((id) => id === vatId);
+        // BOYD has to reach the vat, the vat has to answer, and the kernel has
+        // to act on the answer — but a round can queue more work, so loop until
+        // the queue is actually empty rather than guessing at a crank count.
         await kernel.queueMessage(rootKRef, 'noop', []);
         await waitUntilQuiescent(500);
+        if ([...kernelStore.getGCActions()].length === 0 && done()) {
+          return;
+        }
       }
+      throw Error(
+        `GC did not settle after ${maxRounds} rounds; actions pending: ${
+          [...kernelStore.getGCActions()].join(', ') || '(none)'
+        }`,
+      );
     }
 
     it('survives until both importers let go', async () => {
@@ -306,10 +323,10 @@ describe('Garbage Collection', () => {
       await kernel.queueMessage(importerKRef, 'makeWeak', [objectId]);
       await kernel.queueMessage(importerKRef, 'forgetImport', []);
       await waitUntilQuiescent();
-      await reapAndSettle(importerVatId, importerKRef, () =>
-        kernelStore
-          .getImporters(sharedKRef)
-          .every((vatId) => vatId !== importerVatId),
+      await reapAndSettle(
+        importerVatId,
+        importerKRef,
+        () => !kernelStore.getImporters(sharedKRef).includes(importerVatId),
       );
 
       // The exporter must not have been told to drop it: the second importer

packages/ocap-kernel/CHANGELOG.md

@@ -77,6 +77,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Buffered vat outputs are flushed after that work rather than before it, so a later failure can no longer roll the crank back underneath an answer already given. Termination still settles the dying vat's own promises immediately
 - `rollbackCrank` now also reverts the state a database rollback cannot reach, whether or not the rollback itself succeeded: cached stored values are re-read, and the crank's accumulated garbage-collection candidates are discarded ([#1021](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1021))
   - Otherwise a cache kept the abandoned crank's value and the next write persisted it, a GC action taken out of the set for an aborted delivery was lost rather than retried, and a later `collectGarbage` killed the run loop on a promise the rollback had deleted
+  - The candidate set is restored to its state at the savepoint rather than emptied, since candidates accrued while the run loop was idle are owed a collection an unrelated crank's rollback must not cancel ([#1023](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1023))
 - Cleanup after a dead crank no longer replaces the error that killed the run loop: a failing `endCrank` reports it as `cause`, and neither `endCrank` nor `rollbackCrank` retries a savepoint that a failed rollback or release has already discarded ([#1021](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1021))
 - A failing savepoint rollback in `RemoteHandle.handleRemoteMessage` and `RemoteManager`'s incarnation change is logged rather than thrown, so the failure it was cleaning up after is what reaches the caller ([#1021](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1021))
 - Keep a crank and a savepoint taken through `KernelStore.createSavepoint` from overlapping ([#1021](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1021))
@@ -101,6 +102,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - **A store written by an earlier version must be reset.** There is no migration: every object in it is still at `(1, 1)` and no vat root is pinned, so the second importer's `dropImports` underflows mid-crank and the last importer's drop can retire a live vat's root. Pins also moved from a single `pinnedObjects` row to a count per object at `pinned.${kref}`, and the old row is no longer read by anything — so every pin in such a store is silently lost on open while the refcount unit each one took remains. `recomputeRefCounts` can rebuild the counts, but not the pins, so it is a diagnostic rather than an upgrade path
 - Pin vat root objects for the lifetime of their vat, and release the pin on termination ([#1020](https://github.com/MetaMask/ocap-kernel/pull/1020))
   - 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
+  - The pin is released when a relaunch fails too, which vat cleanup does not do ([#1023](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1023))
 - 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))
@@ -111,6 +113,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - 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))
+- A vat's death is recorded in one synchronous step, so a worker that refuses to go cannot leave the record half-written ([#1023](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1023))
+  - Only `deleteVat` removes a vat's config, and terminated-vat cleanup does not call it, so the previous interleaving could leave a vat marked terminated whose config survived — 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
+- The run loop carries out a vat restart itself, as a queued request, so a vat is never out of the kernel's reach while cranks run; a crank that landed in that window read a live vat as a dead one ([#1023](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1023))
+  - Adds the `restartVat` run-queue item and `KernelQueue.enqueueRestartVat()`. `Kernel.restartVat` settles when the crank has done it, and rejects outright if the run loop is dead
+  - One request per vat is queued at a time. Two left the leftover one restarting a vat whose caller had already been handed a live handle, and terminating it if that relaunch failed
+- A delivery whose endpoint has vanished is dropped only where the endpoint is gone for good — a terminated vat or a remote. `notify` and `bringOutYourDead` no longer take the run loop down when it has, and a `send` no longer reports a live endpoint as unreachable and discards a deliverable message ([#1023](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1023))
+  - Adds `VatManager.provideVat()`, which waits out a vat being torn down before answering, and makes the kernel's endpoint lookup asynchronous
+- A restart that cannot relaunch its vat now terminates it and reports the failure to the caller, instead of killing the run loop — which rolled the crank back, undoing the termination records and returning the request to the queue, so every subsequent process start replayed the same failing restart ([#1023](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1023))
+  - Including a relaunch whose stream dies before its handle exists, which took the run loop down even so
+- Terminating a vat with a restart still queued for it no longer kills the run loop when the crank reaches that request ([#1023](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1023))
+- Work outliving a vat that has already been cleaned up — a `bringOutYourDead` scheduled before it died, say — is dropped rather than taken as a live vat the kernel has lost track of, which killed the run loop. Cleanup unmarks the vat it finishes, so "terminated" alone could not identify one ([#1023](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1023))
+- `getImporters` now counts remotes, and terminated vats cleanup has not reached, so retiring an object queues a `retireImport` for each rather than deleting the object and leaving their c-list entries naming nothing ([#1015](https://github.com/Consensys-Incorporated/ocap-kernel/issues/1015))
+  - `deleteVat` drops the `vatConfig` row that enumerates a vat while its c-list survives until cleanup reaches it, one vat per crank. Terminating an object's owner and its importer close together, owner marked first, collects the orphan inside that window
+- A vat reports its dropped imports on the `bringOutYourDead` that provoked the collection, rather than on some later one. The queues are now drained before the sweep, since a pending continuation still holds its closure's objects and a sweep run with work outstanding finds them reachable ([#1023](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1023))
 - 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.test.ts

@@ -31,13 +31,23 @@ const mocks = vi.hoisted(() => {
 
     #rejectRunLoop: ((error: Error) => void) | undefined;
 
+    #deliver: ((item: unknown) => Promise<unknown>) | undefined;
+
     // Like the real run loop, this settles only if the kernel dies.
-    run = vi.fn(
-      async () =>
-        new Promise<never>((_resolve, reject) => {
-          this.#rejectRunLoop = reject;
-        }),
-    );
+    run = vi.fn(async (deliver: (item: unknown) => Promise<unknown>) => {
+      this.#deliver = deliver;
+      return new Promise<never>((_resolve, reject) => {
+        this.#rejectRunLoop = reject;
+      });
+    });
+
+    // A restart is the run loop's work, so stand in for it reaching the request
+    // on its next crank. The failure is absorbed here rather than dropped: the
+    // real run loop would die of it, and the caller hears about it from the
+    // waiter `restartVat` registered, not from this call.
+    enqueueRestartVat = vi.fn((vatId: string) => {
+      this.#deliver?.({ type: 'restartVat', vatId }).catch(() => undefined);
+    });
 
     /**
      * Fail the run loop, in the order the real `KernelQueue.run` does: the

packages/ocap-kernel/src/Kernel.ts

@@ -111,12 +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. 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.
+   * crank, and throw on any mismatch. Not optional instrumentation: it is what
+   * establishes that the accounting is right, and 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. Note
+   * that it checks counts against their holders, which is a different invariant
+   * from the one `collectGarbage`'s `retireExport` branch still cannot assert.
    */
   // eslint-disable-next-line no-restricted-syntax
   private constructor(
@@ -155,10 +155,13 @@ export class Kernel {
     // which would deadlock — this callback is invoked from within a crank.
     this.#kernelQueue = new KernelQueue(
       this.#kernelStore,
-      async (vatId, reason) => {
-        await this.#vatManager.stopVat(vatId, true, reason);
-        this.#kernelStore.markVatAsTerminated(vatId);
-      },
+      // `stopVat` rather than `terminateVat`: this runs inside the crank that
+      // decided the vat has to go, and `terminateVat` would wait for that same
+      // crank to end. It needs no such wait — the run loop is right here — and
+      // `stopVat` puts the whole death on record before its first await, so a
+      // worker that refuses to die cannot leave the store half-told.
+      async (vatId, reason) =>
+        await this.#vatManager.stopVat(vatId, true, reason),
     );
 
     this.#vatManager = new VatManager({
@@ -230,6 +233,7 @@ export class Kernel {
       this.#kernelServiceManager.invokeKernelService.bind(
         this.#kernelServiceManager,
       ),
+      this.#vatManager.performVatRestart.bind(this.#vatManager),
       this.#logger,
     );
 
@@ -651,13 +655,19 @@ export class Kernel {
   /**
    * Gets an endpoint by its ID.
    *
+   * Asynchronous because a vat may be mid-teardown: `provideVat` waits that out
+   * rather than answering from a vat table the store has not caught up with, so
+   * by the time a caller is told the vat is gone the store says so too — which
+   * is what lets `#resolveEndpoint` tell a terminated vat from a missing one. A
+   * restart needs no such window, being carried out by the run loop itself.
+   *
    * @param endpointId - The ID of the endpoint to retrieve.
-   * @returns The endpoint handle for the given ID.
+   * @returns A promise for the endpoint handle for the given ID.
    * @throws If the endpoint ID is invalid (neither a vat ID nor a remote ID).
    */
-  #getEndpoint(endpointId: EndpointId): EndpointHandle {
+  async #getEndpoint(endpointId: EndpointId): Promise<EndpointHandle> {
     if (isVatId(endpointId)) {
-      return this.#vatManager.getVat(endpointId);
+      return await this.#vatManager.provideVat(endpointId);
     }
     if (isRemoteId(endpointId)) {
       return this.#remoteManager.getRemote(endpointId);

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

@@ -1310,6 +1310,27 @@ describe('KernelQueue', () => {
     });
   });
 
+  describe('enqueueRestartVat', () => {
+    it('enqueues the request for the run loop to carry out', () => {
+      kernelQueue.enqueueRestartVat('v1');
+
+      expect(kernelStore.enqueueRun).toHaveBeenCalledWith({
+        type: 'restartVat',
+        vatId: 'v1',
+      });
+    });
+
+    it('refuses once the run loop has died', async () => {
+      await killRunLoop(new Error('boom'));
+
+      // The restart is the loop's work, so a dead loop will never do it and the
+      // caller would wait forever.
+      expect(() => kernelQueue.enqueueRestartVat('v1')).toThrow(
+        'Kernel run loop died; cannot restart a vat',
+      );
+    });
+  });
+
   describe('waitForCrank', () => {
     it('handles when waitForCrank returns a delayed promise', async () => {
       let resolvePromise: ((value: void) => void) | undefined;

packages/ocap-kernel/src/KernelQueue.ts

@@ -514,6 +514,23 @@ export class KernelQueue {
     }
   }
 
+  /**
+   * Enqueue a request to replace a vat's worker.
+   *
+   * The work itself belongs to the run loop, which is the point: a restart done
+   * where it is asked for takes the vat out of the kernel's reach while cranks
+   * continue, and a crank that lands in that window reads a live vat as a dead
+   * one. Queued, the restart happens in a crank of its own.
+   *
+   * @param vatId - The vat whose worker is to be replaced.
+   */
+  enqueueRestartVat(vatId: VatId): void {
+    // The restart is the run loop's work now, so a dead loop will never do it,
+    // and a caller awaiting it would wait forever.
+    this.assertRunLoopAlive('restart a vat');
+    this.#enqueueRun({ type: 'restartVat', vatId });
+  }
+
   /**
    * Enqueue a notification of promise resolution to an endpoint.
    *