From a239f100e34c69e2b3ee6c2bf6928e798487c442 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Fri, 8 Mar 2024 10:07:35 -0800 Subject: [PATCH] refactor(daemon): Synchronize host `makeUnconfined()` Synchronizes the host's `makeUnconfined()` per #2086. Refactoring `daemon.js` in support of this goal fixed one bug while revealing another. In particular, #2074 is progressed by enabling indirect cancellation of caplets via their workers. The issue is not resolved since indirect cancellation of caplets via their caplet dependencies still does not work as intended. A new, failing regression test has been added for this specific case. The revealed bug is #2021, which we believed to be fixed by #2092. Rather than fixing the bug, that PR concealed it by always creating a new incarnation of `eval` formula workers, even if they already existed. The regression test for #2021 has been marked as failing, and we will have to find a different solution for it. --- packages/daemon/src/daemon.js | 164 ++++++++++++++++++++++-------- packages/daemon/src/host.js | 47 +++++++-- packages/daemon/src/types.d.ts | 12 ++- packages/daemon/test/doubler.js | 15 +++ packages/daemon/test/test-endo.js | 65 +++++++++++- 5 files changed, 244 insertions(+), 59 deletions(-) create mode 100644 packages/daemon/test/doubler.js diff --git a/packages/daemon/src/daemon.js b/packages/daemon/src/daemon.js index cd5eeb1672..f5c08b3497 100644 --- a/packages/daemon/src/daemon.js +++ b/packages/daemon/src/daemon.js @@ -1007,41 +1007,38 @@ const makeDaemonCore = async ( ); }; - /** @type {import('./types.js').DaemonCore['incarnateGuest']} */ - const incarnateGuest = async (hostFormulaIdentifier, deferredTasks) => { + /** + * Helper for callers of `incarnateNumberedGuest`. + * @param {string} hostFormulaIdentifier - The formula identifier of the host. + */ + const incarnateGuestDependencies = async hostFormulaIdentifier => + harden({ + guestFormulaNumber: await randomHex512(), + hostHandleFormulaIdentifier: ( + await incarnateNumberedHandle( + await randomHex512(), + hostFormulaIdentifier, + ) + ).formulaIdentifier, + storeFormulaIdentifier: ( + await incarnateNumberedPetStore(await randomHex512()) + ).formulaIdentifier, + workerFormulaIdentifier: ( + await incarnateNumberedWorker(await randomHex512()) + ).formulaIdentifier, + }); + + /** + * + * @param {ReturnType} identifiers + */ + const incarnateNumberedGuest = identifiers => { const { guestFormulaNumber, hostHandleFormulaIdentifier, storeFormulaIdentifier, workerFormulaIdentifier, - } = await formulaGraphMutex.enqueue(async () => { - const formulaNumber = await randomHex512(); - const hostHandle = await incarnateNumberedHandle( - await randomHex512(), - hostFormulaIdentifier, - ); - const storeIncarnation = await incarnateNumberedPetStore( - await randomHex512(), - ); - const workerIncarnation = await incarnateNumberedWorker( - await randomHex512(), - ); - - await deferredTasks.execute({ - guestFormulaIdentifier: serializeFormulaIdentifier({ - type: 'guest', - number: formulaNumber, - node: ownNodeIdentifier, - }), - }); - - return harden({ - guestFormulaNumber: formulaNumber, - hostHandleFormulaIdentifier: hostHandle.formulaIdentifier, - storeFormulaIdentifier: storeIncarnation.formulaIdentifier, - workerFormulaIdentifier: workerIncarnation.formulaIdentifier, - }); - }); + } = identifiers; /** @type {import('./types.js').GuestFormula} */ const formula = { @@ -1055,6 +1052,44 @@ const makeDaemonCore = async ( ); }; + /** @type {import('./types.js').DaemonCore['incarnateGuest']} */ + const incarnateGuest = async (hostFormulaIdentifier, deferredTasks) => { + return incarnateNumberedGuest( + await formulaGraphMutex.enqueue(async () => { + const identifiers = await incarnateGuestDependencies( + hostFormulaIdentifier, + ); + + await deferredTasks.execute({ + guestFormulaIdentifier: serializeFormulaIdentifier({ + type: 'guest', + number: identifiers.guestFormulaNumber, + node: ownNodeIdentifier, + }), + }); + + return identifiers; + }), + ); + }; + + /** + * @param {string} [specifiedWorkerFormulaIdentifier] + */ + const provideWorkerFormulaIdentifier = + async specifiedWorkerFormulaIdentifier => { + await null; + if (typeof specifiedWorkerFormulaIdentifier === 'string') { + return specifiedWorkerFormulaIdentifier; + } + + const workerFormulaNumber = await randomHex512(); + const workerIncarnation = await incarnateNumberedWorker( + workerFormulaNumber, + ); + return workerIncarnation.formulaIdentifier; + }; + /** @type {import('./types.js').DaemonCore['incarnateEval']} */ const incarnateEval = async ( nameHubFormulaIdentifier, @@ -1067,7 +1102,7 @@ const makeDaemonCore = async ( const { workerFormulaIdentifier, endowmentFormulaIdentifiers, - evalFormulaIdentifier, + evalFormulaNumber, } = await formulaGraphMutex.enqueue(async () => { const ownFormulaNumber = await randomHex512(); const ownFormulaIdentifier = serializeFormulaIdentifier({ @@ -1075,14 +1110,11 @@ const makeDaemonCore = async ( number: ownFormulaNumber, node: ownNodeIdentifier, }); - const workerFormulaNumber = await (specifiedWorkerFormulaIdentifier - ? parseFormulaIdentifier(specifiedWorkerFormulaIdentifier).number - : randomHex512()); const identifiers = harden({ - workerFormulaIdentifier: ( - await incarnateNumberedWorker(workerFormulaNumber) - ).formulaIdentifier, + workerFormulaIdentifier: await provideWorkerFormulaIdentifier( + specifiedWorkerFormulaIdentifier, + ), endowmentFormulaIdentifiers: await Promise.all( endowmentFormulaIdsOrPaths.map(async formulaIdOrPath => { if (typeof formulaIdOrPath === 'string') { @@ -1102,15 +1134,13 @@ const makeDaemonCore = async ( }), ), evalFormulaIdentifier: ownFormulaIdentifier, + evalFormulaNumber: ownFormulaNumber, }); await deferredTasks.execute(identifiers); return identifiers; }); - const { number: evalFormulaNumber } = parseFormulaIdentifier( - evalFormulaIdentifier, - ); /** @type {import('./types.js').EvalFormula} */ const formula = { type: 'eval', @@ -1151,18 +1181,64 @@ const makeDaemonCore = async ( ); }; + /** + * @param {string} hostFormulaIdentifier + * @param {string} [specifiedPowersFormulaIdentifier] + */ + const providePowersFormulaIdentifier = async ( + hostFormulaIdentifier, + specifiedPowersFormulaIdentifier, + ) => { + await null; + if (typeof specifiedPowersFormulaIdentifier === 'string') { + return specifiedPowersFormulaIdentifier; + } + + const guestIncarnationData = await incarnateGuestDependencies( + hostFormulaIdentifier, + ); + const guestIncarnation = await incarnateNumberedGuest(guestIncarnationData); + return guestIncarnation.formulaIdentifier; + }; + /** @type {import('./types.js').DaemonCore['incarnateUnconfined']} */ const incarnateUnconfined = async ( - workerFormulaIdentifier, - powersFormulaIdentifiers, + hostFormulaIdentifier, specifier, + deferredTasks, + specifiedWorkerFormulaIdentifier, + specifiedPowersFormulaIdentifier, ) => { - const formulaNumber = await randomHex512(); + const { + powersFormulaIdentifier, + unconfinedFormulaNumber: formulaNumber, + workerFormulaIdentifier, + } = await formulaGraphMutex.enqueue(async () => { + const ownFormulaNumber = await randomHex512(); + const identifiers = harden({ + powersFormulaIdentifier: await providePowersFormulaIdentifier( + hostFormulaIdentifier, + specifiedPowersFormulaIdentifier, + ), + unconfinedFormulaIdentifier: serializeFormulaIdentifier({ + type: 'make-unconfined', + number: ownFormulaNumber, + node: ownNodeIdentifier, + }), + unconfinedFormulaNumber: ownFormulaNumber, + workerFormulaIdentifier: await provideWorkerFormulaIdentifier( + specifiedWorkerFormulaIdentifier, + ), + }); + await deferredTasks.execute(identifiers); + return identifiers; + }); + /** @type {import('./types.js').MakeUnconfinedFormula} */ const formula = { type: 'make-unconfined', worker: workerFormulaIdentifier, - powers: powersFormulaIdentifiers, + powers: powersFormulaIdentifier, specifier, }; return /** @type {import('./types.js').IncarnateResult} */ ( diff --git a/packages/daemon/src/host.js b/packages/daemon/src/host.js index 82f4a14a71..931a551c5b 100644 --- a/packages/daemon/src/host.js +++ b/packages/daemon/src/host.js @@ -9,6 +9,11 @@ import { parseFormulaIdentifier } from './formula-identifier.js'; const { quote: q } = assert; +/** @param {string} name */ +const assertPowersName = name => { + ['NONE', 'SELF', 'ENDO'].includes(name) || assertPetName(name); +}; + /** * @param {object} args * @param {import('./types.js').DaemonCore['provideValueForFormulaIdentifier']} args.provideValueForFormulaIdentifier @@ -259,7 +264,6 @@ export const makeHostMaker = ({ deferTask(identifiers => petStore.write(workerName, identifiers.workerFormulaIdentifier), ); - return undefined; } return workerFormulaIdentifier; }; @@ -283,6 +287,21 @@ export const makeHostMaker = ({ return guestFormulaIdentifier; }; + /** + * @param {string | 'NONE' | 'SELF' | 'ENDO'} partyName + * @param {import('./types.js').DeferredTasks<{ powersFormulaIdentifier: string }>['push']} deferTask + * @returns {string | undefined} + */ + const providePowersFormulaIdentifierSync = (partyName, deferTask) => { + const powersFormulaIdentifier = petStore.identifyLocal(partyName); + if (powersFormulaIdentifier === undefined) { + deferTask(identifiers => + petStore.write(partyName, identifiers.powersFormulaIdentifier), + ); + } + return powersFormulaIdentifier; + }; + /** * @param {string | 'MAIN' | 'NEW'} workerName * @param {string} source @@ -356,24 +375,36 @@ export const makeHostMaker = ({ powersName, resultName, ) => { - const workerFormulaIdentifier = await provideWorkerFormulaIdentifier( + assertPowersName(powersName); + + /** @type {import('./types.js').DeferredTasks} */ + const tasks = makeDeferredTasks(); + + const workerFormulaIdentifier = provideWorkerFormulaIdentifierSync( workerName, + tasks.push, ); - const powersFormulaIdentifier = await providePowersFormulaIdentifier( + const powersFormulaIdentifier = providePowersFormulaIdentifierSync( powersName, + tasks.push, ); + if (resultName !== undefined) { + tasks.push(identifiers => + petStore.write(resultName, identifiers.unconfinedFormulaIdentifier), + ); + } + // Behold, recursion: // eslint-disable-next-line no-use-before-define - const { formulaIdentifier, value } = await incarnateUnconfined( + const { value } = await incarnateUnconfined( + hostFormulaIdentifier, + specifier, + tasks, workerFormulaIdentifier, powersFormulaIdentifier, - specifier, ); - if (resultName !== undefined) { - await petStore.write(resultName, formulaIdentifier); - } return value; }; diff --git a/packages/daemon/src/types.d.ts b/packages/daemon/src/types.d.ts index c6b02dcbd9..111311e8fd 100644 --- a/packages/daemon/src/types.d.ts +++ b/packages/daemon/src/types.d.ts @@ -143,6 +143,12 @@ type MakeUnconfinedFormula = { // TODO formula slots }; +export type MakeUnconfinedDeferredTaskParams = { + powersFormulaIdentifier: string; + unconfinedFormulaIdentifier: string; + workerFormulaIdentifier: string; +}; + type MakeBundleFormula = { type: 'make-bundle'; worker: string; @@ -771,9 +777,11 @@ export interface DaemonCore { specifiedWorkerFormulaIdentifier?: string, ) => IncarnateResult; incarnateUnconfined: ( - workerFormulaIdentifier: string, - powersFormulaIdentifier: string, + hostFormulaIdentifier: string, specifier: string, + deferredTasks: DeferredTasks, + specifiedWorkerFormulaIdentifier?: string, + specifiedPowersFormulaIdentifier?: string, ) => IncarnateResult; incarnateBundler: ( powersFormulaIdentifier: string, diff --git a/packages/daemon/test/doubler.js b/packages/daemon/test/doubler.js new file mode 100644 index 0000000000..514cb266b2 --- /dev/null +++ b/packages/daemon/test/doubler.js @@ -0,0 +1,15 @@ +import { E, Far } from '@endo/far'; + +export const make = powers => { + const counter = E(powers).request( + 'HOST', + 'a counter, suitable for doubling', + 'my-counter', + ); + return Far('Doubler', { + async incr() { + const n = await E(counter).incr(); + return n * 2; + }, + }); +}; diff --git a/packages/daemon/test/test-endo.js b/packages/daemon/test/test-endo.js index 4f3290a2cf..d8e3aa412f 100644 --- a/packages/daemon/test/test-endo.js +++ b/packages/daemon/test/test-endo.js @@ -675,12 +675,12 @@ test('direct cancellation', async t => { ); }); -// See: https://github.com/endojs/endo/issues/2074 -test.failing('indirect cancellation', async t => { +// Regression test 1 for https://github.com/endojs/endo/issues/2074 +test('indirect cancellation via worker', async t => { const { promise: cancelled, reject: cancel } = makePromiseKit(); t.teardown(() => cancel(Error('teardown'))); - const locator = makeLocator('tmp', 'cancellation-indirect'); + const locator = makeLocator('tmp', 'cancellation-indirect-worker'); await start(locator); t.teardown(() => stop(locator)); @@ -756,6 +756,61 @@ test.failing('indirect cancellation', async t => { ); }); +// Regression test 2 for https://github.com/endojs/endo/issues/2074 +test.failing('indirect cancellation via caplet', async t => { + const { promise: cancelled, reject: cancel } = makePromiseKit(); + t.teardown(() => cancel(Error('teardown'))); + + const locator = makeLocator('tmp', 'cancellation-indirect-caplet'); + + await start(locator); + t.teardown(() => stop(locator)); + + const { getBootstrap } = await makeEndoClient( + 'client', + locator.sockPath, + cancelled, + ); + const bootstrap = getBootstrap(); + const host = E(bootstrap).host(); + + await E(host).provideWorker('w1'); + const counterPath = path.join(dirname, 'test', 'counter.js'); + const counterLocation = url.pathToFileURL(counterPath).href; + await E(host).makeUnconfined('w1', counterLocation, 'SELF', 'counter'); + + await E(host).provideWorker('w2'); + await E(host).provideGuest('guest'); + const doublerPath = path.join(dirname, 'test', 'doubler.js'); + const doublerLocation = url.pathToFileURL(doublerPath).href; + await E(host).makeUnconfined('w2', doublerLocation, 'guest', 'doubler'); + E(host).resolve(0, 'counter'); + + t.is( + 1, + await E(host).evaluate('w1', 'E(counter).incr()', ['counter'], ['counter']), + ); + t.is( + 4, + await E(host).evaluate('w2', 'E(doubler).incr()', ['doubler'], ['doubler']), + ); + t.is( + 6, + await E(host).evaluate('w2', 'E(doubler).incr()', ['doubler'], ['doubler']), + ); + + await E(host).cancel('counter'); + + t.is( + 1, + await E(host).evaluate('w1', 'E(counter).incr()', ['counter'], ['counter']), + ); + t.is( + 4, + await E(host).evaluate('w2', 'E(doubler).incr()', ['doubler'], ['doubler']), + ); +}); + test('cancel because of requested capability', async t => { const { promise: cancelled, reject: cancel } = makePromiseKit(); t.teardown(() => cancel(Error('teardown'))); @@ -979,8 +1034,8 @@ test('name and reuse inspector', async t => { await stop(locator); }); -// Regression test for: https://github.com/endojs/endo/issues/2021 -test('eval-mediated worker name', async t => { +// Regression test for https://github.com/endojs/endo/issues/2021 +test.failing('eval-mediated worker name', async t => { const { promise: cancelled, reject: cancel } = makePromiseKit(); t.teardown(() => cancel(Error('teardown'))); const locator = makeLocator('tmp', 'eval-worker-name');