fix(ocap-kernel): make c-list import accounting symmetric

packages/extension/test/e2e/control-panel.test.ts

@@ -252,7 +252,6 @@ test.describe('Control Panel', () => {
       `{"key":"v3.c.o+0","value":"${v3Root}"}`,
       `{"key":"v3.c.${v3Promise}","value":"R p-1"}`,
       `{"key":"v3.c.p-1","value":"${v3Promise}"}`,
-      `{"key":"${v3Root}.refCount","value":"1,1"}`,
       `{"key":"${v3Promise}.refCount","value":"2"}`,
     ];
     // Derived too: v1 imports the two roots as the bootstrap's calls are

packages/kernel-test/src/endowment-globals.test.ts

@@ -12,7 +12,11 @@ import type { AllowedGlobalName, KRef, VatId } from '@metamask/ocap-kernel';
 import { getWorkerFile } from '@ocap/nodejs-test-workers';
 import { describe, expect, it } from 'vitest';
 
-import { extractTestLogs, getBundleSpec } from './utils.ts';
+import {
+  extractTestLogs,
+  getBundleSpec,
+  makeAuditedKernelOptions,
+} from './utils.ts';
 
 describe('global endowments', () => {
   const vatId: VatId = 'v1';
@@ -38,6 +42,7 @@ describe('global endowments', () => {
       resetStorage: true,
       logger,
       allowedGlobalNames,
+      ...makeAuditedKernelOptions(),
     });
 
     await kernel.launchSubcluster({

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

@@ -21,9 +21,11 @@ import {
 /**
  * Make a test subcluster with vats for GC testing
  *
+ * @param extraImporters - Names of additional importer vats to include, for
+ * topologies where more than one vat shares the same exported object.
  * @returns The test subcluster
  */
-function makeTestSubcluster(): ClusterConfig {
+function makeTestSubcluster(extraImporters: string[] = []): ClusterConfig {
   return {
     bootstrap: 'exporter',
     forceReset: true,
@@ -40,6 +42,15 @@ function makeTestSubcluster(): ClusterConfig {
           name: 'Importer',
         },
       },
+      ...Object.fromEntries(
+        extraImporters.map((name) => [
+          name,
+          {
+            bundleSpec: getBundleSpec('importer-vat'),
+            parameters: { name },
+          },
+        ]),
+      ),
     },
   };
 }
@@ -81,10 +92,11 @@ describe('Garbage Collection', () => {
       [objectId],
     );
     const createObjectRef = createObjectData.slots[0] as KRef;
-    // Verify initial reference counts from database
-    const initialRefCounts = kernelStore.getObjectRefCount(createObjectRef);
-    expect(initialRefCounts.reachable).toBe(2);
-    expect(initialRefCounts.recognizable).toBe(2);
+    // Held only by the resolved promise's value, which still carries the slot
+    expect(kernelStore.getObjectRefCount(createObjectRef)).toStrictEqual({
+      reachable: 1,
+      recognizable: 1,
+    });
     // Send the object to the importer vat
     const objectRef = kunser(createObjectData);
     await kernel.queueMessage(importerKRef, 'storeImport', [objectRef]);
@@ -116,10 +128,10 @@ describe('Garbage Collection', () => {
     await waitUntilQuiescent();
     const createObjectRef = createObjectData.slots[0] as KRef;
 
-    // Store initial reference count information
-    const initialRefCounts = kernelStore.getObjectRefCount(createObjectRef);
-    expect(initialRefCounts.reachable).toBe(2);
-    expect(initialRefCounts.recognizable).toBe(2);
+    expect(kernelStore.getObjectRefCount(createObjectRef)).toStrictEqual({
+      reachable: 1,
+      recognizable: 1,
+    });
 
     // Store the reference in the importer vat
     const objectRef = kunser(createObjectData);
@@ -201,4 +213,116 @@ describe('Garbage Collection', () => {
     );
     expect(parseReplyBody(exporterFinalCheck.body)).toBe(false);
   }, 40000);
+
+  describe('an object shared by two importers', () => {
+    let secondImporterKRef: KRef;
+    let secondImporterVatId: VatId;
+
+    beforeEach(async () => {
+      kernelDatabase = await makeSQLKernelDatabase({ dbFilename: ':memory:' });
+      kernelStore = makeKernelStore(kernelDatabase);
+      kernel = await makeKernel(kernelDatabase, true, makeMockLogger());
+      await runTestVats(kernel, makeTestSubcluster(['Importer2']));
+
+      const vats = kernel.getVats();
+      const idOf = (name: string): VatId =>
+        vats.find((row) => row.config.parameters?.name === name)?.id as VatId;
+      exporterVatId = idOf('Exporter');
+      importerVatId = idOf('Importer');
+      secondImporterVatId = idOf('Importer2');
+      exporterKRef = kernelStore.getRootObject(exporterVatId) as KRef;
+      importerKRef = kernelStore.getRootObject(importerVatId) as KRef;
+      secondImporterKRef = kernelStore.getRootObject(
+        secondImporterVatId,
+      ) as KRef;
+    });
+
+    /**
+     * Give an importer a chance to notice a dropped object and tell the kernel.
+     *
+     * @param vatId - The vat to reap.
+     * @param rootKRef - That vat's root, to poke with cranks afterwards.
+     */
+    async function reapAndSettle(vatId: VatId, rootKRef: KRef): Promise<void> {
+      kernel.reapVats((id) => id === vatId);
+      for (let i = 0; i < 3; i++) {
+        await kernel.queueMessage(rootKRef, 'noop', []);
+        await waitUntilQuiescent(500);
+      }
+    }
+
+    it('survives until both importers let go', async () => {
+      const objectId = 'shared-object';
+      const createObjectData = await kernel.queueMessage(
+        exporterKRef,
+        'createObject',
+        [objectId],
+      );
+      const sharedKRef = createObjectData.slots[0] as KRef;
+      const objectRef = kunser(createObjectData);
+
+      for (const importer of [importerKRef, secondImporterKRef]) {
+        await kernel.queueMessage(importer, 'storeImport', [
+          objectRef,
+          objectId,
+        ]);
+      }
+      await waitUntilQuiescent();
+
+      expect(kernelStore.getImporters(sharedKRef)).toStrictEqual(
+        [importerVatId, secondImporterVatId].sort(),
+      );
+      // Two importers, plus the resolved createObject promise whose value
+      // still carries the slot
+      expect(kernelStore.getObjectRefCount(sharedKRef)).toStrictEqual({
+        reachable: 3,
+        recognizable: 3,
+      });
+
+      await kernel.queueMessage(importerKRef, 'makeWeak', [objectId]);
+      await kernel.queueMessage(importerKRef, 'forgetImport', []);
+      await waitUntilQuiescent();
+      await reapAndSettle(importerVatId, importerKRef);
+
+      // The exporter must not have been told to drop it: the second importer
+      // legitimately still holds it
+      expect(kernelStore.getReachableFlag(exporterVatId, sharedKRef)).toBe(
+        true,
+      );
+      expect(kernelStore.getImporters(sharedKRef)).toStrictEqual([
+        secondImporterVatId,
+      ]);
+      expect(
+        parseReplyBody(
+          (
+            await kernel.queueMessage(exporterKRef, 'isObjectPresent', [
+              objectId,
+            ])
+          ).body,
+        ),
+      ).toBe(true);
+
+      expect(
+        parseReplyBody(
+          (
+            await kernel.queueMessage(secondImporterKRef, 'useImport', [
+              objectId,
+            ])
+          ).body,
+        ),
+      ).toBe(objectId);
+
+      await kernel.queueMessage(secondImporterKRef, 'makeWeak', [objectId]);
+      await kernel.queueMessage(secondImporterKRef, 'forgetImport', []);
+      await waitUntilQuiescent();
+      await reapAndSettle(secondImporterVatId, secondImporterKRef);
+
+      expect(kernelStore.getImporters(sharedKRef)).toStrictEqual([]);
+      // Only the createObject result's stored value still names it
+      expect(kernelStore.getObjectRefCount(sharedKRef)).toStrictEqual({
+        reachable: 1,
+        recognizable: 1,
+      });
+    }, 60000);
+  });
 });

packages/kernel-test/src/io.test.ts

@@ -6,7 +6,11 @@ import * as os from 'node:os';
 import * as path from 'node:path';
 import { describe, it, expect, afterEach } from 'vitest';
 
-import { getBundleSpec, makeTestLogger } from './utils.ts';
+import {
+  getBundleSpec,
+  makeAuditedKernelOptions,
+  makeTestLogger,
+} from './utils.ts';
 
 function tempSocketPath(): string {
   return path.join(
@@ -79,6 +83,7 @@ async function makeIoKernel(
       resetStorage: true,
       logger,
       ioListenerFactory: makeIOListenerFactory(),
+      ...makeAuditedKernelOptions(),
     },
   );
 

packages/kernel-test/src/persistence.test.ts

@@ -176,7 +176,8 @@ describe('persistent storage', { timeout: 20_000 }, () => {
     // Enqueue a send message into the database
     kv1.set('queue.run.head', '4');
     kv1.set('nextPromiseId', '4');
-    kv1.set(`${v1Root}.refCount`, '3,3');
+    // The root's pin, plus the send being injected below.
+    kv1.set(`${v1Root}.refCount`, '2,2');
     kv1.set('queue.kp3.head', '1');
     kv1.set('queue.kp3.tail', '1');
     kv1.set('kp3.state', 'unresolved');

packages/kernel-test/src/utils.ts

@@ -11,8 +11,61 @@ import {
 } from '@metamask/logger';
 import type { LogEntry } from '@metamask/logger';
 import { Kernel, kunser } from '@metamask/ocap-kernel';
-import type { ClusterConfig, PlatformServices } from '@metamask/ocap-kernel';
-import { vi } from 'vitest';
+import type {
+  ClusterConfig,
+  OnRunLoopFailure,
+  PlatformServices,
+} from '@metamask/ocap-kernel';
+import { afterAll, afterEach, vi } from 'vitest';
+
+/**
+ * The first run loop death seen since it was last reported, held here rather
+ * than passed to a test because the crank that kills the loop is often one no
+ * test is awaiting — a garbage collection or reap crank, or one that lands
+ * after the last assertion. The hooks below are the only thing guaranteed to
+ * look, so they are registered for every file that imports this module.
+ */
+let runLoopFailure: Error | undefined;
+
+/**
+ * Fail the current test if a kernel's run loop has died since the last check.
+ */
+function assertRunLoopAlive(): void {
+  const failure = runLoopFailure;
+  runLoopFailure = undefined;
+  if (failure) {
+    throw failure;
+  }
+}
+
+afterEach(assertRunLoopAlive);
+afterAll(assertRunLoopAlive);
+
+/**
+ * Kernel options under which reference count drift fails the test run.
+ *
+ * Drift is invisible to ordinary assertions until something gets collected out
+ * from under a live holder, so the audit runs every crank. It reports by
+ * throwing, which kills the run loop — and the kernel hands run loop death to
+ * `onRunLoopFailure` rather than rethrowing it, deliberately, so that an
+ * embedder can decide what to do. Without a handler a violation therefore
+ * surfaces only if the killed crank happened to have a caller waiting on it.
+ *
+ * @returns Options to pass to `Kernel.make`.
+ */
+export function makeAuditedKernelOptions(): {
+  auditRefCounts: true;
+  onRunLoopFailure: OnRunLoopFailure;
+} {
+  return {
+    auditRefCounts: true,
+    // The first failure is the informative one: a dead loop cannot process
+    // anything, so whatever follows is downstream of it.
+    onRunLoopFailure: (failure: Error): void => {
+      runLoopFailure ??= failure;
+    },
+  };
+}
 
 /**
  * Construct a bundle path URL from a bundle name.
@@ -93,6 +146,7 @@ export async function makeKernel(
     resetStorage,
     logger,
     keySeed,
+    ...makeAuditedKernelOptions(),
   });
   return kernel;
 }