refactor(daemon): Synchronize provideWorker()

endojs · Mar 14, 2024 · 9a3ce97 · 9a3ce97
1 parent 55b5909
commit 9a3ce97
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 27 deletions.
diff --git a/packages/daemon/src/daemon.js b/packages/daemon/src/daemon.js
@@ -1011,9 +1011,22 @@ const makeDaemonCore = async (
   /**
    * @type {import('./types.js').DaemonCore['incarnateWorker']}
    */
-  const incarnateWorker = async () => {
-    const formulaNumber = await formulaGraphJobs.enqueue(randomHex512);
-    return incarnateNumberedWorker(formulaNumber);
+  const incarnateWorker = async deferredTasks => {
+    return incarnateNumberedWorker(
+      await formulaGraphJobs.enqueue(async () => {
+        const formulaNumber = await randomHex512();
+
+        await deferredTasks.execute({
+          workerFormulaIdentifier: formatId({
+            type: 'worker',
+            number: formulaNumber,
+            node: ownNodeIdentifier,
+          }),
+        });
+
+        return formulaNumber;
+      }),
+    );
   };
 
   /**

diff --git a/packages/daemon/src/host.js b/packages/daemon/src/host.js
@@ -122,27 +122,27 @@ export const makeHostMaker = ({
      * @param {string} workerName
      */
     const provideWorker = async workerName => {
-      if (typeof workerName !== 'string') {
-        throw new Error('worker name must be string');
-      }
-      let workerFormulaIdentifier = petStore.identifyLocal(workerName);
-      if (workerFormulaIdentifier === undefined) {
-        ({ formulaIdentifier: workerFormulaIdentifier } =
-          await incarnateWorker());
-        assertPetName(workerName);
-        await petStore.write(workerName, workerFormulaIdentifier);
-      } else if (!workerFormulaIdentifier.startsWith('worker:')) {
-        throw new Error(`Not a worker ${q(workerName)}`);
-      }
-      return /** @type {Promise<import('./types.js').EndoWorker>} */ (
-        // Behold, recursion:
-        // eslint-disable-next-line no-use-before-define
-        provideValueForFormulaIdentifier(workerFormulaIdentifier)
+      /** @type {import('./types.js').DeferredTasks<import('./types.js').WorkerDeferredTaskParams>} */
+      const tasks = makeDeferredTasks();
+      // eslint-disable-next-line no-use-before-define
+      const workerFormulaIdentifier = prepareWorkerFormulaIdentifier(
+        workerName,
+        tasks.push,
       );
+
+      if (workerFormulaIdentifier !== undefined) {
+        return /** @type {Promise<import('./types.js').EndoWorker>} */ (
+          // Behold, recursion:
+          provideValueForFormulaIdentifier(workerFormulaIdentifier)
+        );
+      }
+
+      const { value } = await incarnateWorker(tasks);
+      return value;
     };
 
     /**
-     * @param {string | 'MAIN' | 'NEW'} workerName
+     * @param {string} workerName
      * @param {import('./types.js').DeferredTasks<{ workerFormulaIdentifier: string }>['push']} deferTask
      * @returns {string | undefined}
      */

diff --git a/packages/daemon/src/types.d.ts b/packages/daemon/src/types.d.ts
@@ -75,6 +75,10 @@ type WorkerFormula = {
   type: 'worker';
 };
 
+export type WorkerDeferredTaskParams = {
+  workerFormulaIdentifier: string;
+};
+
 /**
  * Deferred tasks parameters for `host` and `guest` formulas.
  */
@@ -810,7 +814,9 @@ export interface DaemonCore {
   incarnateEndoBootstrap: (
     specifiedFormulaNumber: string,
   ) => IncarnateResult<FarEndoBootstrap>;
-  incarnateWorker: () => IncarnateResult<EndoWorker>;
+  incarnateWorker: (
+    deferredTasks: DeferredTasks<WorkerDeferredTaskParams>,
+  ) => IncarnateResult<EndoWorker>;
   incarnateDirectory: () => IncarnateResult<EndoDirectory>;
   incarnateHost: (
     endoFormulaIdentifier: string,

diff --git a/packages/daemon/test/test-endo.js b/packages/daemon/test/test-endo.js
@@ -150,7 +150,8 @@ test('anonymous spawn and evaluate', async t => {
   await stop(locator);
 });
 
-test('cannot spawn worker with existing non-worker name', async t => {
+// Regression test for https://github.com/endojs/endo/issues/2147
+test.failing('spawning a worker can overwrite existing non-worker name', async t => {
   const { promise: cancelled, reject: cancel } = makePromiseKit();
   t.teardown(() => cancel(Error('teardown')));
   const locator = makeLocator('tmp', 'spawn-eval-name-reuse');
@@ -166,11 +167,15 @@ test('cannot spawn worker with existing non-worker name', async t => {
   );
   const bootstrap = getBootstrap();
   const host = E(bootstrap).host();
-  const ten = await E(host).evaluate('MAIN', '10', [], [], 'ten');
-  t.is(ten, 10);
-  await t.throwsAsync(() => E(host).provideWorker('ten'), {
-    message: 'Not a worker "ten"',
-  });
+  const foo = await E(host).evaluate('MAIN', '10', [], [], 'foo');
+  t.is(foo, 10);
+
+  // This should either overwrite 'foo' with a worker or fail with an expected error,
+  // but instead it silently fails.
+  await E(host).provideWorker('foo');
+  // Here the test fails because we try and fail to use 'foo' as a worker.
+  const bar = await E(host).evaluate('foo', '20', [], [], 'bar');
+  t.is(bar, 20);
 
   await stop(locator);
 });