fix: correct bugs due to weird & mistaken buildRootObject usage

Agoric · Mar 21, 2022 · 415529e · 415529e
1 parent e4358bd
commit 415529e
Show file tree

Hide file tree

Showing 11 changed files with 28 additions and 31 deletions.
diff --git a/packages/SwingSet/src/liveslots/collectionManager.js b/packages/SwingSet/src/liveslots/collectionManager.js
@@ -665,15 +665,18 @@ export function makeCollectionManager(
   }
 
   function provideBaggage() {
-    const baggageID = syscall.vatstoreGet('baggageID');
+    let baggageID = syscall.vatstoreGet('baggageID');
     if (baggageID) {
       return convertSlotToVal(baggageID);
     } else {
       const baggage = makeScalarBigMapStore('baggage', {
         keySchema: M.string(),
         durable: true,
       });
-      syscall.vatstoreSet('baggageID', convertValToSlot(baggage));
+      baggageID = convertValToSlot(baggage);
+      syscall.vatstoreSet('baggageID', baggageID);
+      // artificially increment the baggage's refcount so it never gets GC'd
+      vrm.addReachableVref(baggageID);
       return baggage;
     }
   }

diff --git a/packages/SwingSet/src/liveslots/liveslots.js b/packages/SwingSet/src/liveslots/liveslots.js
@@ -1165,6 +1165,7 @@ function build(
     }
 
     const vatParameters = m.unserialize(vatParametersCapData);
+    baggage = collectionManager.provideBaggage();
 
     // Below this point, user-provided code might crash or overrun a meter, so
     // any prior-to-user-code setup that can be done without reference to the
@@ -1184,7 +1185,6 @@ function build(
     );
 
     // here we finally invoke the vat code, and get back the root object
