Skip to content

Commit

Permalink
fix: clean up review issues
Browse files Browse the repository at this point in the history
  • Loading branch information
@FUDCo
FUDCo committed Aug 18, 2020
1 parent ef43070 commit 9ad3b79
Show file tree
Hide file tree
Showing 11 changed files with 103 additions and 97 deletions.
93 changes: 53 additions & 40 deletions packages/SwingSet/src/kernel/kernel.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ function abbreviateReviver(_, arg) {
return arg;
}

function makeError(s) {
// TODO: create a @qclass=error, once we define those or maybe replicate
// whatever happens with {}.foo() or 3.foo() etc: "TypeError: {}.foo is not a
// function"
return harden({ body: JSON.stringify(s), slots: [] });
}

export default function buildKernel(kernelEndowments) {
const {
waitUntilQuiescent,
Expand Down Expand Up @@ -213,27 +220,6 @@ export default function buildKernel(kernelEndowments) {
return resultRead;
}

function getResolveablePromise(kpid, resolvingVatID) {
insistKernelType('promise', kpid);
if (resolvingVatID) {
insistVatID(resolvingVatID);
}
const p = kernelKeeper.getKernelPromise(kpid);
assert(p.state === 'unresolved', details`${kpid} was already resolved`);
assert(
!p.decider || p.decider === resolvingVatID,
details`${kpid} is decided by ${p.decider}, not ${resolvingVatID}`,
);
return p;
}

function makeError(s) {
// TODO: create a @qclass=error, once we define those
// or maybe replicate whatever happens with {}.foo()
// or 3.foo() etc: "TypeError: {}.foo is not a function"
return harden({ body: JSON.stringify(s), slots: [] });
}

function notify(vatID, kpid) {
const m = harden({ type: 'notify', vatID, kpid });
kernelKeeper.incrementRefCount(kpid, `enq|notify`);
Expand All @@ -260,17 +246,18 @@ export default function buildKernel(kernelEndowments) {
}
}

function deliverToError(kpid, errorData, vatToNotNotify) {
function deliverToError(kpid, errorData, expectedDecider) {
insistCapData(errorData);
const p = getResolveablePromise(kpid, vatToNotNotify);
const p = kernelKeeper.getResolveablePromise(kpid, expectedDecider);
const { subscribers, queue } = p;
let idx = 0;
for (const dataSlot of errorData.slots) {
kernelKeeper.incrementRefCount(dataSlot, `reject|s${idx}`);
idx += 1;
}
kernelKeeper.rejectKernelPromise(kpid, errorData);
notifySubscribersAndQueue(kpid, vatToNotNotify, subscribers, queue);
// we don't notify the decider; they already know
notifySubscribersAndQueue(kpid, expectedDecider, subscribers, queue);
if (pendingMessageResults.has(kpid)) {
notePendingMessageResolution(kpid, 'rejected', errorData);
}
Expand All @@ -283,12 +270,18 @@ export default function buildKernel(kernelEndowments) {
kernelKeeper.incStat('dispatches');
kernelKeeper.incStat('dispatchDeliver');
if (vat.dead) {
deliverToError(msg.result, makeError('vat is dead'), vatID);
deliverToError(msg.result, makeError('vat is dead'));
} else {
const kd = harden(['message', target, msg]);
const vd = vat.translators.kernelDeliveryToVatDelivery(kd);
try {
await vat.manager.deliver(vd);
/* // TODO: eventually this will be as follows, once deliver can return a delivery status
const deliveryResult = await vat.manager.deliver(vd);
if (deliveryResult === death) {
vat.notifyTermination(deliveryResult.causeOfDeath);
}
*/
} catch (e) {
// log so we get a stack trace
console.error(`error in kernel.deliver:`, e);
Expand Down Expand Up @@ -371,6 +364,12 @@ export default function buildKernel(kernelEndowments) {
const vd = vat.translators.kernelDeliveryToVatDelivery(kd);
try {
await vat.manager.deliver(vd);
/* // TODO: eventually this will be as follows, once deliver can return a delivery status
const deliveryResult = await vat.manager.deliver(vd);
if (deliveryResult === death) {
vat.notifyTermination(deliveryResult.causeOfDeath);
}
*/
} catch (e) {
// log so we get a stack trace
console.error(`error in kernel.processNotify:`, e);
Expand Down Expand Up @@ -616,7 +615,11 @@ export default function buildKernel(kernelEndowments) {
manager.deliver && manager.setVatSyscallHandler,
`manager lacks .deliver, isPromise=${manager instanceof Promise}`,
);
const { enablePipelining = false } = managerOptions;

const {
enablePipelining = false,
notifyTermination = () => {},
} = managerOptions;
// This should create the vatKeeper. Other users get it from the
// kernelKeeper, so we don't need a reference ourselves.
kernelKeeper.allocateVatKeeperIfNeeded(vatID);
Expand All @@ -627,6 +630,7 @@ export default function buildKernel(kernelEndowments) {
harden({
translators,
manager,
notifyTermination,
enablePipelining: Boolean(enablePipelining),
}),
);
Expand All @@ -639,6 +643,8 @@ export default function buildKernel(kernelEndowments) {
// not
function vatSyscallHandler(vatSyscallObject) {
if (ephemeral.vats.get(vatID).dead) {
// This is a safety check -- this case should never happen unless the
// vatManager is somehow confused.
console.error(`vatSyscallHandler invoked on dead vat ${vatID}`);
return harden(['error', 'vat is dead']);
}
Expand Down Expand Up @@ -681,10 +687,11 @@ export default function buildKernel(kernelEndowments) {
manager.setVatSyscallHandler(vatSyscallHandler);
}

async function removeVatManager(vatID) {
function removeVatManager(vatID) {
const old = ephemeral.vats.get(vatID);
ephemeral.vats.set(vatID, harden({ dead: true }));
old.manager.shutdown();
old.notifyTermination(null);
return old.manager.shutdown();
}

const knownBundles = new Map();
Expand Down Expand Up @@ -799,11 +806,13 @@ export default function buildKernel(kernelEndowments) {

function terminateVat(vatID) {
const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(vatID);
vatKeeper.markAsDead();
removeVatManager(vatID).then(
() => console.debug(`terminated vat ${vatID}`),
e => console.error(`problem terminating vat ${vatID}`, e),
);
if (!vatKeeper.isDead()) {
vatKeeper.markAsDead();
removeVatManager(vatID).then(
() => kdebug(`terminated vat ${vatID}`),
e => console.error(`problem terminating vat ${vatID}`, e),
);
}
}

if (vatAdminDeviceBundle) {
Expand Down Expand Up @@ -859,13 +868,17 @@ export default function buildKernel(kernelEndowments) {
for (const vatID of ephemeral.vats.keys()) {
console.debug(`Replaying transcript of vatID ${vatID}`);
const vat = ephemeral.vats.get(vatID);
// eslint-disable-next-line no-await-in-loop
await vat.manager.replayTranscript();
console.debug(`finished replaying vatID ${vatID} transcript `);
const newLength = kernelKeeper.getRunQueueLength();
if (newLength !== oldLength) {
console.log(`SPURIOUS RUNQUEUE`, kernelKeeper.dump().runQueue);
throw Error(`replay ${vatID} added spurious run-queue entries`);
if (vat.dead) {
console.debug(`skipping reload of dead vat ${vatID}`);
} else {
// eslint-disable-next-line no-await-in-loop
await vat.manager.replayTranscript();
console.debug(`finished replaying vatID ${vatID} transcript `);
const newLength = kernelKeeper.getRunQueueLength();
if (newLength !== oldLength) {
console.log(`SPURIOUS RUNQUEUE`, kernelKeeper.dump().runQueue);
throw Error(`replay ${vatID} added spurious run-queue entries`);
}
}
}
kernelKeeper.loadStats();
Expand Down
18 changes: 3 additions & 15 deletions packages/SwingSet/src/kernel/kernelSyscall.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* global harden */
import { assert, details } from '@agoric/assert';
import { assert } from '@agoric/assert';
import { insistKernelType, parseKernelSlot } from './parseKernelSlots';
import { insistMessage } from '../message';
import { insistCapData } from '../capdata';
Expand Down Expand Up @@ -70,25 +70,13 @@ export function makeKernelSyscallHandler(tools) {
return OKNULL;
}

function getResolveablePromise(kpid, resolvingVatID) {
insistKernelType('promise', kpid);
insistVatID(resolvingVatID);
const p = kernelKeeper.getKernelPromise(kpid);
assert(p.state === 'unresolved', details`${kpid} was already resolved`);
assert(
p.decider === resolvingVatID,
details`${kpid} is decided by ${p.decider}, not ${resolvingVatID}`,
);
return p;
}

function fulfillToPresence(vatID, kpid, targetSlot) {
insistVatID(vatID);
insistKernelType('promise', kpid);
insistKernelType('object', targetSlot);
kernelKeeper.incStat('syscalls');
kernelKeeper.incStat('syscallFulfillToPresence');
const p = getResolveablePromise(kpid, vatID);
const p = kernelKeeper.getResolveablePromise(kpid, vatID);
const { subscribers, queue } = p;
kernelKeeper.fulfillKernelPromiseToPresence(kpid, targetSlot);
notifySubscribersAndQueue(kpid, vatID, subscribers, queue);
Expand All @@ -114,7 +102,7 @@ export function makeKernelSyscallHandler(tools) {
insistCapData(data);
kernelKeeper.incStat('syscalls');
kernelKeeper.incStat('syscallFulfillToData');
const p = getResolveablePromise(kpid, vatID);
const p = kernelKeeper.getResolveablePromise(kpid, vatID);
const { subscribers, queue } = p;
let idx = 0;
for (const dataSlot of data.slots) {
Expand Down
22 changes: 22 additions & 0 deletions packages/SwingSet/src/kernel/state/kernelKeeper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,27 @@ export default function makeKernelKeeper(storage) {
return harden(p);
}

function getResolveablePromise(kpid, expectedDecider) {
insistKernelType('promise', kpid);
if (expectedDecider) {
insistVatID(expectedDecider);
}
const p = getKernelPromise(kpid);
assert(p.state === 'unresolved', details`${kpid} was already resolved`);
if (expectedDecider) {
assert(
p.decider === expectedDecider,
details`${kpid} is decided by ${p.decider}, not ${expectedDecider}`,
);
} else {
assert(
!p.decider,
details`${kpid} is decided by ${p.decider}, not the kernel`,
);
}
return p;
}

function hasKernelPromise(kernelSlot) {
insistKernelType('promise', kernelSlot);
return storage.has(`${kernelSlot}.state`);
Expand Down Expand Up @@ -767,6 +788,7 @@ export default function makeKernelKeeper(storage) {
addKernelPromise,
addKernelPromiseForVat,
getKernelPromise,
getResolveablePromise,
hasKernelPromise,
fulfillKernelPromiseToPresence,
fulfillKernelPromiseToData,
Expand Down
6 changes: 1 addition & 5 deletions packages/SwingSet/src/kernel/state/vatKeeper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,7 @@ export function makeVatKeeper(
}

function markAsDead() {
const deathKey = `${vatID}.dead`;
if (storage.has(deathKey)) {
throw Error(`vat ${vatID} is already dead`);
}
storage.set(deathKey, true);
storage.set(`${vatID}.dead`, true);
}

function isDead() {
Expand Down
16 changes: 7 additions & 9 deletions packages/SwingSet/src/kernel/vatManager/deliver.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,17 +116,15 @@ export function makeDeliver(tools, dispatch) {
}

async function replayTranscript() {
if (!vatKeeper.isDead()) {
transcriptManager.startReplay();
for (const t of vatKeeper.getTranscript()) {
transcriptManager.checkReplayError();
transcriptManager.startReplayDelivery(t.syscalls);
// eslint-disable-next-line no-await-in-loop
await doProcess(t.d, null);
}
transcriptManager.startReplay();
for (const t of vatKeeper.getTranscript()) {
transcriptManager.checkReplayError();
transcriptManager.finishReplay();
transcriptManager.startReplayDelivery(t.syscalls);
// eslint-disable-next-line no-await-in-loop
await doProcess(t.d, null);
}
transcriptManager.checkReplayError();
transcriptManager.finishReplay();
}

return harden({ deliver, replayTranscript });
Expand Down
3 changes: 2 additions & 1 deletion packages/SwingSet/src/kernel/vatManager/localVatManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export function makeLocalVatManagerFactory(tools) {
dispatch,
);

function shutdown() {
async function shutdown() {
// local workers don't need anything special to shut down between turns
}

Expand All @@ -65,6 +65,7 @@ export function makeLocalVatManagerFactory(tools) {
setVatSyscallHandler,
deliver,
shutdown,
notifyTermination,
});
return manager;
}
Expand Down
9 changes: 4 additions & 5 deletions packages/SwingSet/test/basedir-terminate/bootstrap.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@ export function buildRootObject() {
return harden({
async bootstrap(vats, devices) {
const vatMaker = E(vats.vatAdmin).createVatAdminService(devices.vatAdmin);
const vat = await E(vatMaker).createVatByName('dude');
const before = await E(vat.root).dude();
console.log(`success result ${before}`);
E(vat.adminNode).terminate();
const dude = await E(vatMaker).createVatByName('dude');
await E(dude.root).dude();
E(dude.adminNode).terminate();
try {
return await E(vat.root).dude();
return await E(dude.root).dude();
} catch (e) {
return `${e}`;
}
Expand Down
26 changes: 10 additions & 16 deletions packages/SwingSet/test/vat-admin/terminate/bootstrap-terminate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ export function buildRootObject(vatPowers) {
err => testLog(`foo4P.catch ${err}`),
);
// then we try to kill the vat again, which should be idempotent
// CHIP TODO PHASE1: uncomment, make controlFacet.terminate idempotent
// E(dude.adminNode).terminate();
E(dude.adminNode).terminate();

// the run-queue should now look like:
// [dude.elsewhere(3), adminNode.terminate, dude.foo(4), adminNode.terminate]
Expand Down Expand Up @@ -91,31 +90,27 @@ export function buildRootObject(vatPowers) {

// now the duplicate terminate() comes up, and vatAdminVat should ignore it
// (PHASE 1) run-queue is:
// [self.query(3),
// vatAdmin.fireDone, self.notify(foo4P)]
// [self.query(3), vatAdmin.fireDone, self.notify(foo4P)]
// (PHASE 2) run-queue is:
// [self.query(3), vatAdmin.fireDone,
// self.notify(foreverP), self.notify(afterForeverP),
// self.notify(foo4P)]
// [self.query(3), vatAdmin.fireDone, self.notify(foreverP),
// self.notify(afterForeverP), self.notify(foo4P)]

// now the self.query(3) gets delivered, which pushes 'GOT QUERY 3' onto testLog, and
// resolves the result promise. The dead vat is the only subscriber, so the kernel
// pushes a notify event to vat-dude for it (which will never be delivered)
// (PHASE 1) run-queue is:
// [vatAdmin.fireDone,
// self.notify(foo4P), dude.notify(answerP)]
// [vatAdmin.fireDone, self.notify(foo4P), dude.notify(answerP)]
// (PHASE 2) run-queue is:
// [vatAdmin.fireDone,
// self.notify(foreverP), self.notify(afterForeverP),
// [vatAdmin.fireDone, self.notify(foreverP), self.notify(afterForeverP),
// self.notify(foo4P), dude.notify(answerP)]

// now vatAdmin gets fireDone, which resolves the 'done' promise we've been awaiting for,
// which pushes a notify
// (PHASE 1) run-queue is:
// [self.notify(foo4P), dude.notify(answerP), self.notify(doneP)]
// (PHASE 2) run-queue is:
// [self.notify(foreverP), self.notify(afterForeverP),
// self.notify(foo4P), dude.notify(answerP), self.notify(doneP)]
// [self.notify(foreverP), self.notify(afterForeverP), self.notify(foo4P),
// dude.notify(answerP), self.notify(doneP)]

// PHASE 2: we receive the rejection of foreverP, pushing
// 'foreverP.catch (err)' to testLog
Expand All @@ -137,9 +132,8 @@ export function buildRootObject(vatPowers) {
// proceed to the end of the test. We push the 'done' message to testLog
// CHIP TODO PHASE1: (or defer to phase1.5): uncomment, wire up
// queueToExport(vatAdminVat) to make it fire 'done'
// const doneMessage = await doneP;
// testLog(doneMessage);
doneP.then(() => 0); // hush eslint, delete after uncommenting await
await doneP;
testLog('done');

return 'bootstrap done';
},
Expand Down
Loading

0 comments on commit 9ad3b79

Please sign in to comment.