From 8a5245382bbcca46b45f560da26c248489c56f3e Mon Sep 17 00:00:00 2001
From: Erik Marks <rekmarks@protonmail.com>
Date: Tue, 20 Feb 2024 21:36:54 -0800
Subject: [PATCH] fix(endo): Synchronize host cancellation with formula graph

Synchronizes the host's `cancel()` with the formula graph by awaiting
the formula graph mutex in a new daemon method, `cancelValue()`. Modifies
the mutex implementation to permit calling `enqueue()` without
specifying function to call.
---
 packages/daemon/src/daemon.js     | 10 +++++++++-
 packages/daemon/src/mail.js       | 10 +++-------
 packages/daemon/src/mutex.js      |  5 ++---
 packages/daemon/src/types.d.ts    | 10 ++++++++++
 packages/daemon/test/test-endo.js |  6 ------
 5 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/packages/daemon/src/daemon.js b/packages/daemon/src/daemon.js
index e4e84ff275..f66407a7ca 100644
--- a/packages/daemon/src/daemon.js
+++ b/packages/daemon/src/daemon.js
@@ -652,6 +652,14 @@ const makeDaemonCore = async (
     return controller;
   };
 
+  /** @type {import('./types.js').CancelValue} */
+  const cancelValue = async (formulaIdentifier, reason) => {
+    await formulaGraphMutex.enqueue();
+    const controller = provideControllerForFormulaIdentifier(formulaIdentifier);
+    console.log('Cancelled:');
+    return controller.context.cancel(reason);
+  };
+
   /** @type {import('./types.js').ProvideValueForFormulaIdentifier} */
   const provideValueForFormulaIdentifier = formulaIdentifier => {
     const controller = /** @type {import('./types.js').Controller<>} */ (
@@ -1087,8 +1095,8 @@ const makeDaemonCore = async (
   const makeMailbox = makeMailboxMaker({
     getFormulaIdentifierForRef,
     provideValueForFormulaIdentifier,
-    provideControllerForFormulaIdentifier,
     provideControllerForFormulaIdentifierAndResolveHandle,
+    cancelValue,
   });
 
   const makeIdentifiedGuestController = makeGuestMaker({
diff --git a/packages/daemon/src/mail.js b/packages/daemon/src/mail.js
index a98f8657cd..914969c984 100644
--- a/packages/daemon/src/mail.js
+++ b/packages/daemon/src/mail.js
@@ -11,15 +11,15 @@ const { quote: q } = assert;
 /**
  * @param {object} args
  * @param {import('./types.js').ProvideValueForFormulaIdentifier} args.provideValueForFormulaIdentifier
- * @param {import('./types.js').ProvideControllerForFormulaIdentifier} args.provideControllerForFormulaIdentifier
  * @param {import('./types.js').GetFormulaIdentifierForRef} args.getFormulaIdentifierForRef
  * @param {import('./types.js').ProvideControllerForFormulaIdentifierAndResolveHandle} args.provideControllerForFormulaIdentifierAndResolveHandle
+ * @param {import('./types.js').CancelValue} args.cancelValue
  */
 export const makeMailboxMaker = ({
   getFormulaIdentifierForRef,
   provideValueForFormulaIdentifier,
-  provideControllerForFormulaIdentifier,
   provideControllerForFormulaIdentifierAndResolveHandle,
+  cancelValue,
 }) => {
   /**
    * @param {object} args
@@ -81,11 +81,7 @@ export const makeMailboxMaker = ({
       if (formulaIdentifier === undefined) {
         throw new TypeError(`Unknown pet name: ${q(petName)}`);
       }
-      // Behold, recursion:
-      const controller =
-        provideControllerForFormulaIdentifier(formulaIdentifier);
-      console.log('Cancelled:');
-      return controller.context.cancel(reason);
+      return cancelValue(formulaIdentifier, reason);
     };
 
     /** @type {import('./types.js').Mail['list']} */
diff --git a/packages/daemon/src/mutex.js b/packages/daemon/src/mutex.js
index f222fc54f5..44fdad7bd4 100644
--- a/packages/daemon/src/mutex.js
+++ b/packages/daemon/src/mutex.js
@@ -1,7 +1,7 @@
 import { makeQueue } from '@endo/stream';
 
 /**
- * @returns {{ lock: () => Promise<void>, unlock: () => void, enqueue: (asyncFn: () => Promise<any>) => Promise<any> }}
+ * @returns {import('./types.js').Mutex}
  */
 export const makeMutex = () => {
   /** @type {import('@endo/stream').AsyncQueue<void>} */
@@ -17,8 +17,7 @@ export const makeMutex = () => {
   return {
     lock,
     unlock,
-    // helper for correct usage
-    enqueue: async asyncFn => {
+    enqueue: async (asyncFn = /** @type {any} */ (async () => {})) => {
       await lock();
       try {
         return await asyncFn();
diff --git a/packages/daemon/src/types.d.ts b/packages/daemon/src/types.d.ts
index 0da63437ab..90d48ebee0 100644
--- a/packages/daemon/src/types.d.ts
+++ b/packages/daemon/src/types.d.ts
@@ -291,6 +291,10 @@ export type ProvideControllerForFormulaIdentifier = (
 export type ProvideControllerForFormulaIdentifierAndResolveHandle = (
   formulaIdentifier: string,
 ) => Promise<Controller>;
+export type CancelValue = (
+  formulaIdentifier: string,
+  reason: Error,
+) => Promise<void>;
 
 /**
  * A handle is used to create a pointer to a formula without exposing it directly.
@@ -632,3 +636,9 @@ export type DaemonicPowers = {
   persistence: DaemonicPersistencePowers;
   control: DaemonicControlPowers;
 };
+
+type Mutex = {
+  lock: () => Promise<void>;
+  unlock: () => void;
+  enqueue: <T>(asyncFn?: () => Promise<T>) => Promise<T>;
+};
diff --git a/packages/daemon/test/test-endo.js b/packages/daemon/test/test-endo.js
index 7f51c2cb53..3fad922c82 100644
--- a/packages/daemon/test/test-endo.js
+++ b/packages/daemon/test/test-endo.js
@@ -847,9 +847,6 @@ test('unconfined service can respond to cancellation', async t => {
     ['caplet'],
     ['context-consumer'],
   );
-  // TODO:cancel This should not be necessary.
-  // eslint-disable-next-line no-undef
-  await new Promise(resolve => setTimeout(resolve, 100));
   await E(host).cancel('context-consumer');
   t.is(await result, 'cancelled');
 });
@@ -883,9 +880,6 @@ test('confined service can respond to cancellation', async t => {
     ['caplet'],
     ['context-consumer'],
   );
-  // TODO:cancel This should not be necessary.
-  // eslint-disable-next-line no-undef
-  await new Promise(resolve => setTimeout(resolve, 100));
   await E(host).cancel('context-consumer');
   t.is(await result, 'cancelled');
 });