Skip to content

Commit

Permalink
fix: triage await safety
Browse files Browse the repository at this point in the history
  • Loading branch information
erights committed Oct 20, 2022
1 parent 42fdddf commit c7cfd19
Show file tree
Hide file tree
Showing 40 changed files with 453 additions and 36 deletions.
29 changes: 26 additions & 3 deletions packages/SwingSet/src/controller/initializeSwingset.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ const { keys, values, fromEntries } = Object;
* @returns {Promise<Record<string, V>>}
* @template V
*/
const allValues = async obj =>
fromEntries(zip(keys(obj), await Promise.all(values(obj))));
const allValues = async obj => {
const vals = await Promise.all(values(obj));
return fromEntries(zip(keys(obj), vals));
};

const bundleRelative = rel =>
bundleSource(new URL(rel, import.meta.url).pathname);
Expand Down Expand Up @@ -173,7 +175,14 @@ export function loadBasedir(basedir, options = {}) {
*/
async function resolveSpecFromConfig(referrer, specPath) {
try {
return new URL(await resolveModuleSpecifier(specPath, referrer)).pathname;
// This nested await is safe because "terminal-control-flow".
//
// Some code does execute after this one, synchronously or asynchronously.
// That would normally be unsafe, but all that code is clearly not stateful.
// Its correctness is independent on timing.
// eslint-disable-next-line @jessie.js/no-nested-await
const base = await resolveModuleSpecifier(specPath, referrer);
return new URL(base).pathname;
} catch (e) {
if (e.code !== 'MODULE_NOT_FOUND' && e.code !== 'ERR_MODULE_NOT_FOUND') {
throw e;
Expand Down Expand Up @@ -236,7 +245,21 @@ export async function loadSwingsetConfigFile(configPath) {
configPath,
`file:///${process.cwd()}/`,
).toString();
// This nested await is safe because "not-my-problem".
//
// It does occur nested with a control flow branch. But it is at top
// level within that branch, followed by another await also at
// top level within that branch. Therefore, this await introduces
// no unsafety beyond that potentially caused by the next await.
// eslint-disable-next-line @jessie.js/no-nested-await
await normalizeConfigDescriptor(config.vats, referrer, true);
// This nested await is safe because "synchonous-throw-impossible".
//
// normalizeConfigDescriptor is an async function. The only argument
// expression that might in theory throw is `config.bundles`, which we
// are confident could not actually throw. Even if it could, we'd still
// be safe because "terminal-control-flow". The catch body is not stateful.
// eslint-disable-next-line @jessie.js/no-nested-await
await normalizeConfigDescriptor(config.bundles, referrer, false);
// await normalizeConfigDescriptor(config.devices, referrer, true); // TODO: represent devices
assert(config.bootstrap, X`no designated bootstrap vat in ${configPath}`);
Expand Down
9 changes: 9 additions & 0 deletions packages/SwingSet/src/devices/plugin/device-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,15 @@ export function buildRootDeviceNode(tools) {
*/
async function createConnection(mod, index, epoch) {
try {
// This nested await is safe because "terminal-throw-control-flow".
//
// This statement appears at the top level of a top level try block,
// and so is executed
// unconditionally. If it throws, we do nothing significantly stateful
// before exiting. (We do not consider `console.log` to be stateful
// for these purposes.) Otherwise, it will always cause a turn boundary
// before control flow continues.
// eslint-disable-next-line @jessie.js/no-nested-await
const modNS = await endowments.import(mod);
const receiver = obj => {
// console.info('receiver', index, obj);
Expand Down
22 changes: 20 additions & 2 deletions packages/SwingSet/src/kernel/vat-warehouse.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,18 @@ export function makeVatWarehouse(kernelKeeper, vatLoader, policyOptions) {
break;
}
logStartup(`provideVatKeeper for vat ${name} as vat ${vatID}`);
// eslint-disable-next-line no-await-in-loop
// Nested await safe because "terminal-combined-control-flow".
//
// `ensureVatOnline` is an async function, so this turn boundary always
// happens at the end of each iteration. However, the control-flow is
// still unbalanced. If the loop iterates zero times, we proceed to
// the next code synchronously. Else we proceed asynchronously.
// However, the next loop is terminal, and each iteration of that loop
// resemble each iteration of this one. Considered together, in all
// cases the first iteration of one of these loops will happen first,
// if there are any. And if there are more iterations from either loop,
// a turn boundary separated each iteration from the next.
// eslint-disable-next-line no-await-in-loop, @jessie.js/no-nested-await
await ensureVatOnline(vatID, recreate);
numPreloaded += 1;
}
Expand All @@ -187,7 +198,8 @@ export function makeVatWarehouse(kernelKeeper, vatLoader, policyOptions) {
break;
}
logStartup(`provideVatKeeper for dynamic vat ${vatID}`);
// eslint-disable-next-line no-await-in-loop
// Nested await safe because "terminal-control-flow"
// eslint-disable-next-line no-await-in-loop, @jessie.js/no-nested-await
await ensureVatOnline(vatID, recreate);
numPreloaded += 1;
}
Expand Down Expand Up @@ -392,6 +404,12 @@ export function makeVatWarehouse(kernelKeeper, vatLoader, policyOptions) {
// worker may or may not be online
if (ephemeral.vats.has(vatID)) {
try {
// This nested await is safe because "terminal-control-flow".
//
// For these purposes, we do not consider `console.debug` to be
// significantly stateful. That said, no stateful code in this
// function executes after this line.
// eslint-disable-next-line @jessie.js/no-nested-await
await evict(vatID);
} catch (err) {
console.debug('vat termination was already reported; ignoring:', err);
Expand Down
33 changes: 32 additions & 1 deletion packages/SwingSet/src/vats/network/network.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,28 @@ export function makeNetworkProtocol(protocolHandler) {
// Check if we are underspecified (ends in slash)
if (localAddr.endsWith(ENDPOINT_SEPARATOR)) {
for (;;) {
// eslint-disable-next-line no-await-in-loop
// This nested await is safe because "terminal-throw-control-flow" +
// "synchronous-throw-impossible" + "not-my-problem".
//
// We assume `E` cannot synchronously throw, and all the argument
// expressions are simple variables. If the await expression throws
// we exit the function immediately. Thus, there is a reliable
// turn boundary before each iteration of the loop. There remains
// the issue of zero iterations of the loop, which in this case is
// impossible because of the absence of a condition in the head.
//
// Finally, there remains the issue of the unbalances `if`. The
// then case will always take at least one turn boundary before
// proceeding. But the else case will proceed synchronously to
// the subsequent code. Fortunately, the next interesting statement
// is a top level await, so this await does not introduce any
// unsafety beyond that caused by the other await.
// Hence "not-my-problem". That await is manifestly safe because it
// is at top level of the function and always forces a turn boundary.
// The awaited expression is another `E` message send expression in
// which all the argument expressions are only variables, so it will
// not cause anything stateful prior to that turn boundary.
// eslint-disable-next-line no-await-in-loop, @jessie.js/no-nested-await
const portID = await E(protocolHandler).generatePortID(
localAddr,
protocolHandler,
Expand Down Expand Up @@ -360,6 +381,8 @@ export function makeNetworkProtocol(protocolHandler) {
},
});

// The safety of the `await` in the loop above depends on this `await`
// remaining in place.
await E(protocolHandler).onBind(port, localAddr, protocolHandler);
boundPorts.init(localAddr, port);
currentConnections.init(port, new Set());
Expand All @@ -381,6 +404,10 @@ export function makeNetworkProtocol(protocolHandler) {
const [port, listener] = listening.get(listenPrefix);
let localAddr;
try {
// TODO FIXME This code should be refactored to make this
// await checkably safe, or to remove it, or to record here
// why it is actually safe.
//
// See if our protocol is willing to receive this connection.
// eslint-disable-next-line no-await-in-loop
const localInstance = await E(protocolHandler)
Expand Down Expand Up @@ -462,6 +489,10 @@ export function makeNetworkProtocol(protocolHandler) {

let lastFailure;
try {
// TODO FIXME This code should be refactored to make this
// await checkably safe, or to remove it, or to record here
// why it is actually safe.
//
// Attempt the loopback connection.
const attempt = await protocolImpl.inbound(
remoteAddr,
Expand Down
2 changes: 2 additions & 0 deletions packages/agoric-cli/src/cosmos.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ export default async function cosmosMain(progname, rawArgs, powers, opts) {
}

if (popts.pull) {
// This await is safe because "terminal-control-flow"
// eslint-disable-next-line @jessie.js/no-nested-await
const exitStatus = await pspawn('docker', ['pull', IMAGE]);
if (exitStatus) {
return exitStatus;
Expand Down
5 changes: 5 additions & 0 deletions packages/agoric-cli/src/deploy.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// IIUC the purpose of the agoric-cli package, we do not need to worry
// about await safety for any of the code in this package.
// TODO someone who understands this package better should verify this.
/* eslint-disable @jessie.js/no-nested-await */

/* global process setTimeout setInterval clearInterval */
/* eslint-disable no-await-in-loop */

Expand Down
5 changes: 5 additions & 0 deletions packages/agoric-cli/src/init.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// IIUC the purpose of the agoric-cli package, we do not need to worry
// about await safety for any of the code in this package.
// TODO someone who understands this package better should verify this.
/* eslint-disable @jessie.js/no-nested-await */

import chalk from 'chalk';
import { makePspawn } from './helpers.js';

Expand Down
5 changes: 5 additions & 0 deletions packages/agoric-cli/src/install.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// IIUC the purpose of the agoric-cli package, we do not need to worry
// about await safety for any of the code in this package.
// TODO someone who understands this package better should verify this.
/* eslint-disable @jessie.js/no-nested-await */

/* global process AggregateError Buffer */
import path from 'path';
import chalk from 'chalk';
Expand Down
3 changes: 3 additions & 0 deletions packages/agoric-cli/src/open.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ export default async function walletMain(progname, rawArgs, powers, opts) {
process.stdout.write(`${walletUrl}\n`);
if (opts.browser) {
const browser = opener(walletUrl);
// This await is safe because terminal-control-flow.
//
// eslint-disable-next-line @jessie.js/no-nested-await
await new Promise(resolve => browser.on('exit', resolve));
}
}
5 changes: 5 additions & 0 deletions packages/agoric-cli/src/publish.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// IIUC the purpose of the agoric-cli package, we do not need to worry
// about await safety for any of the code in this package.
// TODO someone who understands this package better should verify this.
/* eslint-disable @jessie.js/no-nested-await */

// @ts-check
/// <reference types="ses"/>

Expand Down
5 changes: 5 additions & 0 deletions packages/agoric-cli/src/set-defaults.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// IIUC the purpose of the agoric-cli package, we do not need to worry
// about await safety for any of the code in this package.
// TODO someone who understands this package better should verify this.
/* eslint-disable @jessie.js/no-nested-await */

import { basename } from 'path';
import { assert, details as X } from '@agoric/assert';
import {
Expand Down
5 changes: 5 additions & 0 deletions packages/agoric-cli/src/start.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// IIUC the purpose of the agoric-cli package, we do not need to worry
// about await safety for any of the code in this package.
// TODO someone who understands this package better should verify this.
/* eslint-disable @jessie.js/no-nested-await */

/* global process setTimeout */
import chalk from 'chalk';
import { createHash } from 'crypto';
Expand Down
8 changes: 8 additions & 0 deletions packages/cache/src/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ const applyCacheTransaction = async (
// Loop until our updated state is fresh wrt our current state.
basisState = stateStore.get(keyStr);
while (updatedState && updatedState.generation <= basisState.generation) {
// TODO FIXME This code should be refactored to make this
// await checkably safe, or to remove it, or to record here
// why it is actually safe.
//
// If we knew that this loop is guaranteed to execute at least once,
// then this await would be safe because "terminal-throw-control-flow".
// But if it might execute zero times, then the stateful code after
// this loop might execute synchronously or asynchronously.
// eslint-disable-next-line no-await-in-loop
updatedState = await getUpdatedState(basisState);
// AWAIT INTERLEAVING
Expand Down
25 changes: 18 additions & 7 deletions packages/cosmic-swingset/src/chain-main.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,11 @@ export default async function main(progname, args, { env, homedir, agcc }) {
const port = lastPort;
portHandlers[port] = async (...phArgs) => {
try {
// Nested await is safe because "terminal-control-flow"
//
// For these purposes, `console.error` is not considered significantly
// stateful.
// eslint-disable-next-line @jessie.js/no-nested-await
return await portHandler(...phArgs);
} catch (e) {
console.error('portHandler threw', e);
Expand Down Expand Up @@ -303,13 +308,12 @@ export default async function main(progname, args, { env, homedir, agcc }) {
ROLE: 'chain',
bootMsg,
};
const vatconfig = new URL(
await importMetaResolve(
env.CHAIN_BOOTSTRAP_VAT_CONFIG ||
argv.bootMsg.params.bootstrap_vat_config,
import.meta.url,
),
).pathname;
const url = await importMetaResolve(
env.CHAIN_BOOTSTRAP_VAT_CONFIG ||
argv.bootMsg.params.bootstrap_vat_config,
import.meta.url,
);
const vatconfig = new URL(url).pathname;

const { metricsProvider } = getTelemetryProviders({
console,
Expand Down Expand Up @@ -458,6 +462,13 @@ export default async function main(progname, args, { env, homedir, agcc }) {

// Ensure that initialization has completed.
if (!blockingSend) {
// TODO FIXME This code should be refactored to make this
// await checkably safe, or to remove it, or to record here
// why it is actually safe.
//
// Likely `blockingSend` is not sensitive to whether it is called
// in the same or a subsequent turn. If that were verified, then
// this await would be safe because "terminal-control-flow".
blockingSend = await launchAndInitializeSwingSet(action);
}

Expand Down
27 changes: 27 additions & 0 deletions packages/cosmic-swingset/src/launch-chain.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ async function buildSwingset(

// Find the entrypoints for all the core proposals.
if (config.coreProposals) {
// TODO FIXME This code should be refactored to make this
// await checkably safe, or to remove it, or to record here
// why it is actually safe.
//
// `initializeSwingset` is stateful and is called synchronously
// or asynchronously, depending on which branch of the conditional
// is taken. If it were verified to be insensitive to this,
// then this await would be safe because "terminal-control-flow"
const { bundles, code } = await extractCoreProposalBundles(
config.coreProposals,
vatconfig,
Expand Down Expand Up @@ -654,6 +662,11 @@ export async function launch({
blockHeight,
runNum,
});
// This nested await is safe because "terminal-control-flow".
//
// It occurs at the top level of a case of an unbalanced,
// but top level terminal switch. Nothing happens after the switch.
// eslint-disable-next-line @jessie.js/no-nested-await
await processAction(action.type, bootstrapBlock);
controller.writeSlogObject({
type: 'cosmic-swingset-run-finish',
Expand Down Expand Up @@ -685,6 +698,9 @@ export async function launch({

// Save the kernel's computed state just before the chain commits.
const start2 = Date.now();
// This nested await is safe because "terminal-control-flow".
//
// eslint-disable-next-line @jessie.js/no-nested-await
await saveOutsideState(savedHeight, blockTime);
const saveTime = Date.now() - start2;
controller.writeSlogObject({
Expand Down Expand Up @@ -756,6 +772,14 @@ export async function launch({

provideInstallationPublisher();

// This nested await is safe because "terminal-control-flow".
//
// It occurs at the top level of a branch of an unbalanced
// but terminal if, that is top level and terminal within a case
// of a top level unbalanced but terminal and top level switch.
// Thus, nothing happens after completion of the immeiately
// enclosing if.
// eslint-disable-next-line @jessie.js/no-nested-await
await processAction(action.type, async () =>
runKernel(
blockHeight,
Expand All @@ -767,6 +791,9 @@ export async function launch({

// We write out our on-chain state as a number of chainSends.
const start = Date.now();
// This nested await is safe because "terminal-control-flow".
//
// eslint-disable-next-line @jessie.js/no-nested-await
await saveChainState();
chainTime = Date.now() - start;

Expand Down
Loading

0 comments on commit c7cfd19

Please sign in to comment.