From 00098da1d9bf80565956d78fe592d78b0be9f2c1 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Thu, 7 May 2020 17:00:05 -0700 Subject: [PATCH] fix: don't retire promises that resolve to data structures containing promises --- packages/SwingSet/src/kernel/liveSlots.js | 28 +++++++++++-------- packages/SwingSet/src/kernel/vatManager.js | 24 +++++++++++----- packages/SwingSet/test/test-vpid-liveslots.js | 22 +++++++-------- 3 files changed, 44 insertions(+), 30 deletions(-) diff --git a/packages/SwingSet/src/kernel/liveSlots.js b/packages/SwingSet/src/kernel/liveSlots.js index f125ef96dfa..cfa1f99724b 100644 --- a/packages/SwingSet/src/kernel/liveSlots.js +++ b/packages/SwingSet/src/kernel/liveSlots.js @@ -393,6 +393,19 @@ function build(syscall, _state, makeRoot, forVatID) { slotToVal.delete(promiseID); } + function retirePromiseIDIfEasy(promiseID, data) { + for (const slot of data.slots) { + const { type } = parseVatSlot(slot); + if (type === 'promise') { + lsdebug( + `Unable to retire ${promiseID} because slot ${slot} is a promise`, + ); + return; + } + } + retirePromiseID(promiseID); + } + function thenResolve(promiseID) { insistVatType('promise', promiseID); return res => { @@ -406,9 +419,6 @@ function build(syscall, _state, makeRoot, forVatID) { lsdebug(` ser ${ser.body} ${JSON.stringify(ser.slots)}`); // find out what resolution category we're using const unser = JSON.parse(ser.body); - if (importedPromisesByPromiseID.has(promiseID)) { - importedPromisesByPromiseID.get(promiseID).res(res); - } if ( Object(unser) === unser && QCLASS in unser && @@ -417,12 +427,10 @@ function build(syscall, _state, makeRoot, forVatID) { const slot = ser.slots[unser.index]; insistVatType('object', slot); syscall.fulfillToPresence(promiseID, slot); - retirePromiseID(promiseID); } else { // if it resolves to data, .thens fire but kernel-queued messages are // rejected, because you can't send messages to data syscall.fulfillToData(promiseID, ser); - retirePromiseID(promiseID); } // TODO (for chip): the kernel currently notifies all subscribers of a @@ -445,6 +453,7 @@ function build(syscall, _state, makeRoot, forVatID) { if (pRec) { pRec.resolve(res); } + retirePromiseIDIfEasy(promiseID, ser); }; } @@ -453,17 +462,14 @@ function build(syscall, _state, makeRoot, forVatID) { harden(rej); lsdebug(`ls thenReject fired`, rej); const ser = m.serialize(rej); - if (importedPromisesByPromiseID.has(promiseID)) { - importedPromisesByPromiseID.get(promiseID).rej(rej); - } syscall.reject(promiseID, ser); - retirePromiseID(promiseID); // TODO (for chip): this is also a double-rejection until the // retire-promises branch lands. Delete this comment when that happens. const pRec = importedPromisesByPromiseID.get(promiseID); if (pRec) { pRec.reject(rej); } + retirePromiseIDIfEasy(promiseID, ser); }; } @@ -480,7 +486,7 @@ function build(syscall, _state, makeRoot, forVatID) { const pRec = importedPromisesByPromiseID.get(promiseID); const val = m.unserialize(data); pRec.resolve(val); - retirePromiseID(promiseID); + retirePromiseIDIfEasy(promiseID, data); } function notifyFulfillToPresence(promiseID, slot) { @@ -514,7 +520,7 @@ function build(syscall, _state, makeRoot, forVatID) { const pRec = importedPromisesByPromiseID.get(promiseID); const val = m.unserialize(data); pRec.reject(val); - retirePromiseID(promiseID); + retirePromiseIDIfEasy(promiseID, data); } // here we finally invoke the vat code, and get back the root object diff --git a/packages/SwingSet/src/kernel/vatManager.js b/packages/SwingSet/src/kernel/vatManager.js index e2580e348d0..920cc0f1d99 100644 --- a/packages/SwingSet/src/kernel/vatManager.js +++ b/packages/SwingSet/src/kernel/vatManager.js @@ -107,6 +107,19 @@ export default function makeVatManager( } } + function deleteCListEntryIfEasy(kpid, vpid, kernelData) { + for (const slot of kernelData.slots) { + const { type } = parseKernelSlot(slot); + if (type === 'promise') { + kdebug( + `Unable to delete clist entry ${kpid}<=>${vpid} because slot ${slot} is a promise`, + ); + return; + } + } + vatKeeper.deleteCListEntry(kpid, vpid); + } + // syscall handlers: these are wrapped by the 'syscall' object and made // available to userspace @@ -182,7 +195,7 @@ export default function makeVatManager( data.body } ${JSON.stringify(data.slots)}/${JSON.stringify(kernelSlots)}`, ); - vatKeeper.deleteCListEntry(kpid, promiseID); + deleteCListEntryIfEasy(kpid, promiseID, kernelData); syscallManager.fulfillToData(vatID, kpid, kernelData); } @@ -197,7 +210,7 @@ export default function makeVatManager( data.body } ${JSON.stringify(data.slots)}/${JSON.stringify(kernelSlots)}`, ); - vatKeeper.deleteCListEntry(kpid, promiseID); + deleteCListEntryIfEasy(kpid, promiseID, kernelData); syscallManager.reject(vatID, kpid, kernelData); } @@ -378,7 +391,6 @@ export default function makeVatManager( ['notifyFulfillToPresence', vpid, slot], `vat[${vatID}].promise[${vpid}] fulfillToPresence failed`, ); - vatKeeper.deleteCListEntry(kpid, vpid); } else if (kp.state === 'redirected') { throw new Error('not implemented yet'); } else if (kp.state === 'fulfilledToData') { @@ -387,24 +399,22 @@ export default function makeVatManager( ...kp.data, slots: kp.data.slots.map(slot => mapKernelSlotToVatSlot(slot)), }); - vatKeeper.deleteCListEntry(kpid, vpid); + deleteCListEntryIfEasy(kpid, vpid, kp.data); await doProcess( ['notifyFulfillToData', vpid, vatData], `vat[${vatID}].promise[${vpid}] fulfillToData failed`, ); - vatKeeper.deleteCListEntry(kpid, vpid); } else if (kp.state === 'rejected') { const vpid = mapKernelSlotToVatSlot(kpid); const vatData = harden({ ...kp.data, slots: kp.data.slots.map(slot => mapKernelSlotToVatSlot(slot)), }); - vatKeeper.deleteCListEntry(kpid, vpid); + deleteCListEntryIfEasy(kpid, vpid, kp.data); await doProcess( ['notifyReject', vpid, vatData], `vat[${vatID}].promise[${vpid}] reject failed`, ); - vatKeeper.deleteCListEntry(kpid, vpid); } else { throw new Error(`unknown kernelPromise state '${kp.state}'`); } diff --git a/packages/SwingSet/test/test-vpid-liveslots.js b/packages/SwingSet/test/test-vpid-liveslots.js index c6372e1e357..8924668b8c6 100644 --- a/packages/SwingSet/test/test-vpid-liveslots.js +++ b/packages/SwingSet/test/test-vpid-liveslots.js @@ -243,11 +243,10 @@ async function doVatResolveCase1(t, mode) { type: 'send', targetSlot: target1, method: 'two', - args: capargs([slot0arg], [expectedP3]), - resultSlot: expectedP4, + args: capargs([slot0arg], [expectedP1]), + resultSlot: expectedP3, }); - t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP4 }); - t.deepEqual(log.shift(), resolutionOf(expectedP3, mode, target2)); + t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP3 }); t.deepEqual(log, []); t.end(); @@ -653,28 +652,27 @@ async function doVatResolveCase4(t, mode) { await endOfCrank(); const expectedP4 = nextP(); - const expectedP5 = nextP(); t.deepEqual(log.shift(), { type: 'send', targetSlot: target1, method: 'three', - args: capargs([slot0arg], [expectedP4]), - resultSlot: expectedP5, + args: capargs([slot0arg], [p1]), + resultSlot: expectedP4, }); - t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP5 }); + t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP4 }); if (mode === 'presence') { const expectedP5 = nextP(); t.deepEqual(log.shift(), { type: 'send', - targetSlot: target2, + targetSlot: target2, // this depends on #823 being fixed method: 'four', args: capargs([], []), - resultSlot: expectedP6, + resultSlot: expectedP5, }); - t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP6 }); + t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP5 }); } - t.deepEqual(log.shift(), resolutionOf(expectedP4, mode, target2)); + // if p1 rejects or resolves to data, the kernel never hears about four() t.deepEqual(log, []); t.end();