-    baggage = collectionManager.provideBaggage();
     const rootObject = buildRootObject(
       harden(vpow),
       harden(vatParameters),

diff --git a/packages/SwingSet/test/stores/test-storeGC.js b/packages/SwingSet/test/stores/test-storeGC.js
@@ -28,12 +28,7 @@ let aWeakSetStore;
 const mainHolderIdx = 2;
 const mainHeldIdx = 3;
 
-// eslint's belief that the following var is unused is erroneous
-// eslint-disable-next-line no-unused-vars
-let preventBaggageGC;
-
-function buildRootObject(vatPowers, vatParameters, baggage) {
-  preventBaggageGC = baggage;
+function buildRootObject(vatPowers) {
   const { VatData } = vatPowers;
   const {
     makeScalarBigMapStore,
@@ -243,6 +238,8 @@ function validateCreateBaggage(v, idx) {
   validate(v, matchVatstoreSet(`vc.${idx}.|schemata`, baggageSchema));
   validate(v, matchVatstoreSet(`vc.${idx}.|label`, 'baggage'));
   validate(v, matchVatstoreSet('baggageID', 'o+5/1'));
+  validate(v, matchVatstoreGet('vom.rc.o+5/1', NONE));
+  validate(v, matchVatstoreSet('vom.rc.o+5/1', '1'));
 }
 
 function validateCreate(v, idx, isWeak = false) {

diff --git a/packages/SwingSet/test/test-baggage.js b/packages/SwingSet/test/test-baggage.js
@@ -41,6 +41,8 @@ function validateCreateBaggage(v, idx) {
   validate(v, matchVatstoreSet(`vc.${idx}.|schemata`, baggageSchema));
   validate(v, matchVatstoreSet(`vc.${idx}.|label`, 'baggage'));
   validate(v, matchVatstoreSet('baggageID', 'o+5/1'));
+  validate(v, matchVatstoreGet('vom.rc.o+5/1', NONE));
+  validate(v, matchVatstoreSet('vom.rc.o+5/1', '1'));
 }
 
 function validateSetup(v) {

diff --git a/packages/SwingSet/test/vat-admin/terminate/test-terminate.js b/packages/SwingSet/test/vat-admin/terminate/test-terminate.js
@@ -213,7 +213,7 @@ test('dead vat state removed', async t => {
   const kvStore = hostStorage.kvStore;
   t.is(kvStore.get('vat.dynamicIDs'), '["v6"]');
   t.is(kvStore.get('ko26.owner'), 'v6');
-  t.is(Array.from(kvStore.getKeys('v6.', 'v6/')).length, 17);
+  t.is(Array.from(kvStore.getKeys('v6.', 'v6/')).length, 18);
 
   controller.queueToVatRoot('bootstrap', 'phase2', capargs([]));
   await controller.run();

diff --git a/packages/SwingSet/test/virtualObjects/test-representatives.js b/packages/SwingSet/test/virtualObjects/test-representatives.js
@@ -133,13 +133,13 @@ test.serial('exercise cache', async t => {
     await doSimple('holdThing', what);
   }
   function dataKey(num) {
-    return `v1.vs.vom.o+1/${num}`;
+    return `v1.vs.vom.o+9/${num}`;
   }
   function esKey(num) {
-    return `v1.vs.vom.es.o+1/${num}`;
+    return `v1.vs.vom.es.o+9/${num}`;
   }
   function rcKey(num) {
-    return `v1.vs.vom.rc.o+1/${num}`;
+    return `v1.vs.vom.rc.o+9/${num}`;
   }
   function thingVal(name) {
     return JSON.stringify({
@@ -394,6 +394,7 @@ test('virtual object gc', async t => {
     'v1.vs.vom.o+9/3': '{"label":{"body":"\\"thing #3\\"","slots":[]}}',
     'v1.vs.vom.o+9/8': '{"label":{"body":"\\"thing #8\\"","slots":[]}}',
     'v1.vs.vom.o+9/9': '{"label":{"body":"\\"thing #9\\"","slots":[]}}',
+    'v1.vs.vom.rc.o+5/1': '1',
   });
 });
 

diff --git a/packages/SwingSet/test/virtualObjects/test-virtualObjectGC.js b/packages/SwingSet/test/virtualObjects/test-virtualObjectGC.js
@@ -137,10 +137,6 @@ import { capargs } from '../util.js';
 //   lERV -overwr-> lERv
 //   LERV -overwr-> LERv
 
-// eslint's belief that the following var is unused is erroneous
-// eslint-disable-next-line no-unused-vars
-let preventBaggageGC;
-
 let aWeakMap;
 let aWeakSet;
 
@@ -188,8 +184,7 @@ const cacheDisplacerVref = thingVref(false, 1);
 const fCacheDisplacerVref = thingVref(true, 1);
 const virtualHolderVref = `${holderKindID}/1`;
 
-function buildRootObject(vatPowers, vatParameters, baggage) {
-  preventBaggageGC = baggage;
+function buildRootObject(vatPowers) {
   const { VatData, WeakMap, WeakSet } = vatPowers;
 
   const { defineKind } = VatData;
@@ -459,6 +454,8 @@ function validateCreateBaggage(v, idx) {
   validate(v, matchVatstoreSet(`vc.${idx}.|schemata`, baggageSchema));
   validate(v, matchVatstoreSet(`vc.${idx}.|label`, 'baggage'));
   validate(v, matchVatstoreSet('baggageID', 'o+5/1'));
+  validate(v, matchVatstoreGet(rcKey('o+5/1'), NONE));
+  validate(v, matchVatstoreSet(rcKey('o+5/1'), '1'));
 }
 
 function validateSetup(v) {

diff --git a/packages/vats/src/core/boot.js b/packages/vats/src/core/boot.js
@@ -50,9 +50,10 @@ const roleToGovernanceActions = harden({
  *   bootstrapManifest?: Record<string, Record<string, unknown>>,
  *   governanceActions?: boolean,
  * }} vatParameters
- * @param {typeof console.log} [log]
  */
-const buildRootObject = (vatPowers, vatParameters, log = console.info) => {
+const buildRootObject = (vatPowers, vatParameters) => {
+  // @ts-expect-error no TS defs for rickety test scaffolding
+  const log = vatPowers.logger || console.info;
   const { produce, consume } = makePromiseSpace(log);
   const { agoricNames, spaces } = makeAgoricNamesAccess(log);
   produce.agoricNames.resolve(agoricNames);

diff --git a/packages/vats/test/test-boot.js b/packages/vats/test/test-boot.js
@@ -88,9 +88,8 @@ const testRole = (ROLE, governanceActions) => {
   test(`test manifest permits: ${ROLE} gov: ${governanceActions}`, async t => {
     const root = buildRootObject(
       // @ts-expect-error Device<T> is a little goofy
-      { D: d => d },
+      { D: d => d, logger: t.log },
       { argv: argvByRole[ROLE], governanceActions },
-      t.log,
     );
 
     const fakeVatAdmin = makeFakeVatAdmin(() => {}).admin;

diff --git a/packages/zoe/src/contractFacet/vatRoot.js b/packages/zoe/src/contractFacet/vatRoot.js
@@ -16,16 +16,16 @@ import { makeZCFZygote } from './zcfZygote.js';
 
 /**
  * @param {VatPowers} powers
- * @param {undefined} _params
- * @param {Function | undefined} testJigSetter
  * @returns {{ executeContract: ExecuteContract}}
  */
-export function buildRootObject(powers, _params, testJigSetter = undefined) {
+export function buildRootObject(powers) {
   // Currently, there is only one function, `executeContract` called
   // by the Zoe Service. However, when there is kernel support for
   // zygote vats (essentially freezing and then creating copies of
   // vats), `makeZCFZygote`, `zcfZygote.evaluateContract` and
   // `zcfZygote.startContract` should exposed separately.
+  // @ts-expect-error no TS defs for rickety test scaffolding
+  const { testJigSetter } = powers;
 
   /** @type {ExecuteContract} */
   const executeContract = (

diff --git a/packages/zoe/tools/fakeVatAdmin.js b/packages/zoe/tools/fakeVatAdmin.js
@@ -48,6 +48,7 @@ function makeFakeVatAdmin(testContextSetter = undefined, makeRemote = x => x) {
     D: bundleCap => ({
       getBundle: () => bundleCapToBundle.get(bundleCap),
     }),
+    testJigSetter: testContextSetter,
   };
 
   // This is explicitly intended to be mutable so that
@@ -70,11 +71,7 @@ function makeFakeVatAdmin(testContextSetter = undefined, makeRemote = x => x) {
       return Promise.resolve(
         harden({
           root: makeRemote(
-            E(evalContractBundle(bundle)).buildRootObject(
-              fakeVatPowers,
-              undefined,
-              testContextSetter,
-            ),
+            E(evalContractBundle(bundle)).buildRootObject(fakeVatPowers),
           ),
           adminNode: Far('adminNode', {
             done: () => {