diff --git a/packages/SwingSet/src/kernel/id.js b/packages/SwingSet/src/kernel/id.js index bd5d777e1e7..bd76555b6ab 100644 --- a/packages/SwingSet/src/kernel/id.js +++ b/packages/SwingSet/src/kernel/id.js @@ -26,12 +26,10 @@ export function insistVatID(s) { if (s !== `${s}`) { throw new Error(`not a string`); } - if (s !== 'none') { - if (!s.startsWith(`v`)) { - throw new Error(`does not start with 'v'`); - } - Nat(Number(s.slice(1))); + if (!s.startsWith(`v`)) { + throw new Error(`does not start with 'v'`); } + Nat(Number(s.slice(1))); } catch (e) { throw new Error(`'${s} is not a 'vNN'-style VatID: ${e.message}`); } diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js index 324e611eb06..c632fb16659 100644 --- a/packages/SwingSet/src/kernel/kernel.js +++ b/packages/SwingSet/src/kernel/kernel.js @@ -40,7 +40,6 @@ function makeError(s) { } const VAT_TERMINATION_ERROR = makeError('vat terminated'); -const UNKNOWN_VAT_ERROR = makeError('unknown vat'); export default function buildKernel(kernelEndowments, kernelOptions = {}) { const { @@ -312,8 +311,8 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) { const vat = ephemeral.vats.get(vatID); kernelKeeper.incStat('dispatches'); kernelKeeper.incStat('dispatchDeliver'); - if (!vat || vat.dead || vatID === 'none') { - resolveToError(msg.result, UNKNOWN_VAT_ERROR); + if (!vat) { + resolveToError(msg.result, VAT_TERMINATION_ERROR); } else { const kd = harden(['message', target, msg]); const vd = vat.translators.kernelDeliveryToVatDelivery(kd); @@ -326,8 +325,11 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) { const { type } = parseKernelSlot(target); if (type === 'object') { const vatID = kernelKeeper.ownerOfKernelObject(target); - vatID === 'none' || insistVatID(vatID); - await deliverToVat(vatID, target, msg); + if (vatID) { + await deliverToVat(vatID, target, msg); + } else { + resolveToError(msg.result, VAT_TERMINATION_ERROR); + } } else if (type === 'promise') { const kp = kernelKeeper.getKernelPromise(target); if (kp.state === 'fulfilledToPresence') { @@ -351,11 +353,15 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) { kernelKeeper.addMessageToPromiseQueue(target, msg); } else { insistVatID(kp.decider); - const vat = ephemeral.vats.get(kp.decider); - if (vat.enablePipelining) { - await deliverToVat(kp.decider, target, msg); + const deciderVat = ephemeral.vats.get(kp.decider); + if (deciderVat) { + if (deciderVat.enablePipelining) { + await deliverToVat(kp.decider, target, msg); + } else { + kernelKeeper.addMessageToPromiseQueue(target, msg); + } } else { - kernelKeeper.addMessageToPromiseQueue(target, msg); + resolveToError(msg.result, VAT_TERMINATION_ERROR); } } } else { @@ -385,7 +391,7 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) { insistKernelType('promise', kpid); const vat = ephemeral.vats.get(vatID); kernelKeeper.incStat('dispatches'); - if (!vat || vat.dead) { + if (!vat) { kdebug(`dropping notify of ${kpid} to ${vatID} because vat is dead`); } else { const p = kernelKeeper.getKernelPromise(kpid); @@ -628,7 +634,7 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) { // the VatManager+VatWorker will see the error case, but liveslots will // not function vatSyscallHandler(vatSyscallObject) { - if (ephemeral.vats.get(vatID).dead) { + if (!ephemeral.vats.get(vatID)) { // This is a safety check -- this case should never happen unless the // vatManager is somehow confused. console.error(`vatSyscallHandler invoked on dead vat ${vatID}`); @@ -712,7 +718,7 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) { function removeVatManager(vatID) { const old = ephemeral.vats.get(vatID); - ephemeral.vats.set(vatID, harden({ dead: true })); + ephemeral.vats.delete(vatID); old.notifyTermination(null); return old.manager.shutdown(); } @@ -903,7 +909,7 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) { for (const vatID of ephemeral.vats.keys()) { logStartup(`Replaying transcript of vatID ${vatID}`); const vat = ephemeral.vats.get(vatID); - if (vat.dead) { + if (!vat) { logStartup(`skipping reload of dead vat ${vatID}`); } else { // eslint-disable-next-line no-await-in-loop diff --git a/packages/SwingSet/src/kernel/state/kernelKeeper.js b/packages/SwingSet/src/kernel/state/kernelKeeper.js index f56e77e6636..bbe7710a252 100644 --- a/packages/SwingSet/src/kernel/state/kernelKeeper.js +++ b/packages/SwingSet/src/kernel/state/kernelKeeper.js @@ -49,7 +49,6 @@ const enableKernelPromiseGC = true; // v$NN.c.$vatSlot = $kernelSlot = ko$NN/kp$NN/kd$NN // v$NN.t.$NN = JSON(transcript entry) // v$NN.t.nextID = $NN -// v$NN.dead = missing | true // d$NN.o.nextID = $NN // d$NN.c.$kernelSlot = $deviceSlot = o-$NN/d+$NN/d-$NN @@ -251,8 +250,10 @@ export default function makeKernelKeeper(storage) { function ownerOfKernelObject(kernelSlot) { insistKernelType('object', kernelSlot); - const owner = getRequired(`${kernelSlot}.owner`); - insistVatID(owner); + const owner = storage.get(`${kernelSlot}.owner`); + if (owner) { + insistVatID(owner); + } return owner; } @@ -439,17 +440,37 @@ export default function makeKernelKeeper(storage) { const kpPrefix = `${vatID}.c.p`; const kernelPromisesToReject = []; for (const k of storage.getKeys(`${vatID}.`, `${vatID}/`)) { - // The store semantics ensure this iteration is lexicographic. Any - // changes to the creation of the list of promises need to preserve this - // in order to preserve determinism. + // The current store semantics ensure this iteration is lexicographic. + // Any changes to the creation of the list of promises to be rejected (and + // thus to the order in which they *get* rejected) need to preserve this + // ordering in order to preserve determinism. TODO: we would like to + // shift to a different deterministic ordering scheme that is less fragile + // in the face of potential changes in the nature of the database being + // used. if (k.startsWith(koPrefix)) { + // The void for an object exported by a vat will always be of the form + // `o+NN`. The '+' means that the vat exported the object (rather than + // importing it) and therefor the object is owned by (i.e., within) the + // vat. The corresponding void->koid c-list entry will thus always + // begin with `vMM.c.o+`. In addition to deleting the c-list entry, we + // must also delete the corresponding kernel owner entry for the object, + // since the object will no longer be accessible. const koid = storage.get(k); const ownerKey = `${koid}.owner`; const ownerVat = storage.get(ownerKey); if (ownerVat === vatID) { - storage.set(ownerKey, 'none'); + storage.delete(ownerKey); } } else if (k.startsWith(kpPrefix)) { + // The vpid for a promise imported or exported by a vat (and thus + // potentially a promise for which the vat *might* be the decider) will + // always be of the form `p+NN` or `p-NN`. The corresponding vpid->kpid + // c-list entry will thus always begin with `vMM.c.p`. Decider-ship is + // independent of whether the promise was imported or exported, so we + // have to look up the corresponding kernel promise table entry to see + // whether the vat is the decider or not. If it is, we add the promise + // to the list of promises that must be rejected because the dead vat + // will never be able to act upon them. const kpid = storage.get(k); const p = getKernelPromise(kpid); if (p.state === 'unresolved' && p.decider === vatID) { @@ -459,9 +480,10 @@ export default function makeKernelKeeper(storage) { storage.delete(k); } // TODO: deleting entries from the dynamic vat IDs list requires a linear - // scan; this collection ought to be represented in a way that makes it - // efficient to remove an entry from it, though this should be OK as long as - // we keep the list short. + // scan of the list; arguably this collection ought to be represented in a + // different way that makes it efficient to remove an entry from it, though + // for the time being the linear list should be OK enough as long as we keep + // the list short. const DYNAMIC_IDS_KEY = 'vat.dynamicIDs'; const oldDynamicVatIDs = JSON.parse(getRequired(DYNAMIC_IDS_KEY)); const newDynamicVatIDs = oldDynamicVatIDs.filter(v => v !== vatID); diff --git a/packages/SwingSet/test/vat-admin/terminate/test-terminate.js b/packages/SwingSet/test/vat-admin/terminate/test-terminate.js index 4cf3a4e2d53..756acc70f5c 100644 --- a/packages/SwingSet/test/vat-admin/terminate/test-terminate.js +++ b/packages/SwingSet/test/vat-admin/terminate/test-terminate.js @@ -43,7 +43,7 @@ test('terminate', async t => { 'GOT QUERY 3', 'foreverP.catch vat terminated', 'query3P.catch vat terminated', - 'foo4P.catch unknown vat', + 'foo4P.catch vat terminated', 'afterForeverP.catch vat terminated', 'done', ]); @@ -86,7 +86,7 @@ test('dispatches to the dead do not harm kernel', async t => { t.deepEqual(c2.dump().log, [ 'b: p1b = I so resolve', 'b: p2b fails vat terminated', - 'm: live 2 failed: unknown vat', + 'm: live 2 failed: vat terminated', ]); } }); @@ -139,6 +139,6 @@ test('dead vat state removed', async t => { controller.queueToVatExport('bootstrap', 'o+0', 'phase2', capargs([])); await controller.run(); t.is(storage.get('vat.dynamicIDs'), '[]'); - t.is(storage.get('ko26.owner'), 'none'); + t.is(storage.get('ko26.owner'), undefined); t.is(Array.from(storage.getKeys('v6.', 'v6/')).length, 0); });