bad(Swingset): Releasing a constrained Zalgo on syscall replay

packages/SwingSet/src/kernel/vat-loader/async-helper.js

@@ -0,0 +1,46 @@
+/**
+ * @template T
+ * @typedef {T extends PromiseLike<any> ? never : T} SyncOnly
+ */
+
+/**
+ * @template {boolean} AllowAsync
+ * @template T
+ * @typedef {true extends AllowAsync ? (Promise<T> | T) : T} MaybePromiseResult
+ */
+
+/**
+ * WARNING: Do not use, this is releasing Zalgo
+ *
+ * @template T
+ * @template R
+ * @param {T} v
+ * @param {(v: Awaited<T>) => R} handle
+ * @returns {T extends PromiseLike<any> ? Promise<Awaited<R>> : R}
+ */
+export function maybeAsync(v, handle) {
+  if (typeof v === 'object' && v !== null && 'then' in v) {
+    return Promise.resolve(v).then(handle);
+  } else {
+    return handle(v);
+  }
+}
+
+/**
+ * Somewhat tames Zalgo
+ *
+ * @template {(...args: any[]) => any} T
+ * @param {T} fn
+ * @returns {(...args: Parameters<T>) => SyncOnly<ReturnType<T>>}
+ */
+export function ensureSync(fn) {
+  // eslint-disable-next-line func-names
+  return function (...args) {
+    const result = Reflect.apply(fn, this, args);
+    if (typeof result === 'object' && result !== null && 'then' in result) {
+      throw new Error('Unexpected async result');
+    } else {
+      return result;
+    }
+  };
+}

packages/SwingSet/src/kernel/vat-loader/manager-helper.js

@@ -3,6 +3,7 @@ import { assert } from '@agoric/assert';
 import '../../types-ambient.js';
 import { insistVatDeliveryResult } from '../../lib/message.js';
 import { makeTranscriptManager } from './transcript.js';
+import { ensureSync, maybeAsync } from './async-helper.js';
 
 // We use vat-centric terminology here, so "inbound" means "into a vat",
 // always from the kernel. Conversely "outbound" means "out of a vat", into
