Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(swingset): allow buildRootObject to return a Promise #5250

Merged
merged 1 commit into from
Apr 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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