Skip to content

Commit

Permalink
feat: allow vats to be marked critical and panic the kernel if a crit…
Browse files Browse the repository at this point in the history
…ical vat fails

Fixes #4279
  • Loading branch information
FUDCo authored and mergify[bot] committed May 28, 2022
1 parent 232c1d9 commit 9ef4941
Show file tree
Hide file tree
Showing 13 changed files with 351 additions and 45 deletions.
1 change: 1 addition & 0 deletions packages/SwingSet/src/controller/initializeKernel.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export function initializeKernel(config, hostStorage, verbose = false) {
'enableSetup',
'virtualObjectCacheSize',
'useTranscript',
'critical',
'reapInterval',
]);
creationOptions.description = `static name=${name}`;
Expand Down
16 changes: 15 additions & 1 deletion packages/SwingSet/src/kernel/kernel.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,8 @@ export default function buildKernel(
* @param {SwingSetCapData} info
*/
function terminateVat(vatID, shouldReject, info) {
const vatKeeper = kernelKeeper.provideVatKeeper(vatID);
const critical = vatKeeper.getOptions().critical;
insistCapData(info);
// guard against somebody telling vatAdmin to kill a vat twice
if (kernelKeeper.vatIsAlive(vatID)) {
Expand All @@ -263,6 +265,18 @@ export default function buildKernel(
// eslint-disable-next-line no-use-before-define
vatWarehouse.vatWasTerminated(vatID);
}
if (critical) {
// The following error construction is a bit awkward, but (1) it saves us
// from doing unmarshaling while in the kernel, while (2) it protects
// against the info not actually encoding an error, but (3) it still
// provides some diagnostic information back to the host, and (4) this
// should happen rarely enough that if you have to do a little bit of
// extra interpretive work on the receiving end to diagnose the problem,
// it's going to be a small cost compared to the trouble you're probably
// already in anyway if this happens.
panic(`critical vat ${vatID} failed`, Error(info.body));
return;
}
if (vatAdminRootKref) {
// static vat termination can happen before vat admin vat exists
notifyTermination(
Expand Down Expand Up @@ -306,7 +320,7 @@ export default function buildKernel(
insistCapData(info);
// if vatFatalSyscall was here already, don't override: bad syscalls win
if (!terminationTrigger) {
terminationTrigger = { vatID, abortCrank: false, reject, info };
terminationTrigger = { vatID, abortCrank: reject, reject, info };
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/SwingSet/src/kernel/vat-loader/manager-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export function makeVatManagerFactory({
'enableSetup',
'virtualObjectCacheSize',
'useTranscript',
'critical',
'reapInterval',
'sourcedConsole',
'name',
Expand Down
5 changes: 5 additions & 0 deletions packages/SwingSet/src/kernel/vat-loader/vat-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export function makeVatLoader(stuff) {
'enablePipelining',
'virtualObjectCacheSize',
'useTranscript',
'critical',
'reapInterval',
];

Expand All @@ -106,6 +107,7 @@ export function makeVatLoader(stuff) {
'enablePipelining',
'virtualObjectCacheSize',
'useTranscript',
'critical',
'reapInterval',
];

Expand Down Expand Up @@ -171,6 +173,7 @@ export function makeVatLoader(stuff) {
* @param {string} [options.name]
* @param {string} [options.description]
* @param {boolean} [options.enableDisavow]
* @param {boolean} [options.critical]
*
* @param {boolean} isDynamic If true, the vat being created is a dynamic vat;
* if false, it's a static vat (these have differences in their allowed
Expand Down Expand Up @@ -215,6 +218,7 @@ export function makeVatLoader(stuff) {
enablePipelining = false,
virtualObjectCacheSize,
useTranscript = true,
critical = false,
name,
} = options;

Expand All @@ -239,6 +243,7 @@ export function makeVatLoader(stuff) {
sourcedConsole: makeSourcedConsole(vatID),
virtualObjectCacheSize,
useTranscript,
critical,
name,
...overrideVatManagerOptions,
};
Expand Down
2 changes: 2 additions & 0 deletions packages/SwingSet/src/types-external.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export {};
* managerType: ManagerType,
* gcEveryCrank?: boolean,
* metered?: boolean,
* critical?: boolean,
* enableDisavow?: boolean,
* useTranscript?: boolean,
* reapInterval?: number | 'never',
Expand Down Expand Up @@ -336,6 +337,7 @@ export {};
* virtualObjectCacheSize?: number,
* useTranscript?: boolean,
* reapInterval? : number | 'never',
* critical?: boolean,
* }} DynamicVatOptionsWithoutMeter
* @typedef { { meter?: Meter } } HasMeter
* @typedef { DynamicVatOptionsWithoutMeter & HasMeter } DynamicVatOptions
Expand Down
14 changes: 14 additions & 0 deletions packages/SwingSet/src/vats/vat-admin/vat-vat-admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ export function buildRootObject(vatPowers) {
});
}

const criticalVatKey = Far('criticalVatKey', {});
function getCriticalVatKey() {
return criticalVatKey;
}

function assertType(name, obj, type) {
if (obj) {
assert.typeof(obj, type, `CreateVatOptions(${name})`);
Expand All @@ -138,6 +143,7 @@ export function buildRootObject(vatPowers) {
virtualObjectCacheSize,
useTranscript,
reapInterval,
critical, // converted from cap key to boolean
...rest
} = origOptions;

Expand Down Expand Up @@ -168,6 +174,12 @@ export function buildRootObject(vatPowers) {
meterID = meterIDByMeter.get(origOptions.meter);
}

let isCriticalVat = false;
if (critical) {
assert(critical === criticalVatKey, 'invalid criticalVatKey');
isCriticalVat = true;
}

// TODO: assert vatParameters is Durable

// now glue everything back together
Expand All @@ -181,6 +193,7 @@ export function buildRootObject(vatPowers) {
virtualObjectCacheSize,
useTranscript,
reapInterval,
critical: isCriticalVat,
};
return harden(options);
}
Expand Down Expand Up @@ -325,6 +338,7 @@ export function buildRootObject(vatPowers) {

return Far('root', {
createVatAdminService,
getCriticalVatKey,
bundleInstalled,
newVatCallback,
vatUpgradeCallback,
Expand Down
13 changes: 13 additions & 0 deletions packages/SwingSet/test/vat-admin/terminate/bootstrap-badVatKey.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { E } from '@endo/eventual-send';
import { Far } from '@endo/marshal';

export function buildRootObject() {
return Far('root', {
async bootstrap(vats, devices) {
const vatMaker = E(vats.vatAdmin).createVatAdminService(devices.vatAdmin);
await E(vatMaker).createVatByName('dude', {
critical: true, // this is bogus and should fail
});
},
});
}
67 changes: 43 additions & 24 deletions packages/SwingSet/test/vat-admin/terminate/bootstrap-terminate.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,39 @@ import { Far } from '@endo/marshal';

export function buildRootObject(vatPowers, vatParameters) {
const { testLog } = vatPowers;
const mode = vatParameters.argv[0];
const [mode, critical, dynamic] = vatParameters.argv;

const self = Far('root', {
async bootstrap(vats, devices) {
const vatMaker = E(vats.vatAdmin).createVatAdminService(devices.vatAdmin);

// create a dynamic vat, send it a message and let it respond, to make
// sure everything is working
const dude = await E(vatMaker).createVatByName('dude');
const count1 = await E(dude.root).foo(1);
let root;
let adminNode;
if (dynamic) {
const vatMaker = E(vats.vatAdmin).createVatAdminService(
devices.vatAdmin,
);
let criticalVatKey = false;
if (critical) {
criticalVatKey = await E(vats.vatAdmin).getCriticalVatKey();
}
const dude = await E(vatMaker).createVatByName('dude', {
critical: criticalVatKey,
});
root = dude.root;
adminNode = dude.adminNode;
} else if (critical) {
root = vats.staticCritical;
} else {
root = vats.staticNonCritical;
}
const count1 = await E(root).foo(1);
// pushes 'FOO 1' to testLog
testLog(`count1 ${count1}`); // 'count1 FOO SAYS 1'

// get a promise that will never be resolved, at least not until the
// vat dies
const foreverP = E(dude.root).never();
const foreverP = E(root).never();
foreverP.then(
answer => testLog(`foreverP.then ${answer}`),
err => testLog(`foreverP.catch ${err}`),
Expand All @@ -37,64 +54,64 @@ export function buildRootObject(vatPowers, vatParameters) {
// make it send an outgoing query, both to make sure that works, and
// also to make sure never() has been delivered before the vat is
// killed
const query2 = await E(dude.root).elsewhere(self, 2);
const query2 = await E(root).elsewhere(self, 2);
// pushes 'QUERY 2', 'GOT QUERY 2', 'ANSWER 2' to testLog
testLog(`query2 ${query2}`); // 'query2 2'

// now we queue up a batch of four messages:

// the first will trigger another outgoing query ..
const query3P = E(dude.root).elsewhere(self, 3);
const query3P = E(root).elsewhere(self, 3);
query3P.then(
answer => testLog(`query3P.then ${answer}`),
err => testLog(`query3P.catch ${err}`),
);
// .. but it will be killed ..
switch (mode) {
case 'kill':
E(dude.adminNode).terminateWithFailure(mode);
E(adminNode).terminateWithFailure(mode);
break;
case 'happy':
E(dude.root).dieHappy(mode);
E(root).dieHappy(mode);
break;
case 'exceptionallyHappy':
E(dude.root).dieHappy(Error(mode));
E(root).dieHappy(Error(mode));
break;
case 'happyTalkFirst':
E(dude.root).dieHappyButTalkToMeFirst(self, mode);
E(root).dieHappyButTalkToMeFirst(self, mode);
break;
case 'sad':
E(dude.root).dieSad(mode);
E(root).dieSad(mode);
break;
case 'exceptionallySad':
E(dude.root).dieSad(Error(mode));
E(root).dieSad(Error(mode));
break;
case 'sadTalkFirst':
E(dude.root).dieSadButTalkToMeFirst(self, Error(mode));
E(root).dieSadButTalkToMeFirst(self, Error(mode));
break;
case 'dieReturningAPresence':
E(dude.root).dieReturningAPresence(self, Error(mode));
E(root).dieReturningAPresence(self, Error(mode));
break;
default:
console.log('something terrible has happened');
break;
}
// .. before the third message can be delivered
const foo4P = E(dude.root).foo(4);
const foo4P = E(root).foo(4);
foo4P.then(
answer => testLog(`foo4P.then ${answer}`),
err => testLog(`foo4P.catch ${err}`),
);
// then we try to kill the vat again, which should be idempotent
if (mode === 'kill') {
E(dude.adminNode).terminateWithFailure('because we said so');
E(adminNode).terminateWithFailure('because we said so');
}

// the run-queue should now look like:
// [dude.elsewhere(3), adminNode.terminate, dude.foo(4), adminNode.terminate]

// finally we wait for the kernel to tell us that the vat is dead
const doneP = E(dude.adminNode).done();
const doneP = adminNode ? E(adminNode).done() : null;

// first, dude.elsewhere(self, 3) is delivered, which pushes
// 'QUERY 3' to testLog, and sends query(). The run-queue should now look like:
Expand Down Expand Up @@ -148,11 +165,13 @@ export function buildRootObject(vatPowers, vatParameters) {

// We finally hear about doneP resolving, allowing the bootstrap to
// proceed to the end of the test. We push the 'done' message to testLog
try {
const v = await doneP;
testLog(`done result ${v} (Error=${v instanceof Error})`);
} catch (e) {
testLog(`done exception ${e} (Error=${e instanceof Error})`);
if (doneP) {
try {
const v = await doneP;
testLog(`done result ${v} (Error=${v instanceof Error})`);
} catch (e) {
testLog(`done exception ${e} (Error=${e instanceof Error})`);
}
}
testLog('done');

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"bootstrap": "bootstrap",
"bundles": {
"dude": {
"sourceSpec": "vat-dude-terminate.js"
}
},
"vats": {
"bootstrap": {
"sourceSpec": "bootstrap-badVatKey.js"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@
"vats": {
"bootstrap": {
"sourceSpec": "bootstrap-terminate.js"
},
"staticNonCritical": {
"bundleName": "dude"
},
"staticCritical": {
"bundleName": "dude",
"creationOptions": {
"critical": true
}
}
}
}

0 comments on commit 9ef4941

Please sign in to comment.