From a4f86eb7fd602980a40d00d739897090d3667d3d Mon Sep 17 00:00:00 2001 From: Mathieu Hofman <86499+mhofman@users.noreply.github.com> Date: Fri, 21 Jun 2024 22:20:50 -0700 Subject: [PATCH] fix(vow): handle resolution loops in vows (#9561) closes: #9560 ## Description Add a weak set of previously seen vowV0 payloads when shortening during `watch` or `when` ### Security Considerations None ### Scaling Considerations Potential churn in storage just for shortening loop detection. Every `watch` will cause a new WeakSetStore to be allocated. ### Documentation Considerations None ### Testing Considerations New unit tests ### Upgrade Considerations To avoid state migration concerns, this needs to be deployed before we start using `watch` more broadly. --- packages/vow/src/watch.js | 31 +++++++++++++++++-- packages/vow/src/when.js | 10 ++++++- packages/vow/test/watch.test.js | 53 +++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/packages/vow/src/watch.js b/packages/vow/src/watch.js index c1f87f82095..9391dfeefb7 100644 --- a/packages/vow/src/watch.js +++ b/packages/vow/src/watch.js @@ -65,8 +65,23 @@ const settle = (resolver, watcher, wcb, value, watcherArgs = []) => { * @param {IsRetryableReason} isRetryableReason * @param {ReturnType} watchNextStep */ -const preparePromiseWatcher = (zone, isRetryableReason, watchNextStep) => - zone.exoClass( +const preparePromiseWatcher = (zone, isRetryableReason, watchNextStep) => { + // We use an ephemeral WeakSet for the previously seen vows in a watch operation + // While watch is durable, it suffices to detect the cycle in a single incarnation + /** @type {WeakMap>} */ + const watcherSeenPayloads = new WeakMap(); + + /** @param {PromiseWatcher} watcher */ + const getSeenPayloads = watcher => { + let seenPayloads = watcherSeenPayloads.get(watcher); + if (!seenPayloads) { + seenPayloads = new WeakSet(); + watcherSeenPayloads.set(watcher, seenPayloads); + } + return seenPayloads; + }; + + return zone.exoClass( 'PromiseWatcher', PromiseWatcherI, /** @@ -91,12 +106,20 @@ const preparePromiseWatcher = (zone, isRetryableReason, watchNextStep) => /** @type {Required['onFulfilled']} */ onFulfilled(value) { const { watcher, watcherArgs, resolver } = this.state; - if (getVowPayload(value)) { + const payload = getVowPayload(value); + if (payload) { + const seenPayloads = getSeenPayloads(this.self); + // TODO: rely on endowed helper to get storable cap from payload + if (seenPayloads.has(payload.vowV0)) { + return this.self.onRejected(Error('Vow resolution cycle detected')); + } + seenPayloads.add(payload.vowV0); // We've been shortened, so reflect our state accordingly, and go again. this.state.vow = value; watchNextStep(value, this.self); return; } + watcherSeenPayloads.delete(this.self); this.state.priorRetryValue = undefined; this.state.watcher = undefined; this.state.resolver = undefined; @@ -115,6 +138,7 @@ const preparePromiseWatcher = (zone, isRetryableReason, watchNextStep) => return; } } + watcherSeenPayloads.delete(this.self); this.state.priorRetryValue = undefined; this.state.resolver = undefined; this.state.watcher = undefined; @@ -122,6 +146,7 @@ const preparePromiseWatcher = (zone, isRetryableReason, watchNextStep) => }, }, ); +}; /** * @param {Zone} zone diff --git a/packages/vow/src/when.js b/packages/vow/src/when.js index ffc47165a5c..e8e6c540ea2 100644 --- a/packages/vow/src/when.js +++ b/packages/vow/src/when.js @@ -28,11 +28,19 @@ export const makeWhen = ( let result = await specimenP; let payload = getVowPayload(result); let priorRetryValue; + const seenPayloads = new WeakSet(); while (payload) { - result = await basicE(payload.vowV0) + // TODO: rely on endowed helpers for getting storable cap and performing + // shorten "next step" + const { vowV0 } = payload; + if (seenPayloads.has(vowV0)) { + throw Error('Vow resolution cycle detected'); + } + result = await basicE(vowV0) .shorten() .then( res => { + seenPayloads.add(vowV0); priorRetryValue = undefined; return res; }, diff --git a/packages/vow/test/watch.test.js b/packages/vow/test/watch.test.js index 9d9239733ba..851b43ae9f4 100644 --- a/packages/vow/test/watch.test.js +++ b/packages/vow/test/watch.test.js @@ -171,6 +171,59 @@ test('watcher args arity - shim', async t => { } }); +test('vow self resolution', async t => { + const zone = makeHeapZone(); + const { watch, when, makeVowKit } = prepareVowTools(zone); + + // A direct self vow resolution + const { vow: vow1, resolver: resolver1 } = makeVowKit(); + resolver1.resolve(vow1); + + // A self vow resolution through promise + const { vow: vow2, resolver: resolver2 } = makeVowKit(); + const vow2P = Promise.resolve(vow2); + resolver2.resolve(vow2P); + + // A 2 vow loop + const { vow: vow3, resolver: resolver3 } = makeVowKit(); + const { vow: vow4, resolver: resolver4 } = makeVowKit(); + resolver3.resolve(vow4); + resolver4.resolve(vow3); + + // A head vow pointing to a 2 vow loop (a lasso?) + const { vow: vow5, resolver: resolver5 } = makeVowKit(); + resolver5.resolve(vow4); + + const turnTimeout = async n => { + if (n > 0) { + return Promise.resolve(n - 1).then(turnTimeout); + } + + return 'timeout'; + }; + + /** + * @param {number} n + * @param {Promise} promise + */ + const raceTurnTimeout = async (n, promise) => + Promise.race([promise, turnTimeout(n)]); + + const expectedError = { + message: 'Vow resolution cycle detected', + }; + + await t.throwsAsync(raceTurnTimeout(20, when(vow1)), expectedError); + await t.throwsAsync(raceTurnTimeout(20, when(vow2)), expectedError); + await t.throwsAsync(raceTurnTimeout(20, when(vow3)), expectedError); + await t.throwsAsync(raceTurnTimeout(20, when(vow5)), expectedError); + + await t.throwsAsync(raceTurnTimeout(20, when(watch(vow1))), expectedError); + await t.throwsAsync(raceTurnTimeout(20, when(watch(vow2))), expectedError); + await t.throwsAsync(raceTurnTimeout(20, when(watch(vow3))), expectedError); + await t.throwsAsync(raceTurnTimeout(20, when(watch(vow5))), expectedError); +}); + test('disconnection of non-vow informs watcher', async t => { const zone = makeHeapZone(); const { watch, when } = prepareVowTools(zone, {