refactor(swingset): create vat state non-lazily

Previously, vat state initialization (e.g. setting counters to zero)
happened lazily, the first time that `provideVatKeeper()` was
called. When creating a new vat, the pattern was:

  vk = kernelKeeper.provideVatKeeper();
  vk.setSourceAndOptions(source, options);

Now, we initialize both counters and source/options explicitly, in
`recordVatOptions`, when the static/dynamic vat is first defined:

  kernelKeeper.createVatState(vatID, source, options);

In the future, this will make it easier for `provideVatKeeper` to rely
upon the presence of all vat state keys, especially `options`.

Previously, vatKeeper.getOptions() would tolerate a missing key by
returning empty options. The one place where this was
needed (terminating a barely-created vat because startVat() failed,
using getOptions().critical) was changed, so this tolerance is no
longer needed, and was removed. The tolerance caused bug #9157 (kernel
doesn't panic when it should), which continues to be present, but at
least won't cause a crash.

refs #8980

packages/SwingSet/src/kernel/kernel.js

@@ -252,8 +252,7 @@ export default function buildKernel(
    */
   async function terminateVat(vatID, shouldReject, info) {
     console.log(`kernel terminating vat ${vatID} (failure=${shouldReject})`);
-    const vatKeeper = kernelKeeper.provideVatKeeper(vatID);
-    const critical = vatKeeper.getOptions().critical;
+    let critical = false;
     insistCapData(info);
     // ISSUE: terminate stuff in its own crank like creation?
     // TODO: if a static vat terminates, panic the kernel?
@@ -264,6 +263,14 @@ export default function buildKernel(
     // check will report 'false'. That's fine, there's no state to
     // clean up.
     if (kernelKeeper.vatIsAlive(vatID)) {
+      // If there was no vat state, we can't make a vatKeeper to ask for
+      // options. For now, pretend the vat was non-critical. This will fail
+      // to panic the kernel for startVat failures in critical vats
+      // (#9157). The fix will add .critical to CrankResults, populated by a
+      // getOptions query in deliveryCrankResults() or copied from
+      // dynamicOptions in processCreateVat.
+      critical = kernelKeeper.provideVatKeeper(vatID).getOptions().critical;
+
       // Reject all promises decided by the vat, making sure to capture the list
       // of kpids before that data is deleted.
       const deadPromises = [...kernelKeeper.enumeratePromisesByDecider(vatID)];

packages/SwingSet/src/kernel/state/kernelKeeper.js

@@ -1288,15 +1288,17 @@ export default function makeKernelKeeper(kernelStorage, kernelSlog) {
     maybeFreeKrefs.clear();
   }
 
+  function createVatState(vatID, source, options) {
+    initializeVatState(kvStore, transcriptStore, vatID, source, options);
+  }
+
   function provideVatKeeper(vatID) {
     insistVatID(vatID);
     const found = ephemeral.vatKeepers.get(vatID);
     if (found !== undefined) {
       return found;
     }
-    if (!kvStore.has(`${vatID}.o.nextID`)) {
-      initializeVatState(kvStore, transcriptStore, vatID);
-    }
+    assert(kvStore.has(`${vatID}.o.nextID`), `${vatID} was not initialized`);
     const vk = makeVatKeeper(
       kvStore,
       transcriptStore,
@@ -1610,6 +1612,7 @@ export default function makeKernelKeeper(kernelStorage, kernelSlog) {
     getVatIDForName,
     allocateVatIDForNameIfNeeded,
     allocateUnusedVatID,
+    createVatState,
     provideVatKeeper,
     vatIsAlive,
     evictVatKeeper,

packages/SwingSet/src/kernel/state/vatKeeper.js

@@ -39,11 +39,30 @@ const FIRST_DEVICE_ID = 70n;
  * @param {*} transcriptStore  Accompanying transcript store
  * @param {string} vatID The vat ID string of the vat in question
  * TODO: consider making this part of makeVatKeeper
+ * @param {SourceOfBundle} source
+ * @param {RecordedVatOptions} options
  */
-export function initializeVatState(kvStore, transcriptStore, vatID) {
+export function initializeVatState(
+  kvStore,
+  transcriptStore,
+  vatID,
+  source,
+  options,
+) {
+  assert(options.workerOptions, `vat ${vatID} options missing workerOptions`);
+  assert(source);
+  assert('bundle' in source || 'bundleName' in source || 'bundleID' in source);
+  assert.typeof(options, 'object');
+  const count = options.reapInterval;
+  assert(count === 'never' || isNat(count), `bad reapCountdown ${count}`);
+
   kvStore.set(`${vatID}.o.nextID`, `${FIRST_OBJECT_ID}`);
   kvStore.set(`${vatID}.p.nextID`, `${FIRST_PROMISE_ID}`);
   kvStore.set(`${vatID}.d.nextID`, `${FIRST_DEVICE_ID}`);
+  kvStore.set(`${vatID}.source`, JSON.stringify(source));
+  kvStore.set(`${vatID}.options`, JSON.stringify(options));
+  kvStore.set(`${vatID}.reapInterval`, `${count}`);
+  kvStore.set(`${vatID}.reapCountdown`, `${count}`);
   transcriptStore.initTranscript(vatID);
 }
 
@@ -125,16 +144,10 @@ export function makeVatKeeper(
 
   function getOptions() {
     /** @type { RecordedVatOptions } */
-    const options = JSON.parse(kvStore.get(`${vatID}.options`) || '{}');
+    const options = JSON.parse(getRequired(`${vatID}.options`));
     return harden(options);
   }
 
-  function initializeReapCountdown(count) {
-    count === 'never' || isNat(count) || Fail`bad reapCountdown ${count}`;
-    kvStore.set(`${vatID}.reapInterval`, `${count}`);
-    kvStore.set(`${vatID}.reapCountdown`, `${count}`);
-  }
-
   function updateReapInterval(reapInterval) {
     reapInterval === 'never' ||
       isNat(reapInterval) ||
@@ -656,7 +669,6 @@ export function makeVatKeeper(
     setSourceAndOptions,
     getSourceAndOptions,
     getOptions,
-    initializeReapCountdown,
     countdownToReap,
     updateReapInterval,
     nextDeliveryNum,

packages/SwingSet/src/lib/recordVatOptions.js

@@ -39,9 +39,7 @@ export const makeVatOptionRecorder = (kernelKeeper, bundleHandler) => {
       critical,
       meterID,
     });
-    const vatKeeper = kernelKeeper.provideVatKeeper(vatID);
-    vatKeeper.setSourceAndOptions(source, vatOptions);
-    vatKeeper.initializeReapCountdown(vatOptions.reapInterval);
+    kernelKeeper.createVatState(vatID, source, vatOptions);
   };
 
   /**

packages/SwingSet/test/clist.test.js

@@ -13,6 +13,9 @@ test(`clist reachability`, async t => {
   const s = kk.kvStore;
   kk.createStartingKernelState({ defaultManagerType: 'local' });
   const vatID = kk.allocateUnusedVatID();
+  const source = { bundleID: 'foo' };
+  const options = { workerOptions: 'foo', reapInterval: 1 };
+  kk.createVatState(vatID, source, options);
   const vk = kk.provideVatKeeper(vatID);
 
   const ko1 = kk.addKernelObject('v1', 1);
@@ -98,12 +101,17 @@ test('getImporters', async t => {
 
   kk.createStartingKernelState({ defaultManagerType: 'local' });
   const vatID1 = kk.allocateUnusedVatID();
+  const source = { bundleID: 'foo' };
+  const options = { workerOptions: 'foo', reapInterval: 1 };
+  kk.createVatState(vatID1, source, options);
   kk.addDynamicVatID(vatID1);
   const vk1 = kk.provideVatKeeper(vatID1);
   const vatID2 = kk.allocateUnusedVatID();
+  kk.createVatState(vatID2, source, options);
   kk.addDynamicVatID(vatID2);
   const vk2 = kk.provideVatKeeper(vatID2);
   const vatID3 = kk.allocateUnusedVatID();
+  kk.createVatState(vatID3, source, options);
   kk.addDynamicVatID(vatID3);
   const vk3 = kk.provideVatKeeper(vatID3);
 

packages/SwingSet/test/state.test.js

@@ -511,8 +511,11 @@ test('vatKeeper', async t => {
   const store = buildKeeperStorageInMemory();
   const k = makeKernelKeeper(store, null);
   k.createStartingKernelState({ defaultManagerType: 'local' });
-
   const v1 = k.allocateVatIDForNameIfNeeded('name1');
+  const source = { bundleID: 'foo' };
+  const options = { workerOptions: 'foo', reapInterval: 1 };
+  k.createVatState(v1, source, options);
+
   const vk = k.provideVatKeeper(v1);
   // TODO: confirm that this level of caching is part of the API
   t.is(vk, k.provideVatKeeper(v1));
@@ -547,18 +550,15 @@ test('vatKeeper.getOptions', async t => {
   const store = buildKeeperStorageInMemory();
   const k = makeKernelKeeper(store, null);
   k.createStartingKernelState({ defaultManagerType: 'local' });
-
   const v1 = k.allocateVatIDForNameIfNeeded('name1');
-  const vk = k.provideVatKeeper(v1);
   const bundleID =
     'b1-00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000';
-  vk.setSourceAndOptions(
-    { bundleID },
-    {
-      workerOptions: { type: 'local' },
-      name: 'fred',
-    },
-  );
+  const source = { bundleID };
+  const workerOptions = { type: 'local' };
+  const options = { workerOptions, name: 'fred', reapInterval: 1 };
+  k.createVatState(v1, source, options);
+
+  const vk = k.provideVatKeeper(v1);
   const { name } = vk.getOptions();
   t.is(name, 'fred');
 });