Skip to content

Commit

Permalink
fix(swingset): make terminateVat() async
Browse files Browse the repository at this point in the history
The kernel has a function named `terminateVat()`, which is called from
both the delivery side (illegal syscall, `syscall.exit`, or meter
overflow), and the device side (when someone asks vat-admin to
terminate a vat from the outside).

Part of this function's job is to shut down any worker that was
online. Doing that is an async task.

Previously, `terminateVat` was synchronous, and when it shut down the
worker, it used `void` to disclaim interest on when exactly the
shutdown occurred (as well as any errors encountered).

This commit changes `terminateVat` to be `async`, and to wait for that
shutdown to finish.

This moves the `void` discomfort into `vat-admin-hooks.js`, where a
new TODO is a reminder that we need to propagate the async-ness
outwards, probably by converting the external shutdown request into a
run-queue event, like `create-vat` and `upgrade-vat`.
  • Loading branch information
warner authored and mergify[bot] committed Jul 14, 2022
1 parent ef5f730 commit bd93950
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
6 changes: 3 additions & 3 deletions packages/SwingSet/src/kernel/kernel.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ export default function buildKernel(
* @param {boolean} shouldReject
* @param {SwingSetCapData} info
*/
function terminateVat(vatID, shouldReject, info) {
async function terminateVat(vatID, shouldReject, info) {
const vatKeeper = kernelKeeper.provideVatKeeper(vatID);
const critical = vatKeeper.getOptions().critical;
insistCapData(info);
Expand All @@ -264,7 +264,7 @@ export default function buildKernel(
// TODO: if a static vat terminates, panic the kernel?

// ISSUE: terminate stuff in its own crank like creation?
void vatWarehouse.vatWasTerminated(vatID);
await vatWarehouse.vatWasTerminated(vatID);
}
if (critical) {
// The following error construction is a bit awkward, but (1) it saves us
Expand Down Expand Up @@ -1119,7 +1119,7 @@ export default function buildKernel(

// state changes reflecting the termination must also survive, so these
// happen after a possible abortCrank()
terminateVat(vatID, reject, info);
await terminateVat(vatID, reject, info);
kernelSlog.terminateVat(vatID, reject, info);
kdebug(`vat terminated: ${JSON.stringify(info)}`);
}
Expand Down
5 changes: 4 additions & 1 deletion packages/SwingSet/src/kernel/vat-admin-hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ export function makeVatAdminHooks(tools) {
// we don't need to incrementRefCount because if terminateVat sends
// 'reason' to vat-admin, it uses notifyTermination / queueToKref /
// doSend, and doSend() does its own incref
terminateVat(vatID, true, reasonCD);
void terminateVat(vatID, true, reasonCD);
// TODO: terminateVat is async, result doesn't fire until worker
// is dead. To fix this we'll probably need to move termination
// to a run-queue ['terminate-vat', vatID] event, like createVat
return harden({ body: stringify(undefined), slots: [] });
},

Expand Down

0 comments on commit bd93950

Please sign in to comment.