diff --git a/packages/vats/src/core/promise-space.js b/packages/vats/src/core/promise-space.js index 3a4044b9445..309a3afb156 100644 --- a/packages/vats/src/core/promise-space.js +++ b/packages/vats/src/core/promise-space.js @@ -1,7 +1,8 @@ // @ts-check +import { E } from '@endo/far'; import { assertKey } from '@agoric/store'; import { canBeDurable } from '@agoric/vat-data'; -import { isPromise, makePromiseKit } from '@endo/promise-kit'; +import { makePromiseKit } from '@endo/promise-kit'; /** * @typedef {{ @@ -59,11 +60,7 @@ export const makeStoreHooks = (store, log = noop) => { return harden({ ...logHooks, onResolve: (name, valueP) => { - if (isPromise(valueP)) { - void valueP.then(value => save(name, value)); - } else { - save(name, valueP); - } + void E.when(valueP, value => save(name, value)); }, onReset: name => { if (store.has(name)) { @@ -77,7 +74,7 @@ export const makeStoreHooks = (store, log = noop) => { * Make { produce, consume } where for each name, `consume[name]` is a promise * and `produce[name].resolve` resolves it. * - * Note: repeated resolve()s are noops. + * Note: repeated resolve()s without an intervening reset() are noops. * * @template {Record} [T=Record] * @param {{ log?: typeof console.log } & ( @@ -95,102 +92,97 @@ export const makePromiseSpace = (optsOrLog = {}) => { const { onAddKey, onSettled, onResolve, onReset } = hooks; /** - * @typedef {PromiseRecord & { - * reset: (reason?: unknown) => void, - * isSettling: boolean, - * }} PromiseState + * @typedef {{ pk: PromiseRecord, isSettling: boolean }} PromiseState */ /** @type {Map} */ const nameToState = new Map(); /** @type {Set} */ const remaining = new Set(); - /** @param {string} name */ + /** @type {(name: string) => PromiseState} */ const provideState = name => { - /** @type {PromiseState} */ - let state; - const currentState = nameToState.get(name); - if (currentState) { - state = currentState; - } else { + let state = nameToState.get(name); + if (!state) { onAddKey(name); + remaining.add(name); const pk = makePromiseKit(); - pk.promise .finally(() => { remaining.delete(name); onSettled(name, remaining); }) .catch(() => {}); - - const settling = () => { - assert(state); - state = harden({ ...state, isSettling: true }); - nameToState.set(name, state); - }; - - const resolve = value => { - settling(); - onResolve(name, value); - pk.resolve(value); - }; - const reject = reason => { - settling(); - pk.reject(reason); - }; - - const reset = (reason = undefined) => { - onReset(name); - if (!state.isSettling) { - if (!reason) { - // Reuse the old promise; don't reject it. - return; - } - reject(reason); - } - // Now publish a new promise. - nameToState.delete(name); - remaining.delete(name); - }; - - state = harden({ - isSettling: false, - resolve, - reject, - reset, - promise: pk.promise, - }); + state = harden({ pk, isSettling: false }); nameToState.set(name, state); - remaining.add(name); } return state; }; + // we must tolerate these producer methods being retrieved both + // before and after the consumer is retrieved, and also both before + // and after reset() is invoked, so they only close over 'name' and + // not over any state variables + + const makeProducer = name => { + const resolve = value => { + const old = provideState(name); + if (old.isSettling) { + // First attempt to settle always wins. + return; + } + onResolve(name, value); + nameToState.set(name, harden({ ...old, isSettling: true })); + old.pk.resolve(value); + }; + const reject = reason => { + const old = provideState(name); + if (old.isSettling) { + // First attempt to settle always wins. + return; + } + nameToState.set(name, harden({ ...old, isSettling: true })); + old.pk.reject(reason); + }; + const reset = (reason = undefined) => { + onReset(name); + const old = provideState(name); + if (!old.isSettling) { + // we haven't produced a value yet, and there might be + // consumers still watching old.pk.promise + if (reason === undefined) { + // so just let them wait for the new value: resetting an + // unresolved item is a no-op + return; + } + // reject those watchers; new watchers will wait for the new + // value through the replacement promise + reject(reason); + } + // delete the state, so new callers will get a new promise kit + nameToState.delete(name); + remaining.delete(name); + }; + + return harden({ resolve, reject, reset }); + }; + /** @type {PromiseSpaceOf['consume']} */ // @ts-expect-error cast - const consume = new Proxy( - {}, - { - get: (_target, name) => { - assert.typeof(name, 'string'); - const kit = provideState(name); - return kit.promise; - }, + const consume = new Proxy(harden({}), { + get: (_target, name) => { + assert.typeof(name, 'string'); + return provideState(name).pk.promise; }, - ); + }); /** @type {PromiseSpaceOf['produce']} */ // @ts-expect-error cast - const produce = new Proxy( - {}, - { - get: (_target, name) => { - assert.typeof(name, 'string'); - const { reject, resolve, reset } = provideState(name); - return harden({ reject, resolve, reset }); - }, + const produce = new Proxy(harden({}), { + get: (_target, name) => { + assert.typeof(name, 'string'); + return makeProducer(name); }, - ); + }); return harden({ produce, consume }); }; diff --git a/packages/vats/test/test-promise-space.js b/packages/vats/test/test-promise-space.js index f415dddc3a8..38b7fda8fa8 100644 --- a/packages/vats/test/test-promise-space.js +++ b/packages/vats/test/test-promise-space.js @@ -38,7 +38,7 @@ test('makePromiseSpace', async t => { await checkAlice(reusedAlice, `Hi, I'm Alice 3!`); }); -test('makePromiseSpace backed by store', async t => { +test('makePromiseSpace copied into store', async t => { /** @type {MapStore} */ const store = makeScalarBigMapStore('stuff', { durable: true }); { @@ -64,6 +64,7 @@ test('makePromiseSpace backed by store', async t => { produce.chocolate.resolve(doesNotResolve); const nonPassable = harden(() => {}); produce.strawberry.resolve(nonPassable); + await null; const actual = Object.fromEntries(store.entries()); t.deepEqual( actual, @@ -78,3 +79,77 @@ test('makePromiseSpace backed by store', async t => { produce.strawberry.resolve('ignored already resolved'); } }); + +test('resolve after reset', async t => { + /** @type {MapStore} */ + const store = makeScalarBigMapStore('stuff', { durable: true }); + + const { consume, produce } = makePromiseSpace({ store }); + + // sample resolve/consume early, then use after reset(): #7709 + + // for foo, we produce first + { + // reset before resolving the first time + const { resolve, reset } = produce.foo; + const foo1 = consume.foo; + reset(); + resolve(1); + t.is(await foo1, 1); + const foo2 = consume.foo; + t.is(await foo2, 1); + } + + { + const { resolve, reset } = produce.foo; + const foo1 = consume.foo; + reset(); + resolve(2); + t.is(await foo1, 1); // captured before reset() + const foo2 = consume.foo; + t.is(await foo2, 2); + } + + { + const foo1 = consume.foo; + produce.foo.reset(); + const foo2 = consume.foo; + produce.foo.resolve(3); + const foo3 = consume.foo; + t.is(await foo1, 2); // captured before reset() + t.is(await foo2, 3); + t.is(await foo3, 3); + } + + // for 'bar', we consume first + { + const bar1 = consume.bar; + const { resolve, reset } = produce.bar; + reset(); + resolve(1); + t.is(await bar1, 1); + const bar2 = consume.bar; + t.is(await bar2, 1); + } + + { + const { resolve, reset } = produce.bar; + const bar1 = consume.bar; + reset(); + resolve(2); + t.is(await bar1, 1); // captured before reset() + const bar2 = consume.bar; + t.is(await bar2, 2); + } + + { + const bar1 = consume.bar; + produce.bar.reset(); + const bar2 = consume.bar; + produce.bar.resolve(3); + const bar3 = consume.bar; + t.is(await bar1, 2); // captured before reset() + t.is(await bar2, 3); + t.is(await bar3, 3); + } +});