diff --git a/packages/daemon/src/daemon.js b/packages/daemon/src/daemon.js index b929dac396..3dc5053175 100644 --- a/packages/daemon/src/daemon.js +++ b/packages/daemon/src/daemon.js @@ -13,6 +13,7 @@ import { makeHostMaker } from './host.js'; import { assertPetName } from './pet-name.js'; import { makeContextMaker } from './context.js'; import { parseFormulaIdentifier } from './formula-identifier.js'; +import { makeMutex } from './mutex.js'; const delay = async (ms, cancelled) => { // Do not attempt to set up a timer if already cancelled. @@ -49,7 +50,6 @@ const makeInspector = (type, number, record) => * @param {import('./types.js').DaemonicPowers} powers * @param {Promise} webletPortP * @param {object} args - * @param {Promise} args.cancelled * @param {(error: Error) => void} args.cancel * @param {number} args.gracePeriodMs * @param {Promise} args.gracePeriodElapsed @@ -57,7 +57,7 @@ const makeInspector = (type, number, record) => const makeDaemonCore = async ( powers, webletPortP, - { cancelled, cancel, gracePeriodMs, gracePeriodElapsed }, + { cancel, gracePeriodMs, gracePeriodElapsed }, ) => { const { crypto: cryptoPowers, @@ -67,6 +67,7 @@ const makeDaemonCore = async ( } = powers; const { randomHex512 } = cryptoPowers; const contentStore = persistencePowers.makeContentSha512Store(); + const formulaGraphMutex = makeMutex(); /** * The two functions "provideValueForNumberedFormula" and "provideValueForFormulaIdentifier" @@ -336,7 +337,7 @@ const makeDaemonCore = async ( * @param {import('./types.js').Formula} formula * @param {import('./types.js').Context} context */ - const makeControllerForFormula = async ( + const makeControllerForFormula = ( formulaIdentifier, formulaNumber, formula, @@ -550,7 +551,7 @@ const makeDaemonCore = async ( // Memoize for lookup. console.log(`Making ${formulaIdentifier}`); - const { promise: partial, resolve } = + const { promise: partial, resolve: resolvePartial } = /** @type {import('@endo/promise-kit').PromiseKit>} */ ( makePromiseKit() ); @@ -571,15 +572,23 @@ const makeDaemonCore = async ( }); controllerForFormulaIdentifier.set(formulaIdentifier, controller); - await persistencePowers.writeFormula(formula, formulaType, formulaNumber); - resolve( - makeControllerForFormula( - formulaIdentifier, - formulaNumber, - formula, - context, - ), + // The controller _must_ be constructed in the synchronous prelude of this function. + const controllerValue = makeControllerForFormula( + formulaIdentifier, + formulaNumber, + formula, + context, + ); + + // Ensure that failure to flush the formula to storage + // causes a rejection for both the controller and the incarnation value. + const written = persistencePowers.writeFormula( + formula, + formulaType, + formulaNumber, ); + resolvePartial(written.then(() => /** @type {any} */ (controllerValue))); + await written; return harden({ formulaIdentifier, @@ -622,15 +631,18 @@ const makeDaemonCore = async ( }; /** @type {import('./types.js').ProvideValueForFormulaIdentifier} */ - const provideValueForFormulaIdentifier = async formulaIdentifier => { + const provideValueForFormulaIdentifier = formulaIdentifier => { const controller = /** @type {import('./types.js').Controller<>} */ ( provideControllerForFormulaIdentifier(formulaIdentifier) ); - const value = await controller.external; - if (typeof value === 'object' && value !== null) { - formulaIdentifierForRef.set(value, formulaIdentifier); - } - return value; + return controller.external.then(value => { + // Release the value to the public only after ensuring + // we can reverse-lookup its nonce. + if (typeof value === 'object' && value !== null) { + formulaIdentifierForRef.set(value, formulaIdentifier); + } + return value; + }); }; /** @type {import('./types.js').ProvideControllerForFormulaIdentifierAndResolveHandle} */ @@ -716,6 +728,23 @@ const makeDaemonCore = async ( ); }; + /** + * Incarnates a `worker` formula and synchronously adds it to the formula graph. + * The returned promise is resolved after the formula is persisted. + * @param {string} formulaNumber - The worker formula number. + * @returns {Promise<{ formulaIdentifier: string, value: import('./types').EndoWorker }>} + */ + const incarnateNumberedWorker = formulaNumber => { + /** @type {import('./types.js').WorkerFormula} */ + const formula = { + type: 'worker', + }; + + return /** @type {Promise<{ formulaIdentifier: string, value: import('./types').EndoWorker }>} */ ( + provideValueForNumberedFormula(formula.type, formulaNumber, formula) + ); + }; + /** * @param {string} endoFormulaIdentifier * @param {string} leastAuthorityFormulaIdentifier @@ -775,19 +804,61 @@ const makeDaemonCore = async ( }; /** - * @param {string} workerFormulaIdentifier + * @param {string} hostFormulaIdentifier * @param {string} source * @param {string[]} codeNames - * @param {string[]} endowmentFormulaIdentifiers + * @param {(string | string[])[]} endowmentFormulaIdsOrPaths + * @param {import('./types.js').EvalFormulaHook[]} hooks + * @param {string} [specifiedWorkerFormulaIdentifier] * @returns {Promise<{ formulaIdentifier: string, value: unknown }>} */ const incarnateEval = async ( - workerFormulaIdentifier, + hostFormulaIdentifier, source, codeNames, - endowmentFormulaIdentifiers, + endowmentFormulaIdsOrPaths, + hooks, + specifiedWorkerFormulaIdentifier, ) => { - const formulaNumber = await randomHex512(); + const { + workerFormulaIdentifier, + endowmentFormulaIdentifiers, + evalFormulaNumber, + } = await formulaGraphMutex.enqueue(async () => { + const ownFormulaNumber = await randomHex512(); + const workerFormulaNumber = await (specifiedWorkerFormulaIdentifier + ? parseFormulaIdentifier(specifiedWorkerFormulaIdentifier).number + : randomHex512()); + + const identifiers = harden({ + workerFormulaIdentifier: ( + await incarnateNumberedWorker(workerFormulaNumber) + ).formulaIdentifier, + endowmentFormulaIdentifiers: await Promise.all( + endowmentFormulaIdsOrPaths.map(async formulaIdOrPath => { + if (typeof formulaIdOrPath === 'string') { + return formulaIdOrPath; + } + return ( + /* eslint-disable no-use-before-define */ + ( + await incarnateNumberedLookup( + await randomHex512(), + hostFormulaIdentifier, + formulaIdOrPath, + ) + ).formulaIdentifier + /* eslint-enable no-use-before-define */ + ); + }), + ), + evalFormulaNumber: ownFormulaNumber, + }); + + await Promise.all(hooks.map(hook => hook(identifiers))); + return identifiers; + }); + /** @type {import('./types.js').EvalFormula} */ const formula = { type: 'eval', @@ -797,26 +868,33 @@ const makeDaemonCore = async ( values: endowmentFormulaIdentifiers, }; return /** @type {Promise<{ formulaIdentifier: string, value: unknown }>} */ ( - provideValueForNumberedFormula(formula.type, formulaNumber, formula) + provideValueForNumberedFormula(formula.type, evalFormulaNumber, formula) ); }; /** - * @param {string} hubFormulaIdentifier - * A "naming hub" is an objected with a variadic lookup method. It includes - * objects such as guests and hosts. - * @param {string[]} petNamePath - * @returns {Promise<{ formulaIdentifier: string, value: unknown }>} + * Incarnates a `lookup` formula and synchronously adds it to the formula graph. + * The returned promise is resolved after the formula is persisted. + * @param {string} formulaNumber - The lookup formula's number. + * @param {string} hubFormulaIdentifier - The formula identifier of the naming + * hub to call `lookup` on. A "naming hub" is an objected with a variadic + * lookup method. It includes objects such as guests and hosts. + * @param {string[]} petNamePath - The pet name path to look up. + * @returns {Promise<{ formulaIdentifier: string, value: import('./types').EndoWorker }>} */ - const incarnateLookup = async (hubFormulaIdentifier, petNamePath) => { - const formulaNumber = await randomHex512(); + const incarnateNumberedLookup = ( + formulaNumber, + hubFormulaIdentifier, + petNamePath, + ) => { /** @type {import('./types.js').LookupFormula} */ const formula = { type: 'lookup', hub: hubFormulaIdentifier, path: petNamePath, }; - return /** @type {Promise<{ formulaIdentifier: string, value: unknown }>} */ ( + + return /** @type {Promise<{ formulaIdentifier: string, value: import('./types.js').EndoWorker }>} */ ( provideValueForNumberedFormula(formula.type, formulaNumber, formula) ); }; @@ -938,11 +1016,11 @@ const makeDaemonCore = async ( }; /** - * @param {string} [specifiedFormulaNumber] + * @param {string} [specifiedFormulaNumber] - The formula number of the endo bootstrap. * @returns {Promise<{ formulaIdentifier: string, value: import('./types').FarEndoBootstrap }>} */ const incarnateEndoBootstrap = async specifiedFormulaNumber => { - const formulaNumber = specifiedFormulaNumber || (await randomHex512()); + const formulaNumber = await (specifiedFormulaNumber ?? randomHex512()); const endoFormulaIdentifier = `endo:${formulaNumber}`; const { formulaIdentifier: defaultHostWorkerFormulaIdentifier } = @@ -985,7 +1063,6 @@ const makeDaemonCore = async ( }); const makeMailbox = makeMailboxMaker({ - incarnateLookup, getFormulaIdentifierForRef, provideValueForFormulaIdentifier, provideControllerForFormulaIdentifier, @@ -1010,7 +1087,6 @@ const makeDaemonCore = async ( incarnateWebBundle, incarnateHandle, storeReaderRef, - randomHex512, makeMailbox, }); @@ -1144,7 +1220,6 @@ const makeDaemonCore = async ( incarnateHost, incarnateGuest, incarnateEval, - incarnateLookup, incarnateUnconfined, incarnateReadableBlob, incarnateBundler, @@ -1159,7 +1234,6 @@ const makeDaemonCore = async ( * @param {import('./types.js').DaemonicPowers} powers * @param {Promise} webletPortP * @param {object} args - * @param {Promise} args.cancelled * @param {(error: Error) => void} args.cancel * @param {number} args.gracePeriodMs * @param {Promise} args.gracePeriodElapsed @@ -1168,12 +1242,11 @@ const makeDaemonCore = async ( const provideEndoBootstrap = async ( powers, webletPortP, - { cancelled, cancel, gracePeriodMs, gracePeriodElapsed }, + { cancel, gracePeriodMs, gracePeriodElapsed }, ) => { const { persistence: persistencePowers } = powers; const daemonCore = await makeDaemonCore(powers, webletPortP, { - cancelled, cancel, gracePeriodMs, gracePeriodElapsed, @@ -1228,7 +1301,6 @@ export const makeDaemon = async (powers, daemonLabel, cancel, cancelled) => { powers, assignedWebletPortP, { - cancelled, cancel, gracePeriodMs, gracePeriodElapsed, diff --git a/packages/daemon/src/host.js b/packages/daemon/src/host.js index c0cf74bc42..29cb7c72ab 100644 --- a/packages/daemon/src/host.js +++ b/packages/daemon/src/host.js @@ -17,7 +17,6 @@ export const makeHostMaker = ({ incarnateWebBundle, incarnateHandle, storeReaderRef, - randomHex512, makeMailbox, }) => { /** @@ -52,7 +51,6 @@ export const makeHostMaker = ({ reverseLookup, identifyLocal, listMessages, - provideLookupFormula, followMessages, resolve, reject, @@ -212,6 +210,7 @@ export const makeHostMaker = ({ await incarnateWorker(); return workerFormulaIdentifier; } + assertPetName(workerName); let workerFormulaIdentifier = identifyLocal(workerName); if (workerFormulaIdentifier === undefined) { @@ -223,6 +222,29 @@ export const makeHostMaker = ({ return workerFormulaIdentifier; }; + /** + * @param {string | 'MAIN' | 'NEW'} workerName + * @param {(hook: import('./types.js').EvalFormulaHook) => void} addHook + * @returns {string | undefined} + */ + const provideWorkerFormulaIdentifierSync = (workerName, addHook) => { + if (workerName === 'MAIN') { + return mainWorkerFormulaIdentifier; + } else if (workerName === 'NEW') { + return undefined; + } + + assertPetName(workerName); + const workerFormulaIdentifier = identifyLocal(workerName); + if (workerFormulaIdentifier === undefined) { + addHook(identifiers => + petStore.write(workerName, identifiers.workerFormulaIdentifier), + ); + return undefined; + } + return workerFormulaIdentifier; + }; + /** * @param {string | 'NONE' | 'SELF' | 'ENDO'} partyName * @returns {Promise} @@ -256,10 +278,6 @@ export const makeHostMaker = ({ petNamePaths, resultName, ) => { - const workerFormulaIdentifier = await provideWorkerFormulaIdentifier( - workerName, - ); - if (resultName !== undefined) { assertPetName(resultName); } @@ -267,8 +285,21 @@ export const makeHostMaker = ({ throw new Error('Evaluator requires one pet name for each code name'); } - const endowmentFormulaIdentifiers = await Promise.all( - petNamePaths.map(async (petNameOrPath, index) => { + /** @type {import('./types.js').EvalFormulaHook[]} */ + const hooks = []; + /** @type {(hook: import('./types.js').EvalFormulaHook) => void} */ + const addHook = hook => { + hooks.push(hook); + }; + + const workerFormulaIdentifier = provideWorkerFormulaIdentifierSync( + workerName, + addHook, + ); + + /** @type {(string | string[])[]} */ + const endowmentFormulaIdsOrPaths = petNamePaths.map( + (petNameOrPath, index) => { if (typeof codeNames[index] !== 'string') { throw new Error(`Invalid endowment name: ${q(codeNames[index])}`); } @@ -282,23 +313,24 @@ export const makeHostMaker = ({ return formulaIdentifier; } - const { formulaIdentifier: lookupFormulaIdentifier } = - await provideLookupFormula(petNamePath); - return lookupFormulaIdentifier; - }), + return petNamePath; + }, ); - // Behold, recursion: - // eslint-disable-next-line no-use-before-define - const { formulaIdentifier, value } = await incarnateEval( - workerFormulaIdentifier, + if (resultName !== undefined) { + addHook(identifiers => + petStore.write(resultName, `eval:${identifiers.evalFormulaNumber}`), + ); + } + + const { value } = await incarnateEval( + hostFormulaIdentifier, source, codeNames, - endowmentFormulaIdentifiers, + endowmentFormulaIdsOrPaths, + hooks, + workerFormulaIdentifier, ); - if (resultName !== undefined) { - await petStore.write(resultName, formulaIdentifier); - } return value; }; diff --git a/packages/daemon/src/mail.js b/packages/daemon/src/mail.js index e8a0974427..a98f8657cd 100644 --- a/packages/daemon/src/mail.js +++ b/packages/daemon/src/mail.js @@ -10,14 +10,12 @@ const { quote: q } = assert; /** * @param {object} args - * @param {(hubFormulaIdentifier: string, petNamePath: string[]) => Promise<{ formulaIdentifier: string, value: unknown }>} args.incarnateLookup * @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 */ export const makeMailboxMaker = ({ - incarnateLookup, getFormulaIdentifierForRef, provideValueForFormulaIdentifier, provideControllerForFormulaIdentifier, @@ -84,10 +82,8 @@ export const makeMailboxMaker = ({ throw new TypeError(`Unknown pet name: ${q(petName)}`); } // Behold, recursion: - // eslint-disable-next-line no-use-before-define - const controller = await provideControllerForFormulaIdentifier( - formulaIdentifier, - ); + const controller = + provideControllerForFormulaIdentifier(formulaIdentifier); console.log('Cancelled:'); return controller.context.cancel(reason); }; @@ -122,12 +118,6 @@ export const makeMailboxMaker = ({ return reverseLookupFormulaIdentifier(formulaIdentifier); }; - /** @type {import('./types.js').Mail['provideLookupFormula']} */ - const provideLookupFormula = async petNamePath => { - // TODO:lookup Check if the lookup formula already exists in the store - return incarnateLookup(selfFormulaIdentifier, petNamePath); - }; - /** * @param {import('./types.js').InternalMessage} message * @returns {import('./types.js').Message | undefined} @@ -541,7 +531,6 @@ export const makeMailboxMaker = ({ reverseLookup, reverseLookupFormulaIdentifier, identifyLocal, - provideLookupFormula, followMessages, listMessages, request, diff --git a/packages/daemon/src/types.d.ts b/packages/daemon/src/types.d.ts index c9e3b1b2e0..c8a2db33a0 100644 --- a/packages/daemon/src/types.d.ts +++ b/packages/daemon/src/types.d.ts @@ -97,6 +97,14 @@ type EvalFormula = { // TODO formula slots }; +export type EvalFormulaHook = ( + identifiers: Readonly<{ + endowmentFormulaIdentifiers: string[]; + evalFormulaNumber: string; + workerFormulaIdentifier: string; + }>, +) => Promise; + type ReadableBlobFormula = { type: 'readable-blob'; content: string; @@ -315,14 +323,6 @@ export interface Mail { listAll(): Array; reverseLookupFormulaIdentifier(formulaIdentifier: string): Array; cancel(petName: string, reason: unknown): Promise; - /** - * Takes a sequence of pet names and returns a formula identifier and value - * for the corresponding lookup formula. - * - * @param petNamePath A sequence of pet names. - * @returns The formula identifier and value of the lookup formula. - */ - provideLookupFormula(petNamePath: string[]): Promise; // Mail operations: listMessages(): Promise>; followMessages(): Promise>>; diff --git a/packages/daemon/test/test-endo.js b/packages/daemon/test/test-endo.js index 16a63f0108..c0abd773b4 100644 --- a/packages/daemon/test/test-endo.js +++ b/packages/daemon/test/test-endo.js @@ -647,7 +647,8 @@ test('direct termination', async t => { t.pass(); }); -test('indirect termination', async t => { +// See: https://github.com/endojs/endo/issues/2074 +test.failing('indirect termination', async t => { const { promise: cancelled, reject: cancel } = makePromiseKit(); t.teardown(() => cancel(Error('teardown'))); @@ -880,8 +881,7 @@ test('name and reuse inspector', async t => { await stop(locator); }); -// TODO: This test verifies existing behavior when pet-naming workers. -// This behavior is undesirable. See: https://github.com/endojs/endo/issues/2021 +// Regression test for: https://github.com/endojs/endo/issues/2021 test('eval-mediated worker name', async t => { const { promise: cancelled, reject: cancel } = makePromiseKit(); t.teardown(() => cancel(Error('teardown'))); @@ -903,6 +903,16 @@ test('eval-mediated worker name', async t => { const counterPath = path.join(dirname, 'test', 'counter.js'); await E(host).makeUnconfined('worker', counterPath, 'NONE', 'counter'); + t.is( + await E(host).evaluate( + 'worker', + 'E(counter).incr()', + ['counter'], + ['counter'], + ), + 1, + ); + // We create a petname for the worker of `counter`. // Note that while `worker === counter-worker`, it doesn't matter here. const counterWorker = await E(host).evaluate( @@ -914,19 +924,18 @@ test('eval-mediated worker name', async t => { ); t.regex(String(counterWorker), /Alleged: EndoWorker/u); - try { + // We should be able to use the new name for the worker. + t.is( await E(host).evaluate( - 'counter-worker', // Our worker pet name + 'counter-worker', 'E(counter).incr()', ['counter'], ['counter'], - ); - t.fail('should have thrown'); - } catch (error) { - // This is the error that we don't want - t.regex(error.message, /typeof target is "undefined"/u); - await stop(locator); - } + ), + 2, + ); + + await stop(locator); }); test('lookup with single petname', async t => {