From ee710caee3ef62aea618576dea99ffc8090cb594 Mon Sep 17 00:00:00 2001 From: Mathieu Hofman Date: Thu, 3 Oct 2024 18:11:22 +0000 Subject: [PATCH] feat(async-flow)!: require explicit wakeAll after flows redefined --- packages/async-flow/src/async-flow.js | 16 ++-- .../test/async-flow-early-completion.test.js | 2 + .../async-flow/test/async-flow-wake.test.js | 96 +++++++++---------- packages/async-flow/test/async-flow.test.js | 4 + packages/async-flow/test/bad-host.test.js | 2 + .../async-flow/test/prepare-test-env-ava.js | 5 + .../orchestration/src/utils/start-helper.js | 4 +- .../test/facade-durability.test.ts | 6 ++ packages/orchestration/test/facade.test.ts | 20 +++- 9 files changed, 91 insertions(+), 64 deletions(-) diff --git a/packages/async-flow/src/async-flow.js b/packages/async-flow/src/async-flow.js index 1419f7881ba..f6bdba1388d 100644 --- a/packages/async-flow/src/async-flow.js +++ b/packages/async-flow/src/async-flow.js @@ -537,6 +537,10 @@ export const prepareAsyncFlowTools = (outerZone, outerOptions = {}) => { getFailures() { return failures.snapshot(); }, + /** + * Must be called once on upgrade during the start crank, but after all + * async flows have been redefined + */ wakeAll() { wakeUpgradeGate(); // [...stuff.keys()] in order to snapshot before iterating @@ -550,22 +554,16 @@ export const prepareAsyncFlowTools = (outerZone, outerOptions = {}) => { } }, getFlowForOutcomeVow(outcomeVow) { - return flowForOutcomeVowKey.get(toPassableCap(outcomeVow)); + return /** @type {AsyncFlow} */ ( + flowForOutcomeVowKey.get(toPassableCap(outcomeVow)) + ); }, }); - // Cannot call this until everything is prepared, so postpone to a later - // turn. (Ideally, we'd postpone to a later crank because prepares are - // allowed anytime in the first crank. But there's currently no pleasant - // way to postpone to a later crank.) - // See https://github.com/Agoric/agoric-sdk/issues/9377 - const allWokenP = E.when(null, () => adminAsyncFlow.wakeAll()); - return harden({ prepareAsyncFlowKit, asyncFlow, adminAsyncFlow, - allWokenP, prepareEndowment, }); }; diff --git a/packages/async-flow/test/async-flow-early-completion.test.js b/packages/async-flow/test/async-flow-early-completion.test.js index f4324e3ef95..8c84236a357 100644 --- a/packages/async-flow/test/async-flow-early-completion.test.js +++ b/packages/async-flow/test/async-flow-early-completion.test.js @@ -96,6 +96,7 @@ const testFirstPlay = async (t, zone) => { }; const wrapperFunc = asyncFlow(zone, 'AsyncFlow1', guestMethod); + await adminAsyncFlow.wakeAll(); const outcomeV = zone.makeOnce('outcomeV', () => wrapperFunc(hOrch7, v1, v3)); @@ -175,6 +176,7 @@ const testBadShortReplay = async (t, zone, rejection) => { // invoked, that would be a *new* activation with a new outcome and // flow, and would have nothing to do with the existing one. asyncFlow(zone, 'AsyncFlow1', guestMethod); + await adminAsyncFlow.wakeAll(); const outcomeV = /** @type {Vow} */ ( zone.makeOnce('outcomeV', () => Fail`need outcomeV`) diff --git a/packages/async-flow/test/async-flow-wake.test.js b/packages/async-flow/test/async-flow-wake.test.js index ddf168eba7c..86d47a94252 100644 --- a/packages/async-flow/test/async-flow-wake.test.js +++ b/packages/async-flow/test/async-flow-wake.test.js @@ -122,55 +122,27 @@ testAsyncLife( }, ); -testAsyncLife.failing( - 'failed wake', - async (t, { zone, asyncFlow, makeVowKit, allWokenP }) => { - t.plan(2); - // Spend a bunch of turns to pretend any concurrent async operation has settled - // Triggers https://github.com/Agoric/agoric-sdk/issues/9377 - for (let i = 0; i < 100; i += 1) { - await null; - } - - makeTestKit({ zone, makeVowKit }); - const guestFunc = async w => { - t.pass('not triggered - invocation cannot be awoken'); - return w.wait('foo'); - }; - t.notThrows(() => - asyncFlow(zone, 'guestFunc', guestFunc, { - // Next incarnation should not start eager - startEager: false, - }), - ); - - return { allWokenP }; - }, - async (t, { allWokenP }) => { - await t.notThrowsAsync( - () => allWokenP, - 'will actually throw due to crank bug #9377', - ); - }, -); - -testAsyncLife( - 'failed wake redo', - async (t, { zone, asyncFlow, makeVowKit }) => { - t.plan(2); - makeTestKit({ zone, makeVowKit }); - const guestFunc = async w => { - t.pass(); - return w.wait('foo'); - }; - t.notThrows(() => - asyncFlow(zone, 'guestFunc', guestFunc, { - // Next incarnation should not start eager - startEager: false, - }), - ); - }, -); +testAsyncLife('failed wake', async (t, { zone, asyncFlow, makeVowKit }) => { + t.plan(2); + // Spend a bunch of turns to pretend any concurrent async operation has settled + // Previously the async-flows would be awoken one turn after preparation of the tools + // which would prevent asynchronous definitions. + for (let i = 0; i < 100; i += 1) { + await null; + } + + makeTestKit({ zone, makeVowKit }); + const guestFunc = async w => { + t.pass(); + return w.wait('foo'); + }; + t.notThrows(() => + asyncFlow(zone, 'guestFunc', guestFunc, { + // Next incarnation should not start eager + startEager: false, + }), + ); +}); testAsyncLife( 'not eager waker stay sleeping 3', @@ -217,3 +189,29 @@ testAsyncLife( t.is(guestCalled.tripped, true, 'flow woke up'); }, ); + +testAsyncLife( + 'no wakeAll causes start failure', + async (t, { zone, asyncFlow, makeVowKit }) => { + // Not reconnected handler invoked + t.plan(2); + makeTestKit({ zone, makeVowKit }); + const guestFunc = async _ => t.fail(`Should not restart`); + + t.notThrows(() => asyncFlow(zone, 'guestFunc', guestFunc)); + }, + undefined, + { + skipWakeAll: true, + notAllKindsReconnectedHandler: (e, t) => { + t.throws( + () => { + throw e; + }, + { + message: 'defineDurableKind not called for tags: [WakeGateSentinel]', + }, + ); + }, + }, +); diff --git a/packages/async-flow/test/async-flow.test.js b/packages/async-flow/test/async-flow.test.js index 91cf31e0385..ea7b8c1beae 100644 --- a/packages/async-flow/test/async-flow.test.js +++ b/packages/async-flow/test/async-flow.test.js @@ -99,6 +99,7 @@ const testFirstPlay = async (t, zone) => { }; const wrapperFunc = asyncFlow(zone, 'AsyncFlow1', guestMethod); + await adminAsyncFlow.wakeAll(); const outcomeV = zone.makeOnce('outcomeV', () => wrapperFunc(hOrch7, v1, v3)); @@ -174,6 +175,7 @@ const testBadReplay = async (t, zone) => { // invoked, that would be a *new* activation with a new outcome and // flow, and would have nothing to do with the existing one. asyncFlow(zone, 'AsyncFlow1', guestMethod); + await adminAsyncFlow.wakeAll(); const outcomeV = /** @type {Vow} */ ( zone.makeOnce('outcomeV', () => Fail`need outcomeV`) @@ -258,6 +260,7 @@ const testGoodReplay = async (t, zone) => { // invoked, that would be a *new* activation with a new outcome and // flow, and would have nothing to do with the existing one. asyncFlow(zone, 'AsyncFlow1', guestMethod); + await adminAsyncFlow.wakeAll(); const outcomeV = /** @type {Vow} */ ( zone.makeOnce('outcomeV', () => Fail`need outcomeV`) @@ -329,6 +332,7 @@ const testAfterPlay = async (t, zone) => { // invoked, that would be a *new* activation with a new outcome and // flow, and would have nothing to do with the existing one. asyncFlow(zone, 'AsyncFlow1', guestMethod); + await adminAsyncFlow.wakeAll(); const outcomeV = /** @type {Vow} */ ( zone.makeOnce('outcomeV', () => Fail`need outcomeV`) diff --git a/packages/async-flow/test/bad-host.test.js b/packages/async-flow/test/bad-host.test.js index c3b8cf7c3fe..2bb81a2cbdf 100644 --- a/packages/async-flow/test/bad-host.test.js +++ b/packages/async-flow/test/bad-host.test.js @@ -87,6 +87,7 @@ const testBadHostFirstPlay = async (t, zone) => { }; const wrapperFunc = asyncFlow(zone, 'AsyncFlow1', guestMethod); + await adminAsyncFlow.wakeAll(); const badHost = zone.makeOnce('badHost', () => makeBadHost()); @@ -166,6 +167,7 @@ const testBadHostReplay1 = async (t, zone) => { }; asyncFlow(zone, 'AsyncFlow1', guestMethod); + await adminAsyncFlow.wakeAll(); const badHost = zone.makeOnce('badHost', () => Fail`need badHost`); diff --git a/packages/async-flow/test/prepare-test-env-ava.js b/packages/async-flow/test/prepare-test-env-ava.js index 1c289227031..9f1552c6869 100644 --- a/packages/async-flow/test/prepare-test-env-ava.js +++ b/packages/async-flow/test/prepare-test-env-ava.js @@ -27,6 +27,7 @@ export const asyncFlowVerbose = () => { * @typedef {Omit[2], 'notAllKindsReconnectedHandler'> & { * notAllKindsReconnectedHandler?: (e: Error, t: ExecutionContext) => void; * panicHandler?: import('./_utils.js').TestAsyncFlowPanicHandler; + * skipWakeAll?: boolean; * }} StartAsyncLifeOptions */ @@ -43,6 +44,7 @@ export const startAsyncLife = async ( run, { panicHandler, + skipWakeAll, notAllKindsReconnectedHandler: notAllKindsReconnectedHandlerOriginal, ...startOptions } = {}, @@ -59,6 +61,9 @@ export const startAsyncLife = async ( zone: rootZone.subZone('contract'), ...asyncFlowTools, }); + if (!skipWakeAll) { + await asyncFlowTools.adminAsyncFlow.wakeAll(); + } return tools; }, run, diff --git a/packages/orchestration/src/utils/start-helper.js b/packages/orchestration/src/utils/start-helper.js index 16026e79861..6fbad11b589 100644 --- a/packages/orchestration/src/utils/start-helper.js +++ b/packages/orchestration/src/utils/start-helper.js @@ -218,6 +218,8 @@ export const withOrchestration = privateArgs, privateArgs.marshaller, ); - return contractFn(zcf, privateArgs, zone, tools); + const result = await contractFn(zcf, privateArgs, zone, tools); + await tools.asyncFlowTools.adminAsyncFlow.wakeAll(); + return result; }; harden(withOrchestration); diff --git a/packages/orchestration/test/facade-durability.test.ts b/packages/orchestration/test/facade-durability.test.ts index 3952ec59c7e..67c489e9619 100644 --- a/packages/orchestration/test/facade-durability.test.ts +++ b/packages/orchestration/test/facade-durability.test.ts @@ -76,6 +76,8 @@ test.serial('chain info', async t => { return orc.getChain('mock'); }); + await orchKit.asyncFlowTools.adminAsyncFlow.wakeAll(); + const result = (await vt.when(handle())) as Chain; t.deepEqual(await vt.when(result.getChainInfo()), mockChainInfo); }); @@ -122,6 +124,8 @@ test.serial('faulty chain info', async t => { return account; }); + await orchKit.asyncFlowTools.adminAsyncFlow.wakeAll(); + await t.throwsAsync(vt.when(handle()), { message: 'chain info lacks staking denom', }); @@ -209,6 +213,8 @@ test.serial('asset / denom info', async t => { }, ); + await orchKit.asyncFlowTools.adminAsyncFlow.wakeAll(); + await vt.when(handle()); chainHub.registerChain('anotherChain', mockChainInfo); diff --git a/packages/orchestration/test/facade.test.ts b/packages/orchestration/test/facade.test.ts index 728e38016d5..5941933eb3e 100644 --- a/packages/orchestration/test/facade.test.ts +++ b/packages/orchestration/test/facade.test.ts @@ -1,4 +1,3 @@ -/* eslint-disable @jessie.js/safe-await-separator */ import { test as anyTest } from '@agoric/zoe/tools/prepare-test-env-ava.js'; import type { VowTools } from '@agoric/vow'; @@ -10,7 +9,12 @@ import type { OrchestrationFlow } from '../src/orchestration-api.js'; import { provideOrchestration } from '../src/utils/start-helper.js'; import { commonSetup } from './supports.js'; -const test = anyTest as TestFn<{ vt: VowTools; orchestrateAll: any; zcf: ZCF }>; +const test = anyTest as TestFn<{ + vt: VowTools; + orchestrateAll: any; + zcf: ZCF; + wakeAll: () => Promise; +}>; test.beforeEach(async t => { const { facadeServices, commonPrivateArgs } = await commonSetup(t); @@ -32,11 +36,13 @@ test.beforeEach(async t => { ); const { orchestrateAll } = orchKit; - t.context = { vt, orchestrateAll, zcf }; + const wakeAll = async () => orchKit.asyncFlowTools.adminAsyncFlow.wakeAll(); + + t.context = { vt, orchestrateAll, zcf, wakeAll }; }); test('calls between flows', async t => { - const { vt, orchestrateAll, zcf } = t.context; + const { vt, orchestrateAll, zcf, wakeAll } = t.context; const flows = { outer(orch, ctx, ...recipients) { @@ -51,12 +57,14 @@ test('calls between flows', async t => { peerFlows: flows, }); + await wakeAll(); + t.deepEqual(await vt.when(inner('a', 'b', 'c')), 'a b c'); t.deepEqual(await vt.when(outer('a', 'b', 'c')), 'Hello a b c'); }); test('context mapping individual flows', async t => { - const { vt, orchestrateAll, zcf } = t.context; + const { vt, orchestrateAll, zcf, wakeAll } = t.context; const flows = { outer(orch, ctx, ...recipients) { @@ -71,5 +79,7 @@ test('context mapping individual flows', async t => { peerFlows: { inner: flows.inner }, }); + await wakeAll(); + t.deepEqual(await vt.when(outer('a', 'b', 'c')), 'Hello a b c'); });