Skip to content

Commit

Permalink
feat(swingset): allow buildRootObject to return a Promise
Browse files Browse the repository at this point in the history
New (or upgraded) vats are defined by a user-provided
`buildRootObject` function. This function has two responsibilities:

* provide a root object
* use `defineDurableKind` to bind behavior to any pre-existing durable
  kinds

Previously, `buildRootObject` was given exactly one "turn" to fulfill
these responsibilities: it was required to return the root object
synchronously. If it returned a Promise, that was rejected as an
error.

This requirement is at odds with the needs for updated ZCF vats to
evaluate their contract code and interact with it (giving the contract
a chance to bind their own Kind behavior). The `importBundle` function
we use to load contracts is async. However, it is still "prompt": it
needs no IO, nor any messages leaving the vat. So it will still
complete within the *crank*, just not synchronously within the turn.

So this commit changes liveslots to allow `buildRootObject` to return
a Promise for the root object. The requirement, however, is that this
Promise must resolve by the end of the initial crank (before
`waitUntilQuiescent` / `setImmediate` determines that the promise
queue is empty). If the Promise is not fulfilled by then, the vat
creation/update is considered broken.

closes #5246
  • Loading branch information
warner committed Apr 29, 2022
1 parent b35595c commit 169cbc2
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ export function makeXsSubprocessFactory({
async function issueTagged(item) {
parentLog(item[0], '...', item.length - 1);
const result = await worker.issueStringCommand(JSON.stringify(item));
// note: if 'result.reply' is empty, that probably means the
// worker reached end-of-crank (idle) without seeing the
// `dispatch()` promise resolve
const reply = JSON.parse(result.reply);
assert(Array.isArray(reply));
const [tag, ...rest] = reply;
Expand Down
22 changes: 15 additions & 7 deletions packages/SwingSet/src/liveslots/liveslots.js
Original file line number Diff line number Diff line change
Expand Up @@ -1275,7 +1275,7 @@ function build(
);

// here we finally invoke the vat code, and get back the root object
const rootObject = buildRootObject(
const rootObject = await buildRootObject(
harden(vpow),
harden(vatParameters),
baggage,
Expand Down Expand Up @@ -1447,18 +1447,26 @@ function build(
} else if (delivery[0] === 'stopVat') {
return meterControl.runWithoutMeteringAsync(stopVat);
} else {
let complete = false;
// Start user code running, record any internal liveslots errors. We do
// *not* directly wait for the userspace function to complete, nor for
// any promise it returns to fire.
const p = Promise.resolve(delivery).then(unmeteredDispatch);

// Instead, we wait for userspace to become idle by draining the promise
// queue. We clean up and then return 'p' so any bugs in liveslots that
// cause an error during unmeteredDispatch will be reported to the
// supervisor (but only after userspace is idle).
p.finally(() => (complete = true));

// Instead, we wait for userspace to become idle by draining the
// promise queue. We clean up and then examine/return 'p' so any
// bugs in liveslots that cause an error during
// unmeteredDispatch (or a 'buildRootObject' that fails to
// complete in time) will be reported to the supervisor (but
// only after userspace is idle).
return gcTools.waitUntilQuiescent().then(() => {
afterDispatchActions();
return p;
// eslint-disable-next-line prefer-promise-reject-errors
return complete ? p : Promise.reject('buildRootObject unresolved');
// the only delivery that pays attention to a user-provided
// Promise is startVat, so the error message is specialized to
// the only user problem that could cause complete===false
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/SwingSet/src/supervisors/supervisor-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function makeSupervisorDispatch(dispatch) {
err => {
// TODO react more thoughtfully, maybe terminate the vat
console.log(`error ${err} during vat dispatch() of ${delivery}`);
return harden(['error', `${err.message}`, null]);
return harden(['error', `${err}`, null]);
},
);
}
Expand Down
4 changes: 3 additions & 1 deletion packages/SwingSet/test/upgrade/vat-ulrik-2.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const buildRootObject = (_vatPowers, vatParameters, baggage) => {
const durandalHandle = baggage.get('durandalHandle');
defineDurableKind(durandalHandle, initialize, behavior);

return Far('root', {
const root = Far('root', {
getVersion: () => 'v2',
getParameters: () => vatParameters,
getPresence: () => baggage.get('presence'),
Expand All @@ -42,4 +42,6 @@ export const buildRootObject = (_vatPowers, vatParameters, baggage) => {
return { imp33, imp35, imp37, imp38 };
},
});
// exercise async return
return Promise.resolve(root);
};
6 changes: 6 additions & 0 deletions packages/SwingSet/test/vat-admin/broken-hang-vat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { makePromiseKit } from '@endo/promise-kit';

export function buildRootObject() {
const pk = makePromiseKit();
return pk.promise; // never resolves
}
4 changes: 3 additions & 1 deletion packages/SwingSet/test/vat-admin/new-vat-13.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function buildRootObject(_vatPowers, vatParameters) {
},
});
}
return Far('root', {
const root = Far('root', {
getANumber() {
return 13;
},
Expand All @@ -31,4 +31,6 @@ export function buildRootObject(_vatPowers, vatParameters) {
return rcvrMaker(init);
},
});
// exercise async return
return Promise.resolve(root);
}
17 changes: 13 additions & 4 deletions packages/SwingSet/test/vat-admin/test-create-vat.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ test.before(async t => {
const brokenRootVatBundle = await bundleSource(
new URL('broken-root-vat.js', import.meta.url).pathname,
);
const brokenHangVatBundle = await bundleSource(
new URL('broken-hang-vat.js', import.meta.url).pathname,
);
const vatRefcountBundle = await bundleSource(
new URL('new-vat-refcount.js', import.meta.url).pathname,
);
Expand All @@ -39,6 +42,7 @@ test.before(async t => {
vat44Bundle,
brokenModuleVatBundle,
brokenRootVatBundle,
brokenHangVatBundle,
vatRefcountBundle,
nonBundle,
};
Expand All @@ -53,6 +57,7 @@ async function doTestSetup(t, enableSlog = false) {
new13: { bundle: bundles.vat13Bundle },
brokenModule: { bundle: bundles.brokenModuleVatBundle },
brokenRoot: { bundle: bundles.brokenRootVatBundle },
brokenHang: { bundle: bundles.brokenHangVatBundle },
};
const hostStorage = provideHostStorage();
await initializeSwingset(config, [], hostStorage, { kernelBundles });
Expand Down Expand Up @@ -123,7 +128,7 @@ test('counter test', async t => {
t.deepEqual(JSON.parse(c.kpResolution(kpid).body), [4, 9, 2, 8]);
});

async function brokenVatTest(t, bundleName) {
async function brokenVatTest(t, bundleName, expected) {
const { c } = await doTestSetup(t);
const kpid = c.queueToVatRoot(
'bootstrap',
Expand All @@ -136,15 +141,19 @@ async function brokenVatTest(t, bundleName) {
t.truthy(res instanceof Error);
// 'Vat Creation Error: Error: missing is not defined'
t.regex(res.message, /Vat Creation Error/);
t.regex(res.message, /missing/);
t.regex(res.message, expected);
}

test('broken vat creation fails (bad module)', async t => {
await brokenVatTest(t, 'brokenModule');
await brokenVatTest(t, 'brokenModule', /missing/);
});

test('broken vat creation fails (bad buildRootObject)', async t => {
await brokenVatTest(t, 'brokenRoot');
await brokenVatTest(t, 'brokenRoot', /missing/);
});

test('broken vat creation fails (buildRootObject hangs)', async t => {
await brokenVatTest(t, 'brokenHang', /buildRootObject unresolved/);
});

test('error creating vat from non-bundle', async t => {
Expand Down

0 comments on commit 169cbc2

Please sign in to comment.