diff --git a/packages/SwingSet/src/kernel/liveSlots.js b/packages/SwingSet/src/kernel/liveSlots.js index a22665cae80..ac9e446d61a 100644 --- a/packages/SwingSet/src/kernel/liveSlots.js +++ b/packages/SwingSet/src/kernel/liveSlots.js @@ -36,77 +36,6 @@ function build(syscall, _state, makeRoot, forVatID) { } } - // Make a handled Promise that enqueues kernel messages. - // - // at present, for every promise we receive (a `p-NN` reference for which - // we build a Promise object for application-level code) that eventually - // resolves to a Presence (i.e. an `o-NN` that represents an object which - // we might send messages to, for which we create a Presence to hand up to - // our application-level code), we wind up calling makeQueued() twice. - // - // The first time is during importPromise(), where we call - // makeQueued('p-NN') as we build the HandledPromise that uses the - // unfulfilledHandler (the second argument to `new HandledPromise()` to - // close over 'p-NN'. The unfulfilledHandler is used when applications do - // E(p).foo() or p~.foo(), aimed at the Promise they receive. This - // HandledPromise does not ever call the fulfilledHandler, because we never - // call `pr.resPres` or the resolveWithPresence function. - // - // The second time is during notifyFulfillToPresence, when it does - // convertSlotToVal(o-NN) and that function must handle the `type == - // 'object'` case. Here, we call makeQueued('o-NN') to create a - // HandledPromise, then immediately call `pr.resPres` to obtain the - // generated Presence object, and then throw away the HandledPromise. (*we* - // throw it away, however the HandledPromise implementation remembers it, - // and it can be retrieved by `HandledPromise.resolve(presence)`, or in the - // internals of an E(presence).foo() call). In this case, we care about the - // *fulfilledHandler*, not the unfulfilledHandler: the HandledPromise is - // never revealed to anyone before it is resolved-to-presence, which means - // its unfulfilledHandler will never be used. - // - // When notifyFulfillToPresence() resolves the first (importPromise) - // HandledPromise with a Presence that's associated with the second - // promise, all the handlers from the first are replaced with those from - // the second. The first promise becomes behaviorally indistinguishable - // from the second (they still have distinct identities, but all method - // invocations will act as if they are the same). - // - // We might consider de-factoring this in the future, into two separate - // functions for these two cases. The original makeQueued('p-NN') could be - // changed to only set the unfulfilledHandler, and omit `pr.resPres`. The - // second would be named makePresence('o-NN'): it would call - // resolveWithPresence() immediately, wouldn't set unfulfilledHandler, and - // would only return the Presence converted to a Remotable. This might be clearer. - - function makeQueued(slot) { - /* eslint-disable no-use-before-define */ - lsdebug(`makeQueued(${slot})`); - const handler = { - applyMethod(_o, prop, args, returnedP) { - // Support: o~.[prop](...args) remote method invocation - lsdebug(`makeQueued handler.applyMethod (${slot})`); - return queueMessage(slot, prop, args, returnedP); - }, - }; - /* eslint-enable no-use-before-define */ - - const pr = {}; - pr.p = new HandledPromise((res, rej, resolveWithPresence) => { - pr.rej = rej; - pr.resPres = () => resolveWithPresence(handler); // fulfilledHandler - pr.res = res; - }, handler); // unfulfilledHandler - // We harden this Promise because it feeds importPromise(), which is - // where remote promises inside inbound arguments and resolutions are - // created. Both places are additionally hardened by m.unserialize, but - // it seems reasonable to do it here too, just in case. - return harden(pr); - } - - function makeDeviceNode(id) { - return Remotable(`Device ${id}`); - } - const outstandingProxies = new WeakSet(); /** Map in-vat object references -> vat slot strings. @@ -124,6 +53,117 @@ function build(syscall, _state, makeRoot, forVatID) { let nextExportID = 1; let nextPromiseID = 5; + function makeImportedPresence(slot) { + // Called by convertSlotToVal for type=object (an `o-NN` reference). We + // build a Presence for application-level code to receive. This Presence + // is associated with 'slot' so that all handled messages get sent to + // that slot: pres~.foo() causes a syscall.send(target=slot, msg=foo). + + lsdebug(`makeImportedPresence(${slot})`); + const fulfilledHandler = { + applyMethod(_o, prop, args, returnedP) { + // Support: o~.[prop](...args) remote method invocation + lsdebug(`makeImportedPresence handler.applyMethod (${slot})`); + // eslint-disable-next-line no-use-before-define + return queueMessage(slot, prop, args, returnedP); + }, + }; + + let presence; + const p = new HandledPromise((_res, _rej, resolveWithPresence) => { + const remote = resolveWithPresence(fulfilledHandler); + presence = Remotable(`Presence ${slot}`, undefined, remote); + // remote === presence, actually + + // todo: mfig says to swap remote and presence (resolveWithPresence + // gives us a Presence, Remoteable gives us a Remote). I think that + // implies we have a lot of renaming to do, 'makeRemote' instead of + // 'makeImportedPresence', etc. I'd like to defer that for a later + // cleanup/renaming pass. + }); // no unfulfilledHandler + + // The call to resolveWithPresence performs the forwarding logic + // immediately, so by the time we reach here, E(presence).foo() will use + // our fulfilledHandler, and nobody can observe the fact that we failed + // to provide an unfulfilledHandler. + + // We throw 'p' away, but it is retained by the internal tables of + // HandledPromise, and will be returned to anyone who calls + // `HandledPromise.resolve(presence)`. So we must harden it now, for + // safety, to prevent it from being used as a communication channel + // between isolated objects that share a reference to the Presence. + harden(p); + + // Up at the application level, presence~.foo(args) starts by doing + // HandledPromise.resolve(presence), which retrieves it, and then does + // p.eventualSend('foo', [args]), which uses the fulfilledHandler. + + // We harden the presence for the same safety reasons. + return harden(presence); + } + + function makeImportedPromise(vpid) { + // Called by convertSlotToVal(type=promise) for incoming promises (a + // `p-NN` reference), and by queueMessage() for the result of an outbound + // message (a `p+NN` reference). We build a Promise for application-level + // code, to which messages can be pipelined, and we prepare for the + // kernel to tell us that it has been resolved in various ways. + insistVatType('promise', vpid); + lsdebug(`makeImportedPromise(${vpid})`); + + // The Promise will we associated with a handler that converts p~.foo() + // into a syscall.send() that targets the vpid. When the Promise is + // resolved (during receipt of a dispatch.notifyFulfill* or + // notifyReject), this Promise's handler will be replaced by the handler + // of the resolution, which might be a Presence or a local object. + + // for safety as we shake out bugs in HandledPromise, we guard against + // this handler being used after it was supposed to be resolved + let handlerActive = true; + const unfulfilledHandler = { + applyMethod(_o, prop, args, returnedP) { + // Support: o~.[prop](...args) remote method invocation + lsdebug(`makeImportedPromise handler.applyMethod (${vpid})`); + if (!handlerActive) { + console.error(`mIPromise handler called after resolution`); + throw Error(`mIPromise handler called after resolution`); + } + // eslint-disable-next-line no-use-before-define + return queueMessage(vpid, prop, args, returnedP); + }, + }; + + let resolve; + let reject; + const p = new HandledPromise((res, rej, _resPres) => { + resolve = res; + reject = rej; + }, unfulfilledHandler); + + // Prepare for the kernel to tell us about resolution. Both ensure the + // old handler should never be called again. TODO: once we're confident + // about how we interact with HandledPromise, just use harden({ resolve, + // reject }). + const pRec = harden({ + resolve(resolution) { + handlerActive = false; + resolve(resolution); + }, + + reject(rejection) { + handlerActive = false; + reject(rejection); + }, + }); + importedPromisesByPromiseID.set(vpid, pRec); + + return harden(p); + } + + function makeDeviceNode(id) { + return Remotable(`Device ${id}`); + } + // TODO: fix awkward non-orthogonality: allocateExportID() returns a number, // allocatePromiseID() returns a slot, exportPromise() uses the slot from // allocatePromiseID(), exportPassByPresence() generates a slot itself using @@ -185,29 +225,6 @@ function build(syscall, _state, makeRoot, forVatID) { return valToSlot.get(val); } - function importedPromiseThen(vpid) { - insistVatType('promise', vpid); - syscall.subscribe(vpid); - } - - function importPromise(vpid) { - insistVatType('promise', vpid); - assert( - !parseVatSlot(vpid).allocatedByVat, - details`kernel is being presumptuous: vat got unrecognized vatSlot ${vpid}`, - ); - const pr = makeQueued(vpid); - importedPromisesByPromiseID.set(vpid, pr); - const { p } = pr; - // ideally we'd wait until .then is called on p before subscribing, but - // the current Promise API doesn't give us a way to discover this, so we - // must subscribe right away. If we were using Vows or some other - // then-able, we could just hook then() to notify us. - lsdebug(`ls[${forVatID}].importPromise.importedPromiseThen ${vpid}`); - importedPromiseThen(vpid); - return p; - } - function convertSlotToVal(slot) { if (!slotToVal.has(slot)) { let val; @@ -215,18 +232,21 @@ function build(syscall, _state, makeRoot, forVatID) { assert(!allocatedByVat, details`I don't remember allocating ${slot}`); if (type === 'object') { // this is a new import value - // lsdebug(`assigning new import ${slot}`); - // prepare a Promise for this Presence, so E(val) can work - const pr = makeQueued(slot); // TODO find a less confusing name than "pr" - const remote = pr.resPres(); - val = Remotable(`Presence ${slot}`, undefined, remote); - // lsdebug(` for presence`, val); + val = makeImportedPresence(slot); } else if (type === 'promise') { - val = importPromise(slot); + assert( + !parseVatSlot(slot).allocatedByVat, + details`kernel is being presumptuous: vat got unrecognized vatSlot ${slot}`, + ); + val = makeImportedPromise(slot); + // ideally we'd wait until .then is called on p before subscribing, + // but the current Promise API doesn't give us a way to discover + // this, so we must subscribe right away. If we were using Vows or + // some other then-able, we could just hook then() to notify us. + syscall.subscribe(slot); } else if (type === 'device') { val = makeDeviceNode(slot); } else { - // todo (temporary): resolver? throw Error(`unrecognized slot type '${type}'`); } slotToVal.set(slot, val); @@ -239,26 +259,36 @@ function build(syscall, _state, makeRoot, forVatID) { function queueMessage(targetSlot, prop, args, returnedP) { const serArgs = m.serialize(harden(args)); - const result = allocatePromiseID(); - lsdebug(`Promise allocation ${forVatID}:${result} in queueMessage`); - const done = makeQueued(result); - lsdebug(`ls.qm send(${JSON.stringify(targetSlot)}, ${prop}) -> ${result}`); - syscall.send(targetSlot, prop, serArgs, result); - - // prepare for notifyFulfillToData/etc - importedPromisesByPromiseID.set(result, done); - - // ideally we'd wait until someone .thens done.p, but with native - // Promises we have no way of spotting that, so subscribe immediately - lsdebug(`ls[${forVatID}].queueMessage.importedPromiseThen ${result}`); - importedPromiseThen(result); - - // prepare the serializer to recognize the promise we will return, - // if it's used as an argument or return value - valToSlot.set(returnedP, result); - slotToVal.set(result, returnedP); - - return done.p; + const resultVPID = allocatePromiseID(); + lsdebug(`Promise allocation ${forVatID}:${resultVPID} in queueMessage`); + // create a Promise which callers follow for the result, give it a + // handler so we can pipeline messages to it, and prepare for the kernel + // to notify us of its resolution + const p = makeImportedPromise(resultVPID); + + lsdebug( + `ls.qm send(${JSON.stringify(targetSlot)}, ${prop}) -> ${resultVPID}`, + ); + syscall.send(targetSlot, prop, serArgs, resultVPID); + + // ideally we'd wait until .then is called on p before subscribing, but + // the current Promise API doesn't give us a way to discover this, so we + // must subscribe right away. If we were using Vows or some other + // then-able, we could just hook then() to notify us. + syscall.subscribe(resultVPID); + + // We return 'p' to the handler, and the eventual resolution of 'p' will + // be used to resolve the caller's Promise, but the caller never sees 'p' + // itself. The caller got back their Promise before the handler ever got + // invoked, and thus before queueMessage was called.. If that caller + // passes the Promise they received as argument or return value, we want + // it to serialize as resultVPID. And if someone passes resultVPID to + // them, we want the user-level code to get back that Promise, not 'p'. + + valToSlot.set(returnedP, resultVPID); + slotToVal.set(resultVPID, returnedP); + + return p; } function DeviceHandler(slot) { @@ -302,42 +332,57 @@ function build(syscall, _state, makeRoot, forVatID) { if (!t) { throw Error(`no target ${target}`); } + // TODO: if we acquire new decision-making authority over a promise that + // we already knew about ('result' is already in slotToVal), we should no + // longer accept dispatch.notifyFulfill from the kernel. We currently use + // importedPromisesByPromiseID to track a combination of "we care about + // when this promise resolves" and "we are listening for the kernel to + // resolve it". We should split that into two tables or something. And we + // should error-check cases that the kernel shouldn't do, like getting + // the same vpid as a result= twice, or getting a result= for an exported + // promise (for which we were already the decider). + const args = m.unserialize(argsdata); - const p = Promise.resolve().then(_ => { - // The idiom here results in scheduling the method invocation on the next - // turn, but more importantly arranges for errors to become promise - // rejections rather than errors in the kernel itself + + let notifySuccess = () => undefined; + let notifyFailure = () => undefined; + if (result) { + insistVatType('promise', result); + // eslint-disable-next-line no-use-before-define + notifySuccess = thenResolve(result); + // eslint-disable-next-line no-use-before-define + notifyFailure = thenReject(result); + } + + // If the method is missing, or is not a Function, or the method throws a + // synchronous exception, we notify the caller (by rejecting the result + // promise, if any). If the method returns an eventually-rejected + // Promise, we notify them when it resolves. + + // If the method returns a synchronous value, we notify the caller right + // away. If it returns an eventually-fulfilled Promise, we notify the + // caller when it resolves. + + // Both situations are the business of this vat and the calling vat, not + // the kernel. deliver() does not report such exceptions to the kernel. + + try { if (!(method in t)) { - throw new TypeError( - `target[${method}] does not exist, has ${Object.getOwnPropertyNames( - t, - )}`, - ); + const names = Object.getOwnPropertyNames(t); + throw new TypeError(`target[${method}] does not exist, has ${names}`); } if (!(t[method] instanceof Function)) { + const ftype = typeof t[method]; + const names = Object.getOwnPropertyNames(t); throw new TypeError( - `target[${method}] is not a function, typeof is ${typeof t[ - method - ]}, has ${Object.getOwnPropertyNames(t)}`, + `target[${method}] is not a function, typeof is ${ftype}, has ${names}`, ); } - return t[method](...args); - }); - if (result) { - lsdebug(` ls.deliver attaching then ->${result}`); - insistVatType('promise', result); - - // We return the results of the resolve/reject, rather - // than rejecting the dispatch with whatever the method - // did. - // It is up to the caller to handle or fail to handle the - // rejection. Failing to handle should trigger a platform - // log, rather than via our parent doProcess call. - - // eslint-disable-next-line no-use-before-define - return p.then(thenResolve(result), thenReject(result)); + const res = t[method](...args); + Promise.resolve(res).then(notifySuccess, notifyFailure); + } catch (err) { + notifyFailure(err); } - return p; } function thenResolve(promiseID) { @@ -345,6 +390,7 @@ function build(syscall, _state, makeRoot, forVatID) { return res => { harden(res); lsdebug(`ls.thenResolve fired`, res); + // We need to know if this is resolving to an imported/exported // presence, because then the kernel can deliver queued messages. We // could build a simpler way of doing this. @@ -358,17 +404,34 @@ function build(syscall, _state, makeRoot, forVatID) { unser[QCLASS] === 'slot' ) { const slot = ser.slots[unser.index]; - const { type } = parseVatSlot(slot); - if (type === 'object') { - syscall.fulfillToPresence(promiseID, slot); - } else { - throw new Error(`thenResolve to non-object slot ${slot}`); - } + insistVatType('object', slot); + syscall.fulfillToPresence(promiseID, slot); } else { // if it resolves to data, .thens fire but kernel-queued messages are // rejected, because you can't send messages to data syscall.fulfillToData(promiseID, ser); } + + // TODO (for chip): the kernel currently notifies all subscribers of a + // promise about its resolution, even a subscriber who causes that + // promise to be resolved. We notify ourselves here anyways, because + // we'll need this when the retire-promises branch lands and the kernel + // behavior is changed to refrain from echoing back the notification to + // the resolving vat. So for now, we're double-resolving the promise, + // but it doesn't seem to cause any problems, and the tests + // (test-vpid-liveslots) depends upon it (the test doesn't simulate the + // kernel doing a echoed notification). When we land that branch, we + // can delete this first comment, without changing any of the code. + // (leave the following comment, it becomes correct then) + + // If we were *also* waiting on this promise (perhaps we received it as + // an argument, and also as a result=), then we are responsible for + // notifying ourselves. The kernel assumes we're a grownup and don't + // need to be reminded of something we did ourselves. + const pRec = importedPromisesByPromiseID.get(promiseID); + if (pRec) { + pRec.resolve(res); + } }; } @@ -378,6 +441,12 @@ function build(syscall, _state, makeRoot, forVatID) { lsdebug(`ls thenReject fired`, rej); const ser = m.serialize(rej); syscall.reject(promiseID, ser); + // TODO (for chip): this is also a double-rejection until the + // retire-promises branch lands. Delete this comment when that happens. + const pRec = importedPromisesByPromiseID.get(promiseID); + if (pRec) { + pRec.reject(rej); + } }; } @@ -387,34 +456,45 @@ function build(syscall, _state, makeRoot, forVatID) { `ls.dispatch.notifyFulfillToData(${promiseID}, ${data.body}, ${data.slots})`, ); insistVatType('promise', promiseID); + // TODO: insist that we do not have decider authority for promiseID if (!importedPromisesByPromiseID.has(promiseID)) { throw new Error(`unknown promiseID '${promiseID}'`); } + const pRec = importedPromisesByPromiseID.get(promiseID); const val = m.unserialize(data); - importedPromisesByPromiseID.get(promiseID).res(val); + pRec.resolve(val); } function notifyFulfillToPresence(promiseID, slot) { lsdebug(`ls.dispatch.notifyFulfillToPresence(${promiseID}, ${slot})`); insistVatType('promise', promiseID); + // TODO: insist that we do not have decider authority for promiseID + insistVatType('object', slot); if (!importedPromisesByPromiseID.has(promiseID)) { throw new Error(`unknown promiseID '${promiseID}'`); } + const pRec = importedPromisesByPromiseID.get(promiseID); const val = convertSlotToVal(slot); - importedPromisesByPromiseID.get(promiseID).res(val); + // val is either a local pass-by-presence object, or a Presence (which + // points at some remote pass-by-presence object). + pRec.resolve(val); } + // TODO: when we add notifyForward, guard against cycles + function notifyReject(promiseID, data) { insistCapData(data); lsdebug( `ls.dispatch.notifyReject(${promiseID}, ${data.body}, ${data.slots})`, ); insistVatType('promise', promiseID); + // TODO: insist that we do not have decider authority for promiseID if (!importedPromisesByPromiseID.has(promiseID)) { throw new Error(`unknown promiseID '${promiseID}'`); } + const pRec = importedPromisesByPromiseID.get(promiseID); const val = m.unserialize(data); - importedPromisesByPromiseID.get(promiseID).rej(val); + pRec.reject(val); } // here we finally invoke the vat code, and get back the root object diff --git a/packages/SwingSet/test/message-patterns.js b/packages/SwingSet/test/message-patterns.js index 72d7558b471..a254cba6095 100644 --- a/packages/SwingSet/test/message-patterns.js +++ b/packages/SwingSet/test/message-patterns.js @@ -419,6 +419,7 @@ export function buildPatterns(E, log) { // todo: all the pipelined calls (pipe123) get sent to the kernel a // turn after the two call-to-presence calls (getpx/resolvex), which is // a bit weird. Feels like p.post gets one extra stall. + // todo: is this still true? const px = E(b.bob).b71_getpx(); const p1 = E(px).pipe1(); const p2 = E(p1).pipe2(); @@ -451,7 +452,15 @@ export function buildPatterns(E, log) { p1.resolve(x); }; } - out.a71 = ['pipe1', 'pipe2', 'pipe3', 'p1.then', 'p2.then', 'p3.then']; + out.a71 = ['pipe1', 'p1.then', 'pipe2', 'p2.then', 'pipe3', 'p3.then']; + outPipelined.a71 = [ + 'pipe1', + 'pipe2', + 'pipe3', + 'p1.then', + 'p2.then', + 'p3.then', + ]; test('a71'); // px!pipe1()!pipe2()!pipe3(); px.resolve() but better diff --git a/packages/SwingSet/test/test-vpid-liveslots.js b/packages/SwingSet/test/test-vpid-liveslots.js index 46630029305..8924668b8c6 100644 --- a/packages/SwingSet/test/test-vpid-liveslots.js +++ b/packages/SwingSet/test/test-vpid-liveslots.js @@ -93,35 +93,68 @@ function hush(p) { // protect against reentrant handlers. For details see // https://github.com/Agoric/agoric-sdk/issues/886 -// In addition, we want to exercise the promises being resolved in three -// different ways: -// X: resolveToPresence, messages can be sent to resolution -// Y: resolveToData, messages are rejected as DataIsNotCallable -// Z: reject, messages are rejected - -function resolvePR(pr, mode, target2) { +// In addition, we want to exercise the promises being resolved in various +// ways: +const modes = [ + 'presence', // resolveToPresence, messages can be sent to resolution + 'local-object', // resolve to a local object, messages can be sent but do + // // not create syscalls + 'data', // resolveToData, messages are rejected as DataIsNotCallable + 'promise-data', // resolveToData that contains a promise ID + 'reject', // reject, messages are rejected + 'promise-reject', // reject to data that contains a promise ID +]; + +function resolvePR(pr, mode, targets) { switch (mode) { case 'presence': - pr.resolve(target2); + pr.resolve(targets.target2); + break; + case 'local-object': + pr.resolve( + harden({ + two() { + /* console.log(`local two() called`); */ + }, + four() { + /* console.log(`local four() called`); */ + }, + }), + ); break; case 'data': pr.resolve(4); break; + case 'promise-data': + pr.resolve([targets.p1]); + break; case 'reject': pr.reject('error'); break; + case 'promise-reject': + pr.reject([targets.p1]); + break; default: throw Error(`unknown mode ${mode}`); } } -function resolutionOf(vpid, mode, target2) { +const slot0arg = { '@qclass': 'slot', index: 0 }; +const slot1arg = { '@qclass': 'slot', index: 1 }; + +function resolutionOf(vpid, mode, targets) { switch (mode) { case 'presence': return { type: 'fulfillToPresence', promiseID: vpid, - slot: target2, + slot: targets.target2, + }; + case 'local-object': + return { + type: 'fulfillToPresence', + promiseID: vpid, + slot: targets.localTarget, }; case 'data': return { @@ -129,12 +162,24 @@ function resolutionOf(vpid, mode, target2) { promiseID: vpid, data: capargs(4, []), }; + case 'promise-data': + return { + type: 'fulfillToData', + promiseID: vpid, + data: capargs([slot0arg], [targets.p1]), + }; case 'reject': return { type: 'reject', promiseID: vpid, data: capargs('error', []), }; + case 'promise-reject': + return { + type: 'reject', + promiseID: vpid, + data: capargs([slot0arg], [targets.p1]), + }; default: throw Error(`unknown mode ${mode}`); } @@ -150,7 +195,7 @@ async function doVatResolveCase1(t, mode) { async run(target1, target2) { const p1 = pr.promise; E(target1).one(p1); - resolvePR(pr, mode, target2); + resolvePR(pr, mode, { target2, p1 }); // TODO: this stall shouldn't be necessary, but if I omit it, the // fulfillToPresence happens *after* two() is sent await Promise.resolve(); @@ -161,11 +206,10 @@ async function doVatResolveCase1(t, mode) { const dispatch = makeLiveSlots(syscall, {}, build, 'vatA'); t.deepEqual(log, []); - const slot0arg = { '@qclass': 'slot', index: 0 }; - const slot1arg = { '@qclass': 'slot', index: 1 }; const rootA = 'o+0'; const target1 = 'o-1'; const target2 = 'o-2'; + const localTarget = 'o+1'; const expectedP1 = 'p+5'; const expectedP2 = 'p+6'; const expectedP3 = 'p+7'; @@ -189,7 +233,8 @@ async function doVatResolveCase1(t, mode) { t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP2 }); // next the vat should resolve the promise it created - t.deepEqual(log.shift(), resolutionOf(expectedP1, mode, target2)); + const targets = { target2, localTarget, p1: expectedP1 }; + t.deepEqual(log.shift(), resolutionOf(expectedP1, mode, targets)); // then it should send 'two'. For now it should cite the same promise ID, // but in the future that vpid will have been retired, and we should see a @@ -207,57 +252,96 @@ async function doVatResolveCase1(t, mode) { t.end(); } -test('liveslots vpid handling case1 presence', async t => { - await doVatResolveCase1(t, 'presence'); -}); - -test('liveslots vpid handling case1 data', async t => { - await doVatResolveCase1(t, 'data'); -}); +for (const mode of modes) { + test(`liveslots vpid handling case1 ${mode}`, async t => { + await doVatResolveCase1(t, mode); + }); +} -test('liveslots vpid handling case1 reject', async t => { - await doVatResolveCase1(t, 'reject'); -}); +// This exercises cases 2 and 3, which are unusual in that this vat is both a +// subscriber and the decider for the same promise. The decision-making +// authority is exercised when the vat resolves the Promise that was returned +// from an inbound `result()` message. Liveslots always notifies the kernel +// about this act of resolution. + +// In the current code, the kernel then echoes the resolution back into the +// vat (by delivering a `dispatch.notifyFulfillTo..` message in a subsequent +// crank). In the upcoming retire-promiseid branch, the kernel does not, and +// liveslots must notice that we're also subscribed to this promise, and +// exercise the resolver/rejector that would normally be waiting for a +// message from the kernel. + +// Ordering guarantees: we don't intend to make any promises (haha) about the +// relative ordering of the syscall that resolves the promise, and the +// syscall.sends which convey messages that were queued up before or after +// the resolve. The syscall.sends need to appear in the same order they were +// sent, but the resolve is allowed to appear anywhere in the middle of (or +// even after) the sends. This test happens to assert a specific total +// ordering, but retaining this order is not the purpose of the test. (For +// determinism's sake, we might want to assert that it always uses the same +// otherwise-arbitrary order). If the total order changes, we should +// understand *why* it changed, and make sure the new order meets the +// guarantees that we *do* provide, and then update this test to match. async function doVatResolveCase23(t, which, mode, stalls) { // case 2 and 3 const { log, syscall } = buildSyscall(); + let resolutionOfP1; + + let stashP1; function build(E) { let p1; const pr = producePromise(); + const p0 = pr.promise; return harden({ - async promise(p) { + promise(p) { p1 = p; + stashP1 = p1; + p1.then( + x => { + // console.log(`p1 resolved to`, x); + resolutionOfP1 = x; + }, + _ => { + // console.log(`p1 rejected`); + resolutionOfP1 = 'rejected'; + }, + ); }, - async result() { - return pr.promise; + result() { + return p0; }, async run(target1, target2) { + // console.log(`calling one()`); const p2 = E(target1).one(p1); hush(p2); + // console.log(`calling two()`); const p3 = E(p1).two(); - if (mode === 'data' || mode === 'reject') { - hush(p3); - } + hush(p3); // The call to our result() method gave our vat access to p1 (along // with resolution authority), but it didn't give this user-level - // code access to p1. When we resolve the `pr.promise` we returned - // from `result`, liveslots will resolve p1 for us. But remember that - // pr.promise !== p1 - resolvePR(pr, mode, target2); + // code access to p1. When we resolve the p0 we returned from + // `result`, liveslots will resolve p1 for us. But remember that p0 + // !== p1 . This resolution will eventually cause a + // `syscall.fulfillToData` (or related) call into the kernel, and + // will eventually cause `p1` to be resolved to something, which may + // affect both where previously-queued messages (two) wind up, and + // where subsequently sent messages (four) wind up. + + resolvePR(pr, mode, { target2, p1 }); // We've started the resolution process, but it cannot complete for // another few turns. We wait some number of turns before using p1 - // again. + // again, to exercise as many race conditions as possible. for (let i = 0; i < stalls; i += 1) { // eslint-disable-next-line no-await-in-loop await Promise.resolve(); } - // If we don't stall here, or only stall one turn, then all four - // messages get pipelined out before we tell the kernel about the - // resolution (syscall.fulfillToPresence). + // If we don't stall here, then all four messages get pipelined out + // before we tell the kernel about the resolution + // (syscall.fulfillToPresence). // If we stall two turns, then the fulfillToPresence goes to the // kernel before three() and four(), but our 'p1' is not yet marked @@ -269,22 +353,22 @@ async function doVatResolveCase23(t, which, mode, stalls) { // If we stall a full three turns, then four() is sent directly to // target2. + // console.log(`calling three()`); const p4 = E(target1).three(p1); hush(p4); + // console.log(`calling four()`); const p5 = E(p1).four(); - if (mode === 'data' || mode === 'reject') { - hush(p5); - } + hush(p5); + // console.log(`did all calls`); }, }); } const dispatch = makeLiveSlots(syscall, {}, build, 'vatA'); t.deepEqual(log, []); - const slot0arg = { '@qclass': 'slot', index: 0 }; - const slot1arg = { '@qclass': 'slot', index: 1 }; const rootA = 'o+0'; const target1 = 'o-1'; + const localTarget = 'o+1'; const p1 = 'p-8'; const expectedP2 = 'p+5'; const expectedP3 = 'p+6'; @@ -313,7 +397,30 @@ async function doVatResolveCase23(t, which, mode, stalls) { ); await endOfCrank(); - // first the vat sends one() with the promise + // At the end of the turn in which run() is executed, the promise queue + // will contain deliveries one() and two() (specifically invocations of the + // handler on target1 and p1), plus the resolution of the promise p0 that + // was returned by result(). + + // If stalls=0, it will also have deliveries three() and four(). The crank + // keeps running until the promise queue is empty, so it executes handlers + // one(), two(), the thenResolve() for p0, then three() and four(). If any + // of these handlers were to push something onto the promise queue, that + // something would run after the four() handler runs. + + // If stalls=1, it executes the handlers for one(), two(), and the + // thenResolve() for p0. Then it pushes a stall onto the queue, so if any + // of those three were to push something onto the promise queue, it would + // run *before* three() or four() were pushed. + + // It's conceivable that a handler might push something onto the end, which + // when run pushes something *else* onto the end, etc. We use stalls=2 or + // more to exercise these cases. + + // Here, we examine the full set of syscalls emitted by these handlers. + + // When the one() handler is run, the vat sends one() with promise p1 as an + // argument t.deepEqual(log.shift(), { type: 'send', targetSlot: target1, @@ -323,7 +430,7 @@ async function doVatResolveCase23(t, which, mode, stalls) { }); t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP2 }); - // then the vat pipelines 'two' to the promise we gave them + // The two() handler pipelines 'two' to p1, the promise we gave the vat t.deepEqual(log.shift(), { type: 'send', targetSlot: p1, @@ -337,23 +444,37 @@ async function doVatResolveCase23(t, which, mode, stalls) { // for(let l of log) { // console.log(l); // } - - // then it resolves p1, which was used as the result of rootA~.result() - - // At this point, Liveslots and HandledPromise have not yet seen the - // resolution of p1: that will occur over the next few turns. The vat now - // stalls 0 to 3 turns to exercise the various possible states of Liveslots - // and HandledPromise. After those stalls, the vat sends three() with the - // promise (as an argument). Then it sends four() *to* the promise (as a - // target). - - // Here we build up the syscalls we expect to see. - + // return t.end(); + + // Now liveslots processes the callback ("notifySuccess", mapped to a + // function returned by "thenResolve") that got pushed when p0 was + // resolved, where p0 is the promise that was returned from rootA~.result() + // . This callback sends `syscall.fulfillToPresence(vpid1, stuff)` (or one + // of the other fulfill/reject variants) into the kernel, notifying any + // remote subscribers that p1 has been resolved. Since the vat is also a + // subscriber, thenResolve's callback must also invoke p1's resolver (which + // was stashed in importedPromisesByPromiseID), as if the kernel had call + // the vat's dispatch.notifyFulfillToPresence. This causes the p1.then + // callback to be pushed to the back of the promise queue, which will set + // resolutionOfP1 after all the syscalls have been made. + + // Resolving p1 changes the handler of p1, so subsequent messages sent to + // p1 will instead be sent to its resolution (which will be target2, or a + // local object, or something that causes rejections). These messages are + // already in the promise queue at this point, but their handlers haven't + // been invoked yet. We exercise the way HandledPromise changes the handler + // immediately, by looking at the syscalls issued (or not) by the callbacks + // for three() and four(). + + // At this point, we expect to see the vat tell the kernel that p1 is + // resolved. + const targets = { target2, localTarget, p1 }; + t.deepEqual(log.shift(), resolutionOf(p1, mode, targets)); + + // The VPIDs in the remaining messages will depend upon whether we retired + // p1 during that resolution. const RETIRE_VPIDS = false; - const expectedFulfill = [resolutionOf(p1, mode, target2)]; - const expectedThree = []; - const expectedFour = []; let expectedVPIDInThree = p1; let expectedResultOfThree = expectedP4; let expectedResultOfFour = expectedP5; @@ -364,69 +485,65 @@ async function doVatResolveCase23(t, which, mode, stalls) { expectedResultOfFour = expectedP6; } - // At present, three() always references the promise. This should change - // once we retire the vpid during resolution: it will need to allocate a - // new vpid. As a consequence, all the resultSlot vpids will increment too. - expectedThree.push({ + // three() references the old promise, which will use a new VPID if we + // retired the old one as it was resolved + t.deepEqual(log.shift(), { type: 'send', targetSlot: target1, method: 'three', args: capargs([slot0arg], [expectedVPIDInThree]), resultSlot: expectedResultOfThree, }); - expectedThree.push({ type: 'subscribe', target: expectedResultOfThree }); - - // At present, four() is always pipelined to the promise. This should - // change: once the vat knows what the promise has resolved to, it should - // send the message there directly. If mode === 'presence', the kernel - // should see a send() of four() directly to target2. If mode === 'data' or - // 'reject', it should reject locally, and the kernel should never see a - // syscall that sends four(). We explored some of this in #823, and code - // was landed to address it, but the behavior is still kind of wrong. - if (RETIRE_VPIDS) { - if (mode === 'presence') { - expectedFour.push({ - type: 'send', - targetSlot: target2, - method: 'four', - args: capargs([], []), - resultSlot: expectedResultOfFour, - }); - expectedFour.push({ type: 'subscribe', target: expectedResultOfFour }); - } - // but if we resolved p1 to data or rejected it, four() is not sent (the - // vat creates an error for it locally), and the kernel shouldn't see - // anything - } else { - expectedFour.push({ + t.deepEqual(log.shift(), { + type: 'subscribe', + target: expectedResultOfThree, + }); + + // four() is sent *to* the old promise, which means it's sent to the + // resolution target. If that target is a Presence, we'll see a syscall, + // otherwise the vat handles it internally. + + if (mode === 'presence') { + t.deepEqual(log.shift(), { type: 'send', - targetSlot: p1, + targetSlot: target2, method: 'four', args: capargs([], []), resultSlot: expectedResultOfFour, }); - expectedFour.push({ type: 'subscribe', target: expectedResultOfFour }); + t.deepEqual(log.shift(), { + type: 'subscribe', + target: expectedResultOfFour, + }); } - // TODO: the behavior ideally shouldn't depend upon how long we stall after - // resolution + // that's all the syscalls we should see + t.deepEqual(log, []); - let expected; - if (stalls === 0 || stalls === 1) { - // for some reason, if we don't stall long enough, the kernel sees - // three() and four() before it sees the resolution - expected = expectedThree.concat(expectedFour).concat(expectedFulfill); - } else { - expected = expectedFulfill.concat(expectedThree).concat(expectedFour); + // assert that the vat saw the local promise being resolved too + if (mode === 'presence') { + t.equal(resolutionOfP1.toString(), `[Presence ${target2}]`); + } else if (mode === 'data') { + t.equal(resolutionOfP1, 4); + } else if (mode === 'promise-data') { + t.equal(resolutionOfP1 instanceof Array, true); + t.equal(resolutionOfP1.length, 1); + t.is(resolutionOfP1[0], Promise.resolve(resolutionOfP1[0])); + t.is(resolutionOfP1[0], stashP1); + } else if (mode === 'reject') { + t.equal(resolutionOfP1, 'rejected'); } - t.deepEqual(log, expected); - t.end(); } +// uncomment this when debugging specific problems +// test.only(`XX`, async t => { +// await doVatResolveCase23(t, 2, 'presence', 0); +// }); + for (const caseNum of [2, 3]) { - for (const mode of ['presence', 'data', 'reject']) { + for (const mode of modes) { for (let stalls = 0; stalls < 4; stalls += 1) { test(`liveslots vpid handling case${caseNum} ${mode} stalls=${stalls}`, async t => { await doVatResolveCase23(t, caseNum, mode, stalls); @@ -445,7 +562,7 @@ async function doVatResolveCase4(t, mode) { p1 = p; // if we don't add this, node will complain when the kernel notifies // us of the rejection - p1.catch(_e => undefined); + hush(p1); }, async first(target1) { const p2 = E(target1).one(p1); @@ -459,12 +576,15 @@ async function doVatResolveCase4(t, mode) { const p5 = E(p1).four(); hush(p5); }, + // we re-use this root object as a resolution of p1 the 'local-object' + // case, so make sure it can accept the messages + two() {}, + four() {}, }); } const dispatch = makeLiveSlots(syscall, {}, build, 'vatA'); t.deepEqual(log, []); - const slot0arg = { '@qclass': 'slot', index: 0 }; const rootA = 'o+0'; const target1 = 'o-1'; const p1 = 'p-8'; @@ -507,15 +627,26 @@ async function doVatResolveCase4(t, mode) { if (mode === 'presence') { dispatch.notifyFulfillToPresence(p1, target2); + } else if (mode === 'local-object') { + dispatch.notifyFulfillToPresence(p1, rootA); } else if (mode === 'data') { dispatch.notifyFulfillToData(p1, capargs(4, [])); + } else if (mode === 'promise-data') { + dispatch.notifyFulfillToData(p1, capargs([slot0arg], [p1])); } else if (mode === 'reject') { dispatch.notifyReject(p1, capargs('error', [])); + } else if (mode === 'promise-reject') { + dispatch.notifyReject(p1, capargs([slot0arg], [p1])); } else { throw Error(`unknown mode ${mode}`); } await endOfCrank(); t.deepEqual(log, []); + // TODO: when we switch to retiring VPIDs in the syscall.notifyFulfill* + // direction, this is the point at which both vat and kernel agree to stop + // using the 'p1' identifier. Subsequent sycalls from the vat will allocate + // a new VPID for that promise, and the vat will emit a syscall.fulfillTo* + // shortly after mentioning the new identifier. dispatch.deliver(rootA, 'second', capargs([slot0arg], [target1])); await endOfCrank(); @@ -531,8 +662,6 @@ async function doVatResolveCase4(t, mode) { t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP4 }); if (mode === 'presence') { - // this message seems to get sent to target2 even though #823 isn't fixed - // yet, I don't know why const expectedP5 = nextP(); t.deepEqual(log.shift(), { type: 'send', @@ -549,18 +678,10 @@ async function doVatResolveCase4(t, mode) { t.end(); } -test('liveslots vpid handling case4 presence', async t => { - await doVatResolveCase4(t, 'presence'); -}); - -test('liveslots vpid handling case4 data', async t => { - await doVatResolveCase4(t, 'data'); -}); - -// broken due to #823, the four() method is pipelined to p1, when it should -// be delivered and rejected within the vat -test('liveslots vpid handling case4 reject', async t => { - await doVatResolveCase4(t, 'reject'); -}); +for (const mode of modes) { + test(`liveslots vpid handling case4 ${mode}`, async t => { + await doVatResolveCase4(t, mode); + }); +} // cases 5 and 6 are not implemented due to #886