@@ -47,9 +48,10 @@ import { makeTranscriptManager } from './transcript.js';
 
 /**
  *
+ * @template {boolean} AsyncSyscall
  * @typedef { { getManager: (shutdown: () => Promise<void>,
  *                           makeSnapshot?: (ss: SnapStore) => Promise<SnapshotInfo>) => VatManager,
- *              syscallFromWorker: (vso: VatSyscallObject) => VatSyscallResult,
+ *              syscallFromWorker: (vso: VatSyscallObject) => import('./async-helper.js').MaybePromiseResult<AsyncSyscall, VatSyscallResult>,
  *              setDeliverToWorker: (dtw: unknown) => void,
  *            } } ManagerKit
  *
@@ -91,7 +93,7 @@ import { makeTranscriptManager } from './transcript.js';
  *
  * The returned syscallFromWorker function should be called when the worker
  * wants to make a syscall. It accepts a VatSyscallObject and will return a
- * (synchronous) VatSyscallResult, never throwing. For remote workers, this
+ * (synchronous if possible) VatSyscallResult. For remote workers, this
  * should be called from a handler that receives a syscall message from the
  * child process.
  *
@@ -106,7 +108,7 @@ import { makeTranscriptManager } from './transcript.js';
  * @param {WorkerAsyncKind} workerAsyncKind
  * @param {import('./transcript.js').CompareSyscalls} [compareSyscalls]
  * @param {boolean} [useTranscript]
- * @returns {ManagerKit}
+ * @returns {ManagerKit<'async-blocking' extends WorkerAsyncKind ? true : false>}
  */
 
 function makeManagerKit(
@@ -120,13 +122,21 @@ function makeManagerKit(
 ) {
   assert(kernelSlog);
   const vatKeeper = kernelKeeper.provideVatKeeper(vatID);
-  /** @type {ReturnType<typeof makeTranscriptManager> | undefined} */
+  /** @typedef {'async-blocking' extends WorkerAsyncKind ? true : false} AsyncMode */
+
+  /** @type {ReturnType<typeof makeTranscriptManager<AsyncMode>> | undefined} */
   let transcriptManager;
   if (useTranscript) {
+    const asyncCompliantCompareSyscall =
+      workerAsyncKind === 'async-blocking' || !compareSyscalls
+        ? /** @type {import('./transcript.js').CompareSyscalls<AsyncMode> | undefined} */ (
+            compareSyscalls
+          )
+        : ensureSync(compareSyscalls);
     transcriptManager = makeTranscriptManager(
       vatKeeper,
       vatID,
-      compareSyscalls,
+      asyncCompliantCompareSyscall,
     );
   }
 
@@ -261,7 +271,17 @@ function makeManagerKit(
 
       // but if the puppy deviates one inch from previous twitches, explode
       kernelSlog.syscall(vatID, undefined, vso);
-      return transcriptManager.simulateSyscall(vso) || doSyscallFromWorker(vso);
+      // We trust simulateSyscall to only return a promise if compareSyscalls
+      // itself returns a promise, and we propagate that mode here.
+      // Since compareSyscalls can be user provided, we ensure it doesn't
+      // return a promise if the worker doesn't support it (workerAsyncKind
+      // is not 'async-blocking')
+      return /** @type {import('./async-helper.js').MaybePromiseResult<AsyncMode, VatSyscallResult>} */ (
+        maybeAsync(
+          transcriptManager.simulateSyscall(vso),
+          vres => vres || doSyscallFromWorker(vso),
+        )
+      );
     } else {
       const vres = doSyscallFromWorker(vso);
       if (transcriptManager) {

packages/SwingSet/src/kernel/vat-loader/manager-nodeworker.js

@@ -85,7 +85,7 @@ export function makeNodeWorkerVatManagerFactory(tools) {
       } else if (type === 'syscall') {
         parentLog(`syscall`, args);
         const [vatSyscallObject] = args;
-        mk.syscallFromWorker(vatSyscallObject);
+        void mk.syscallFromWorker(vatSyscallObject);
       } else if (type === 'testLog') {
         testLog(...args);
       } else if (type === 'deliverDone') {

packages/SwingSet/src/kernel/vat-loader/manager-subprocess-node.js

@@ -82,7 +82,7 @@ export function makeNodeSubprocessFactory(tools) {
       } else if (type === 'syscall') {
         parentLog(`syscall`, args);
         const [vatSyscallObject] = args;
-        mk.syscallFromWorker(vatSyscallObject);
+        void mk.syscallFromWorker(vatSyscallObject);
       } else if (type === 'testLog') {
         testLog(...args);
       } else if (type === 'deliverDone') {

packages/SwingSet/src/kernel/vat-loader/manager-subprocess-xsnap.js

@@ -76,8 +76,8 @@ export function makeXsSubprocessFactory({
       useTranscript,
     );
 
-    /** @type { (item: Tagged) => unknown } */
-    function handleUpstream([type, ...args]) {
+    /** @type { (item: Tagged) => Promise<unknown> } */
+    async function handleUpstream([type, ...args]) {
       parentLog(vatID, `handleUpstream`, type, args.length);
       switch (type) {
         case 'syscall': {
@@ -107,7 +107,7 @@ export function makeXsSubprocessFactory({
     /** @type { (msg: Uint8Array) => Promise<Uint8Array> } */
     async function handleCommand(msg) {
       // parentLog('handleCommand', { length: msg.byteLength });
-      const tagged = handleUpstream(JSON.parse(decoder.decode(msg)));
+      const tagged = await handleUpstream(JSON.parse(decoder.decode(msg)));
       return encoder.encode(JSON.stringify(tagged));
     }
 

packages/SwingSet/src/kernel/vat-loader/transcript.js

@@ -1,6 +1,7 @@
 // @ts-check
 
 import djson from '../../lib/djson.js';
+import { maybeAsync } from './async-helper.js';
 
 // Indicate that a syscall is missing from the transcript but is safe to
 // perform during replay
@@ -12,12 +13,13 @@ export const extraSyscall = Symbol('extra transcript syscall');
 
 /** @typedef {typeof missingSyscall | typeof extraSyscall | Error | undefined} CompareSyscallsResult */
 /**
+ * @template {boolean} [MaybeAsync=boolean]
  * @typedef {(
  *     vatId: any,
  *     originalSyscall: VatSyscallObject,
  *     newSyscall: VatSyscallObject,
  *     originalResponse?: VatSyscallResult,
- *   ) => CompareSyscallsResult
+ *   ) => import('./async-helper.js').MaybePromiseResult<MaybeAsync, CompareSyscallsResult>
  * } CompareSyscalls
  */
 
@@ -98,9 +100,10 @@ export function requireIdenticalExceptStableVCSyscalls(
 }
 
 /**
+ * @template {boolean} AsyncCompare
  * @param {*} vatKeeper
  * @param {*} vatID
- * @param {CompareSyscalls} compareSyscalls
+ * @param {CompareSyscalls<AsyncCompare>} compareSyscalls
  */
 export function makeTranscriptManager(
   vatKeeper,
@@ -179,19 +182,22 @@ export function makeTranscriptManager(
         return failReplay();
       }
 
-      // eslint-disable-next-line no-use-before-define
-      return handleCompareResult(
-        compareSyscalls(
-          vatID,
-          playbackSyscalls[0].d,
-          newSyscall,
-          playbackSyscalls[0].response,
-        ),
+      return /** @type {import('./async-helper.js').MaybePromiseResult<AsyncCompare, VatSyscallResult | undefined>} */ (
+        maybeAsync(
+          compareSyscalls(
+            vatID,
+            playbackSyscalls[0].d,
+            newSyscall,
+            playbackSyscalls[0].response,
+          ),
+          // eslint-disable-next-line no-use-before-define
+          handleCompareResult,
+        )
       );
     }
     /**
      * @param {CompareSyscallsResult} compareError
-     * @returns {VatSyscallResult | undefined}
+     * @returns {import('./async-helper.js').MaybePromiseResult<AsyncCompare, VatSyscallResult | undefined>}
      */
     function handleCompareResult(compareError) {
       if (compareError === missingSyscall) {