Merge pull request #3130 from Agoric/3108-clist-reachability

feat(SwingSet): add "reachable" flag to clist entries

packages/SwingSet/src/blockBuffer.js

@@ -43,7 +43,7 @@ export function buildBlockBuffer(hostDB) {
         keys.delete(k);
       }
       for (const k of Array.from(keys).sort()) {
-        if (start <= k && k < end) {
+        if ((start === '' || start <= k) && (end === '' || k < end)) {
           yield k;
         }
       }

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

@@ -42,7 +42,9 @@ const enableKernelPromiseGC = true;
 // v$NN.o.nextID = $NN
 // v$NN.p.nextID = $NN
 // v$NN.d.nextID = $NN
-// v$NN.c.$kernelSlot = $vatSlot = o+$NN/o-$NN/p+$NN/p-$NN/d+$NN/d-$NN
+// v$NN.c.$kernelSlot = '$R $vatSlot'
+//   $R is 'R' when reachable, '_' when merely recognizable
+//   $vatSlot is one of: o+$NN/o-$NN/p+$NN/p-$NN/d+$NN/d-$NN
 // v$NN.c.$vatSlot = $kernelSlot = ko$NN/kp$NN/kd$NN
 // v$NN.t.$NN = JSON(transcript entry)
 // v$NN.t.nextID = $NN

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

@@ -121,6 +121,8 @@ export function buildCrankBuffer(kvStore) {
     },
 
     *getKeys(start, end) {
+      assert.typeof(start, 'string');
+      assert.typeof(end, 'string');
       const keys = new Set(kvStore.getKeys(start, end));
       for (const k of additions.keys()) {
         keys.add(k);
@@ -129,13 +131,14 @@ export function buildCrankBuffer(kvStore) {
         keys.delete(k);
       }
       for (const k of Array.from(keys).sort()) {
-        if (start <= k && k < end) {
+        if ((start === '' || start <= k) && (end === '' || k < end)) {
           yield k;
         }
       }
     },
 
     get(key) {
+      assert.typeof(key, 'string');
       if (additions.has(key)) {
         return additions.get(key);
       }
@@ -146,11 +149,14 @@ export function buildCrankBuffer(kvStore) {
     },
 
     set(key, value) {
+      assert.typeof(key, 'string');
+      assert.typeof(value, 'string');
       additions.set(key, value);
       deletions.delete(key);
     },
 
     delete(key) {
+      assert.typeof(key, 'string');
       additions.delete(key);
       deletions.add(key);
     },

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

@@ -78,23 +78,61 @@ export function makeVatKeeper(
     return harden({ source, options });
   }
 
+  function parseReachableAndVatSlot(value) {
+    assert.typeof(value, 'string', X`non-string value: ${value}`);
+    const flag = value.slice(0, 1);
+    assert.equal(value.slice(1, 2), ' ');
+    const vatSlot = value.slice(2);
+    let reachable;
+    if (flag === 'R') {
+      reachable = true;
+    } else if (flag === '_') {
+      reachable = false;
+    } else {
+      assert(`flag (${flag}) must be 'R' or '_'`);
+    }
+    return { reachable, vatSlot };
+  }
+
+  function buildReachableAndVatSlot(reachable, vatSlot) {
+    return `${reachable ? 'R' : '_'} ${vatSlot}`;
+  }
+
+  function getReachableAndVatSlot(kernelSlot) {
+    const kernelKey = `${vatID}.c.${kernelSlot}`;
+    return parseReachableAndVatSlot(kvStore.get(kernelKey));
+  }
+
+  function setReachableFlag(kernelSlot) {
+    const kernelKey = `${vatID}.c.${kernelSlot}`;
+    const { vatSlot } = parseReachableAndVatSlot(kvStore.get(kernelKey));
+    kvStore.set(kernelKey, buildReachableAndVatSlot(true, vatSlot));
+  }
+
+  function clearReachableFlag(kernelSlot) {
+    const kernelKey = `${vatID}.c.${kernelSlot}`;
+    const { vatSlot } = parseReachableAndVatSlot(kvStore.get(kernelKey));
+    kvStore.set(kernelKey, buildReachableAndVatSlot(false, vatSlot));
+  }
+
   /**
-   * Provide the kernel slot corresponding to a given vat slot, creating the
-   * kernel slot if it doesn't already exist.
+   * Provide the kernel slot corresponding to a given vat slot, allocating a
+   * new one (for exports only) if it doesn't already exist. If we're allowed
+   * to allocate, we also ensure the 'reachable' flag is set on it (whether
+   * we allocated a new one or used an existing one). If we're not allowed to
+   * allocate, we insist that the reachable flag was already set.
    *
    * @param {string} vatSlot  The vat slot of interest
-   *
+   * @param {bool} setReachable Set the 'reachable' flag on vat exports.
    * @returns {string} the kernel slot that vatSlot maps to
-   *
    * @throws {Error} if vatSlot is not a kind of thing that can be exported by vats
-   *    or is otherwise invalid.
+   * or is otherwise invalid.
    */
-  function mapVatSlotToKernelSlot(vatSlot) {
+  function mapVatSlotToKernelSlot(vatSlot, setReachable = true) {
     assert.typeof(vatSlot, 'string', X`non-string vatSlot: ${vatSlot}`);
+    const { type, allocatedByVat } = parseVatSlot(vatSlot);
     const vatKey = `${vatID}.c.${vatSlot}`;
     if (!kvStore.has(vatKey)) {
-      const { type, allocatedByVat } = parseVatSlot(vatSlot);
-
       if (allocatedByVat) {
         let kernelSlot;
         if (type === 'object') {
@@ -109,7 +147,10 @@ export function makeVatKeeper(
         incrementRefCount(kernelSlot, `${vatID}|vk|clist`);
         const kernelKey = `${vatID}.c.${kernelSlot}`;
         incStat('clistEntries');
-        kvStore.set(kernelKey, vatSlot);
+        // we add the key as "unreachable", and then rely on
+        // setReachableFlag() at the end to both mark it reachable and to
+        // update any necessary refcounts consistently
+        kvStore.set(kernelKey, buildReachableAndVatSlot(false, vatSlot));
         kvStore.set(vatKey, kernelSlot);
         kernelSlog &&
           kernelSlog.changeCList(
@@ -126,22 +167,32 @@ export function makeVatKeeper(
         assert.fail(X`unknown vatSlot ${q(vatSlot)}`);
       }
     }
+    const kernelSlot = kvStore.get(vatKey);
 
-    return kvStore.get(vatKey);
+    if (setReachable) {
+      if (allocatedByVat) {
+        // exports are marked as reachable, if they weren't already
+        setReachableFlag(kernelSlot);
+      } else {
+        // imports must be reachable
+        const { reachable } = getReachableAndVatSlot(kernelSlot);
+        assert(reachable, X`vat tried to access unreachable import`);
+      }
+    }
+    return kernelSlot;
   }
 
   /**
    * Provide the vat slot corresponding to a given kernel slot, including
    * creating the vat slot if it doesn't already exist.
    *
    * @param {string} kernelSlot  The kernel slot of interest
-   *
+   * @param {bool} setReachable Set the 'reachable' flag on vat imports.
    * @returns {string} the vat slot kernelSlot maps to
-   *
    * @throws {Error} if kernelSlot is not a kind of thing that can be imported by vats
-   *    or is otherwise invalid.
+   * or is otherwise invalid.
    */
-  function mapKernelSlotToVatSlot(kernelSlot) {
+  function mapKernelSlotToVatSlot(kernelSlot, setReachable = true) {
     assert.typeof(kernelSlot, 'string', 'non-string kernelSlot');
     const kernelKey = `${vatID}.c.${kernelSlot}`;
     if (!kvStore.has(kernelKey)) {
@@ -166,7 +217,7 @@ export function makeVatKeeper(
       const vatKey = `${vatID}.c.${vatSlot}`;
       incStat('clistEntries');
       kvStore.set(vatKey, kernelSlot);
-      kvStore.set(kernelKey, vatSlot);
+      kvStore.set(kernelKey, buildReachableAndVatSlot(false, vatSlot));
       kernelSlog &&
         kernelSlog.changeCList(
           vatID,
@@ -178,7 +229,19 @@ export function makeVatKeeper(
       kdebug(`Add mapping k->v ${kernelKey}<=>${vatKey}`);
     }
 
-    return kvStore.get(kernelKey);
+    const { reachable, vatSlot } = getReachableAndVatSlot(kernelSlot);
+    const { allocatedByVat } = parseVatSlot(vatSlot);
+    if (setReachable) {
+      if (!allocatedByVat) {
+        // imports are marked as reachable, if they weren't already
+        setReachableFlag(kernelSlot);
+      } else {
+        // if the kernel is sending non-reachable exports back into
+        // exporting vat, that's a kernel bug
+        assert(reachable, X`kernel sent unreachable export`);
+      }
+    }
+    return vatSlot;
   }
 
   /**
@@ -293,6 +356,8 @@ export function makeVatKeeper(
     getSourceAndOptions,
     mapVatSlotToKernelSlot,
     mapKernelSlotToVatSlot,
+    setReachableFlag,
+    clearReachableFlag,
     hasCListEntry,
     deleteCListEntry,
     deleteCListEntriesForKernelSlots,

packages/SwingSet/test/test-clist.js

@@ -0,0 +1,89 @@
+import { test } from '../tools/prepare-test-env-ava';
+
+// eslint-disable-next-line import/order
+import { initSwingStore } from '@agoric/swing-store-simple';
+import { makeDummySlogger } from '../src/kernel/slogger';
+import makeKernelKeeper from '../src/kernel/state/kernelKeeper';
+import { wrapStorage } from '../src/kernel/state/storageWrapper';
+
+test(`clist reachability`, async t => {
+  const slog = makeDummySlogger({});
+  const { enhancedCrankBuffer: s } = wrapStorage(initSwingStore().kvStore);
+
+  const kk = makeKernelKeeper(s, slog);
+  kk.createStartingKernelState('local');
+  const vatID = kk.allocateUnusedVatID();
+  const vk = kk.allocateVatKeeper(vatID);
+
+  t.is(vk.mapKernelSlotToVatSlot('ko1'), 'o-50');
+  t.is(vk.mapKernelSlotToVatSlot('ko2'), 'o-51');
+  t.is(vk.mapKernelSlotToVatSlot('ko1'), 'o-50');
+
+  t.is(vk.mapVatSlotToKernelSlot('o+1'), 'ko20');
+  t.is(vk.mapVatSlotToKernelSlot('o+2'), 'ko21');
+  t.is(vk.mapVatSlotToKernelSlot('o+1'), 'ko20');
+
+  t.is(s.get(`${vatID}.c.o+1`), `ko20`);
+  t.is(s.get(`${vatID}.c.ko20`), 'R o+1');
+
+  // newly allocated imports are marked as reachable
+  t.is(s.get(`${vatID}.c.ko1`), 'R o-50');
+  // now pretend that the vat drops its o-50 import: the syscall.dropImport
+  // will cause the kernel to clear the reachability flag
+  vk.clearReachableFlag('ko1');
+  t.is(s.get(`${vatID}.c.ko1`), '_ o-50');
+  // while dropped, the vat may not access the import
+  t.throws(() => vk.mapVatSlotToKernelSlot('o-50'), {
+    message: /vat tried to access unreachable import/,
+  });
+
+  // now the kernel sends a new message that references ko1, causing a
+  // re-import
+  vk.setReachableFlag('ko1');
+  t.is(s.get(`${vatID}.c.ko1`), 'R o-50');
+  // re-import without intervening dropImport is idempotent
+  vk.setReachableFlag('ko1');
+  t.is(s.get(`${vatID}.c.ko1`), 'R o-50');
+
+  // test the same thing for exports
+  t.is(s.get(`${vatID}.c.ko20`), 'R o+1');
+  // kernel tells vat that kernel is dropping the export, clearing the
+  // reachability flag
+  vk.clearReachableFlag('ko20');
+  t.is(s.get(`${vatID}.c.ko20`), '_ o+1');
+  // while dropped, access by the kernel indicates a bug
+  t.throws(() => vk.mapKernelSlotToVatSlot('ko20'), {
+    message: /kernel sent unreachable export/,
+  });
+
+  // vat re-exports o+1
+  vk.setReachableFlag('ko20');
+  t.is(s.get(`${vatID}.c.ko20`), 'R o+1');
+  // re-export without intervening dropExport is idempotent
+  vk.setReachableFlag('ko20');
+  t.is(s.get(`${vatID}.c.ko20`), 'R o+1');
+
+  // test setReachable=false, used by handlers for GC operations like
+  // syscall.dropImport to talk about an import without claiming it's
+  // reachable or causing it to become reachable
+
+  t.is(vk.mapKernelSlotToVatSlot('ko3'), 'o-52');
+  vk.clearReachableFlag('ko3');
+  t.is(s.get(`${vatID}.c.ko3`), '_ o-52');
+  t.throws(() => vk.mapVatSlotToKernelSlot('o-52'), {
+    message: /vat tried to access unreachable import/,
+  });
+  t.is(vk.mapVatSlotToKernelSlot('o-52', false), 'ko3');
+  t.is(vk.mapKernelSlotToVatSlot('ko3', false), 'o-52');
+  t.is(s.get(`${vatID}.c.ko3`), '_ o-52');
+
+  t.is(vk.mapVatSlotToKernelSlot('o+3'), 'ko22');
+  vk.clearReachableFlag('ko22');
+  t.is(s.get(`${vatID}.c.ko22`), '_ o+3');
+  t.throws(() => vk.mapKernelSlotToVatSlot('ko22'), {
+    message: /kernel sent unreachable export/,
+  });
+  t.is(vk.mapKernelSlotToVatSlot('ko22', false), 'o+3');
+  t.is(vk.mapVatSlotToKernelSlot('o+3', false), 'ko22');
+  t.is(s.get(`${vatID}.c.ko22`), '_ o+3');
+});

packages/SwingSet/test/test-state.js

@@ -47,6 +47,9 @@ function testStorage(t, s, getState, commit) {
   s.set('foo3', 'f3');
   t.deepEqual(Array.from(s.getKeys('foo1', 'foo3')), ['foo1', 'foo2']);
   t.deepEqual(Array.from(s.getKeys('foo1', 'foo4')), ['foo1', 'foo2', 'foo3']);
+  t.deepEqual(Array.from(s.getKeys('', '')), ['foo', 'foo1', 'foo2', 'foo3']);
+  t.deepEqual(Array.from(s.getKeys('foo1', '')), ['foo1', 'foo2', 'foo3']);
+  t.deepEqual(Array.from(s.getKeys('', 'foo2')), ['foo', 'foo1']);
 
   s.delete('foo2');
   t.falsy(s.has('foo2'));
@@ -250,21 +253,15 @@ function duplicateKeeper(getState) {
 }
 
 test('hostStorage param guards', async t => {
-  const { kstorage, commitCrank } = buildKeeperStorageInMemory();
-  kstorage.set('foo', true);
-  t.throws(commitCrank, { message: /must be a string/ });
-  kstorage.set(true, 'foo');
-  t.throws(commitCrank, { message: /must be a string/ });
-  kstorage.has(true);
-  t.throws(commitCrank, { message: /must be a string/ });
-  kstorage.getKeys('foo', true);
-  t.throws(commitCrank, { message: /must be a string/ });
-  kstorage.getKeys(true, 'foo');
-  t.throws(commitCrank, { message: /must be a string/ });
-  kstorage.get(true);
-  t.throws(commitCrank, { message: /must be a string/ });
-  kstorage.delete(true);
-  t.throws(commitCrank, { message: /must be a string/ });
+  const { kstorage } = buildKeeperStorageInMemory();
+  const exp = { message: /true must be a string/ };
+  t.throws(() => kstorage.set('foo', true), exp);
+  t.throws(() => kstorage.set(true, 'foo'), exp);
+  t.throws(() => kstorage.has(true), exp);
+  t.throws(() => Array.from(kstorage.getKeys('foo', true)), exp);
+  t.throws(() => Array.from(kstorage.getKeys(true, 'foo')), exp);
+  t.throws(() => kstorage.get(true), exp);
+  t.throws(() => kstorage.delete(true), exp);
 });
 
 test('kernel state', async t => {