From 714b214012e81faf2ac4955475a8504ef0c74a4a Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Mon, 24 May 2021 10:19:13 -0600 Subject: [PATCH 01/31] feat: implement exportAsSyncable, and Sync powers --- packages/captp/lib/captp.js | 92 +++++++++++++++++++++++++++----- packages/captp/lib/sync.js | 66 +++++++++++++++++++++++ packages/captp/lib/ts-types.d.ts | 50 +++++++++++++++++ packages/captp/lib/types.js | 14 +++++ packages/captp/package.json | 1 + 5 files changed, 211 insertions(+), 12 deletions(-) create mode 100644 packages/captp/lib/sync.js create mode 100644 packages/captp/lib/ts-types.d.ts create mode 100644 packages/captp/lib/types.js diff --git a/packages/captp/lib/captp.js b/packages/captp/lib/captp.js index 22a07a57d08..bff3201450e 100644 --- a/packages/captp/lib/captp.js +++ b/packages/captp/lib/captp.js @@ -8,10 +8,16 @@ import { Remotable as defaultRemotable, Far as defaultFar, makeMarshal as defaultMakeMarshal, + getInterfaceOf as defaultGetInterfaceOf, QCLASS, } from '@agoric/marshal'; import { E, HandledPromise } from '@agoric/eventual-send'; import { isPromise } from '@agoric/promise-kit'; +import { assert, details as X } from '@agoric/assert'; + +import { makeSync } from './sync.js'; + +import './types.js'; export { E }; @@ -26,7 +32,9 @@ export { E }; * @property {typeof defaultRemotable} Remotable * @property {typeof defaultFar} Far * @property {typeof defaultMakeMarshal} makeMarshal + * @property {typeof defaultGetInterfaceOf} getInterfaceOf * @property {number} epoch + * @property {GetOrApplySync} getOrApplySync */ /** * Create a CapTP connection. @@ -41,7 +49,9 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { onReject = err => console.error('CapTP', ourId, 'exception:', err), Remotable = defaultRemotable, makeMarshal = defaultMakeMarshal, + getInterfaceOf = defaultGetInterfaceOf, epoch = 0, + getOrApplySync: rawGetOrApplySync, } = opts; const disconnectReason = id => @@ -92,8 +102,10 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { }, ); + /** @type {WeakMap} */ const valToSlot = new WeakMap(); // exports looked up by val const slotToVal = new Map(); // reverse + const syncableExported = new WeakSet(); // Used to construct slot names for promises/non-promises. // In this verison of CapTP we use strings for export/import slot names. @@ -104,8 +116,11 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { // the question key let lastQuestionID = 0; + /** @type {Map} */ const questions = new Map(); // chosen by us + /** @type {Map} */ const answers = new Map(); // chosen by our peer + /** @type {Map} */ const imports = new Map(); // chosen by our peer // Called at marshalling time. Either retrieves an existing export, or if @@ -140,12 +155,16 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { }), ); } else { - // Since this isn't a promise, we instead increment the lastExportId - // and use that to construct the slot name. - // Non-promises are prefaced with 'o+'. + // Since this isn't a promise, we instead increment the lastExportId and + // use that to construct the slot name. Non-promises are prefaced with + // 'o+' for normal objects, or `s+` for syncable. lastExportID += 1; const exportID = lastExportID; - slot = `o+${exportID}`; + if (syncableExported.has(val)) { + slot = `s+${exportID}`; + } else { + slot = `o+${exportID}`; + } } // Now record the export in both valToSlot and slotToVal so we can look it // up from either the value or the slot name later. @@ -238,7 +257,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { if (!slotToVal.has(slot)) { // Make a new handled promise for the slot. const pr = makeRemoteKit(slot); - if (slot[0] === 'o') { + if (slot[0] === 'o' || slot[0] === 's') { // A new remote presence const pres = pr.resPres(); if (iface === undefined) { @@ -411,14 +430,62 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { dispatch({ type: 'CTP_DISCONNECT', epoch, reason }); }; - return harden({ abort, dispatch, getBootstrap, serialize, unserialize }); + const exportAsSyncable = obj => { + assert( + getInterfaceOf(obj) !== undefined, + X`exportAsSyncable(${obj}) argument must be Remotable`, + ); + syncableExported.add(obj); + return obj; + }; + + /** @type {{Sync?: Sync}} */ + const addSync = {}; + + if (rawGetOrApplySync) { + /** @type {GetOrApplySync} */ + const getOrApplySync = (target, prop, methodArgs = undefined) => { + assert( + Promise.resolve(target) !== target, + X`Sync(${target}) target cannot be a promise`, + ); + + const slot = valToSlot.get(target); + assert( + slot && slot[1] === '-', + X`Sync(${target}) target was not imported`, + ); + assert( + slot[0] === 's', + X`Sync(${target}) imported target was not exportAsSyncable`, + ); + assert( + rawGetOrApplySync, + X`Sync(${target}) failed; no opts.getOrApplySync supplied to makeCapTP`, + ); + return rawGetOrApplySync(target, prop, methodArgs); + }; + + addSync.Sync = makeSync(getOrApplySync); + } + + return harden({ + abort, + dispatch, + getBootstrap, + serialize, + unserialize, + exportAsSyncable, + ...addSync, + }); } /** * Create an async-isolated channel to an object. * * @param {string} ourId - * @returns {{ makeFar(x: T): ERef, makeNear(x: T): ERef }} + * @returns {{ makeFar(x: T): ERef, makeNear(x: T): ERef, + * exportAsSyncable(x: T): T }} */ export function makeLoopback(ourId) { let nextNonce = 0; @@ -438,11 +505,11 @@ export function makeLoopback(ourId) { // Create the tunnel. let farDispatch; - const { dispatch: nearDispatch, getBootstrap: getFarBootstrap } = makeCapTP( - `near-${ourId}`, - o => farDispatch(o), - bootstrap, - ); + const { + dispatch: nearDispatch, + exportAsSyncable, + getBootstrap: getFarBootstrap, + } = makeCapTP(`near-${ourId}`, o => farDispatch(o), bootstrap); const { dispatch, getBootstrap: getNearBootstrap } = makeCapTP( `far-${ourId}`, nearDispatch, @@ -470,5 +537,6 @@ export function makeLoopback(ourId) { return { makeFar: makeRefMaker(farGetter), makeNear: makeRefMaker(nearGetter), + exportAsSyncable, }; } diff --git a/packages/captp/lib/sync.js b/packages/captp/lib/sync.js new file mode 100644 index 00000000000..e001346cf65 --- /dev/null +++ b/packages/captp/lib/sync.js @@ -0,0 +1,66 @@ +// Lifted mostly from `@agoric/eventual-send/src/E.js`. + +import './types'; + +const readOnlyProxyHandler = { + set(_target, _prop, _value) { + return false; + }, + isExtensible(_target) { + return false; + }, + setPrototypeOf(_target, _value) { + return false; + }, + deleteProperty(_target, _prop) { + return false; + }, +}; + +/** + * A Proxy handler for Sync(x) + * + * @param {*} x Any value passed to Sync(x) + * @param {GetOrApplySync} getOrApplySync + * @returns {ProxyHandler} + */ +function SyncProxyHandler(x, getOrApplySync) { + return harden({ + ...readOnlyProxyHandler, + get(_target, p, _receiver) { + return (...args) => getOrApplySync(x, p, args); + }, + apply(_target, _thisArg, argArray = []) { + return getOrApplySync(x, null, argArray); + }, + has(_target, _p) { + // We just pretend everything exists. + return true; + }, + }); +} + +/** + * @param {GetOrApplySync} getOrApplySync + * @returns {Sync} + */ +export function makeSync(getOrApplySync) { + function Sync(x) { + const handler = SyncProxyHandler(x, getOrApplySync); + return harden(new Proxy(() => {}, handler)); + } + + const makeSyncGetterProxy = x => + new Proxy(Object.create(null), { + ...readOnlyProxyHandler, + has(_target, prop) { + return getOrApplySync(x, prop) !== undefined; + }, + get(_target, prop) { + return getOrApplySync(x, prop); + }, + }); + Sync.get = makeSyncGetterProxy; + + return harden(Sync); +} diff --git a/packages/captp/lib/ts-types.d.ts b/packages/captp/lib/ts-types.d.ts new file mode 100644 index 00000000000..b0087e0643e --- /dev/null +++ b/packages/captp/lib/ts-types.d.ts @@ -0,0 +1,50 @@ +/* eslint-disable */ +// eslint-disable-next-line spaced-comment + +type ERef = PromiseLike | T; +type Unpromise = T extends ERef ? U : T; + +export type Syncable = T extends (...args: infer P) => infer R + ? (...args: P) => Unpromise + : T extends Record + ? { + [K in keyof T]: Unpromise; + } + : T; + +/* Types for Sync proxy calls. */ +type SyncSingleMethod = { + readonly [P in keyof T]: ( + ...args: Parameters + ) => Unpromise>; +} +type SyncSingleCall = T extends Function ? + ((...args: Parameters) => Unpromise>) & + ESingleMethod> : ESingleMethod>; +type SyncSingleGet = { + readonly [P in keyof T]: Unpromise; +} + +export interface Sync { + /** + * Sync(x) returns a proxy on which you can call arbitrary methods. Each of + * these method calls will unwrap a promise result. The method will be + * invoked on a remote 'x', and be synchronous from the perspective of this + * caller. + * + * @param {*} x target for method/function call + * @returns {SyncSingleCall} method/function call proxy + */ + (x: T): SyncSingleCall>; + + /** + * Sync.get(x) returns a proxy on which you can get arbitrary properties. + * Each of these properties unwraps a promise result. The value will be the + * property fetched from a remote 'x', and be synchronous from the perspective + * of this caller. + * + * @param {*} x target for property get + * @returns {SyncSingleGet} property get proxy + */ + readonly get(x: T): SyncSingleGet>; +} diff --git a/packages/captp/lib/types.js b/packages/captp/lib/types.js new file mode 100644 index 00000000000..ba1e411802d --- /dev/null +++ b/packages/captp/lib/types.js @@ -0,0 +1,14 @@ +/** + * @callback GetApplySync + * @param {any} target + * @param {string | symbol | number | null} prop the property or method name, + * null if this is a function call of target + * @param {Array} [methodArgs] any function or method arguments + */ + +/** @typedef {import('./ts-types').Sync} Sync */ + +/** + * @template T + * @typedef {import('./ts-types').Syncable} Syncable + */ diff --git a/packages/captp/package.json b/packages/captp/package.json index 4a36ccb06dd..d41f7057c0c 100644 --- a/packages/captp/package.json +++ b/packages/captp/package.json @@ -39,6 +39,7 @@ "ava": "^3.12.1" }, "dependencies": { + "@agoric/assert": "^0.3.6", "@agoric/eventual-send": "^0.13.22", "@agoric/marshal": "^0.4.19", "@agoric/nat": "^4.1.0", From 06168be096fea07feb456d80433375e93af15fe1 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Mon, 24 May 2021 22:17:58 -0600 Subject: [PATCH 02/31] test: update tests for Sync/exportAsSyncable --- packages/captp/test/test-disco.js | 24 ++++----- packages/captp/test/test-sync.js | 84 +++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 14 deletions(-) create mode 100644 packages/captp/test/test-sync.js diff --git a/packages/captp/test/test-disco.js b/packages/captp/test/test-disco.js index 7753efedd46..47271bcd352 100644 --- a/packages/captp/test/test-disco.js +++ b/packages/captp/test/test-disco.js @@ -9,13 +9,11 @@ test('try disconnecting captp', async t => { const { getBootstrap, abort } = makeCapTP( 'us', obj => objs.push(obj), - // TODO Can we avoid the function wrapper? makeCapTP does the needed test - () => - Far('test hello', { - method() { - return 'hello'; - }, - }), + Far('test hello', { + method() { + return 'hello'; + }, + }), { onReject(e) { rejected.push(e); @@ -70,13 +68,11 @@ test('try aborting captp with reason', async t => { const { getBootstrap, abort } = makeCapTP( 'us', obj => objs.push(obj), - // TODO Can we avoid the function wrapper? makeCapTP does the needed test - () => - Far('test hello', { - method() { - return 'hello'; - }, - }), + Far('test hello', { + method() { + return 'hello'; + }, + }), { onReject(e) { rejected.push(e); diff --git a/packages/captp/test/test-sync.js b/packages/captp/test/test-sync.js new file mode 100644 index 00000000000..cb9d7bc34b2 --- /dev/null +++ b/packages/captp/test/test-sync.js @@ -0,0 +1,84 @@ +import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava'; +import { Far } from '@agoric/marshal'; +import { E, makeCapTP, makeLoopback } from '../lib/captp'; +import { nearGetApplySync } from '../lib/sync'; + +function createFarBootstrap(exportAsSyncable) { + // Create a remotable that has a syncable return value. + return Far('test sync', { + getSyncable(n) { + const syncable = exportAsSyncable( + Far('getN', { + getN() { + return n; + }, + }), + ); + return syncable; + }, + }); +} + +async function runSyncTests(t, Sync, bs) { + // Demonstrate async compatibility of syncable. + const pn = E(E(bs).getSyncable(3)).getN(); + t.is(Promise.resolve(pn), pn); + t.is(await pn, 3); + + // Demonstrate Sync cannot be used on a promise. + const ps = E(bs).getSyncable(4); + t.throws(() => Sync(ps).getN(), { + instanceOf: Error, + message: /target cannot be a promise/, + }); + + // Demonstrate Sync used on a remotable. + const s = await ps; + t.is(Sync(s).getN(), 4); + + // Demonstrate Sync fails on an unmarked remotable. + const b = await bs; + t.throws(() => Sync(b).getSyncable(5), { + instanceOf: Error, + message: /imported target was not exportAsSyncable/, + }); +} + +test('try loopback syncable', async t => { + const { makeFar, Sync, exportAsSyncable } = makeLoopback('us'); + const bs = makeFar(createFarBootstrap(exportAsSyncable)); + + await runSyncTests(t, Sync, bs); +}); + +test('try explicit syncable', async t => { + let farDispatch; + const { dispatch: nearDispatch, getBootstrap, Sync } = makeCapTP( + 'near', + o => farDispatch(o), + undefined, + { + getApplySync(slot, prop, methodArgs = undefined) { + // Cross the boundary to pull out the far object. + const body = JSON.stringify({ + '@qclass': 'slot', + index: 0, + }); + // eslint-disable-next-line no-use-before-define + const far = farUnserialize({ body, slots: [slot] }); + return nearGetApplySync(far, prop, methodArgs); + }, + }, + ); + const { + dispatch, + exportAsSyncable, + unserialize: farUnserialize, + } = makeCapTP('far', nearDispatch, () => + createFarBootstrap(exportAsSyncable), + ); + farDispatch = dispatch; + + const bs = getBootstrap(); + await runSyncTests(t, Sync, bs); +}); From 3d500a101d73995d434cbb48b9f5be206a076ed7 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Mon, 24 May 2021 22:18:20 -0600 Subject: [PATCH 03/31] feat: implement Sync for makeLoopback --- packages/captp/lib/captp.js | 64 +++++++++++++++++++++++++------------ packages/captp/lib/sync.js | 38 ++++++++++++++++------ 2 files changed, 72 insertions(+), 30 deletions(-) diff --git a/packages/captp/lib/captp.js b/packages/captp/lib/captp.js index bff3201450e..f74aab94b0c 100644 --- a/packages/captp/lib/captp.js +++ b/packages/captp/lib/captp.js @@ -15,7 +15,7 @@ import { E, HandledPromise } from '@agoric/eventual-send'; import { isPromise } from '@agoric/promise-kit'; import { assert, details as X } from '@agoric/assert'; -import { makeSync } from './sync.js'; +import { makeSync, nearGetApplySync } from './sync.js'; import './types.js'; @@ -34,7 +34,7 @@ export { E }; * @property {typeof defaultMakeMarshal} makeMarshal * @property {typeof defaultGetInterfaceOf} getInterfaceOf * @property {number} epoch - * @property {GetOrApplySync} getOrApplySync + * @property {GetApplySync} getApplySync */ /** * Create a CapTP connection. @@ -51,7 +51,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { makeMarshal = defaultMakeMarshal, getInterfaceOf = defaultGetInterfaceOf, epoch = 0, - getOrApplySync: rawGetOrApplySync, + getApplySync: rawGetApplySync, } = opts; const disconnectReason = id => @@ -442,9 +442,9 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { /** @type {{Sync?: Sync}} */ const addSync = {}; - if (rawGetOrApplySync) { - /** @type {GetOrApplySync} */ - const getOrApplySync = (target, prop, methodArgs = undefined) => { + if (rawGetApplySync) { + /** @type {GetApplySync} */ + const getApplySync = (target, prop, methodArgs = undefined) => { assert( Promise.resolve(target) !== target, X`Sync(${target}) target cannot be a promise`, @@ -460,13 +460,13 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { X`Sync(${target}) imported target was not exportAsSyncable`, ); assert( - rawGetOrApplySync, - X`Sync(${target}) failed; no opts.getOrApplySync supplied to makeCapTP`, + rawGetApplySync, + X`Sync(${target}) failed; no opts.getApplySync supplied to makeCapTP`, ); - return rawGetOrApplySync(target, prop, methodArgs); + return rawGetApplySync(slot, prop, methodArgs); }; - addSync.Sync = makeSync(getOrApplySync); + addSync.Sync = makeSync(getApplySync); } return harden({ @@ -480,20 +480,26 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { }); } +/** + * @typedef {Object} LoopbackOpts + * @property {typeof defaultFar} Far + */ + /** * Create an async-isolated channel to an object. * * @param {string} ourId + * @param {Partial} [opts] * @returns {{ makeFar(x: T): ERef, makeNear(x: T): ERef, - * exportAsSyncable(x: T): T }} + * exportAsSyncable(x: T): T, Sync: Sync }} */ -export function makeLoopback(ourId) { +export function makeLoopback(ourId, opts = {}) { + const { Far = defaultFar } = opts; let nextNonce = 0; const nonceToRef = new Map(); - // TODO use the correct Far, which is not currently in scope here. const bootstrap = harden({ - refGetter: defaultFar('captp bootstrap', { + refGetter: Far('refGetter', { getRef(nonce) { // Find the local ref for the specified nonce. const xFar = nonceToRef.get(nonce); @@ -503,18 +509,33 @@ export function makeLoopback(ourId) { }), }); + const slotBody = JSON.stringify({ + '@qclass': 'slot', + index: 0, + }); + // Create the tunnel. let farDispatch; const { dispatch: nearDispatch, - exportAsSyncable, + Sync, getBootstrap: getFarBootstrap, - } = makeCapTP(`near-${ourId}`, o => farDispatch(o), bootstrap); - const { dispatch, getBootstrap: getNearBootstrap } = makeCapTP( - `far-${ourId}`, - nearDispatch, - bootstrap, - ); + } = makeCapTP(`near-${ourId}`, o => farDispatch(o), bootstrap, { + getApplySync(slot, prop, methodArgs = undefined) { + // Cross the boundary to pull out the far object. + // eslint-disable-next-line no-use-before-define + const far = farUnserialize({ body: slotBody, slots: [slot] }); + return nearGetApplySync(far, prop, methodArgs); + }, + }); + assert(Sync); + + const { + dispatch, + exportAsSyncable, + getBootstrap: getNearBootstrap, + unserialize: farUnserialize, + } = makeCapTP(`far-${ourId}`, nearDispatch, bootstrap); farDispatch = dispatch; const farGetter = E.get(getFarBootstrap()).refGetter; @@ -538,5 +559,6 @@ export function makeLoopback(ourId) { makeFar: makeRefMaker(farGetter), makeNear: makeRefMaker(nearGetter), exportAsSyncable, + Sync, }; } diff --git a/packages/captp/lib/sync.js b/packages/captp/lib/sync.js index e001346cf65..bac58d6fcb9 100644 --- a/packages/captp/lib/sync.js +++ b/packages/captp/lib/sync.js @@ -2,6 +2,26 @@ import './types'; +/** + * Default implementation of GetApplySync. + * + * @type {GetApplySync} + */ +export const nearGetApplySync = harden( + (target, prop, methodArgs = undefined) => { + if (Array.isArray(methodArgs)) { + if (prop === null) { + // Function application. + return target(...methodArgs); + } + // Method application. + return target[prop](...methodArgs); + } + // Property get. + return target[prop]; + }, +); + const readOnlyProxyHandler = { set(_target, _prop, _value) { return false; @@ -21,17 +41,17 @@ const readOnlyProxyHandler = { * A Proxy handler for Sync(x) * * @param {*} x Any value passed to Sync(x) - * @param {GetOrApplySync} getOrApplySync + * @param {GetApplySync} getApplySync * @returns {ProxyHandler} */ -function SyncProxyHandler(x, getOrApplySync) { +function SyncProxyHandler(x, getApplySync) { return harden({ ...readOnlyProxyHandler, get(_target, p, _receiver) { - return (...args) => getOrApplySync(x, p, args); + return (...args) => getApplySync(x, p, args); }, apply(_target, _thisArg, argArray = []) { - return getOrApplySync(x, null, argArray); + return getApplySync(x, null, argArray); }, has(_target, _p) { // We just pretend everything exists. @@ -41,12 +61,12 @@ function SyncProxyHandler(x, getOrApplySync) { } /** - * @param {GetOrApplySync} getOrApplySync + * @param {GetApplySync} getApplySync * @returns {Sync} */ -export function makeSync(getOrApplySync) { +export function makeSync(getApplySync) { function Sync(x) { - const handler = SyncProxyHandler(x, getOrApplySync); + const handler = SyncProxyHandler(x, getApplySync); return harden(new Proxy(() => {}, handler)); } @@ -54,10 +74,10 @@ export function makeSync(getOrApplySync) { new Proxy(Object.create(null), { ...readOnlyProxyHandler, has(_target, prop) { - return getOrApplySync(x, prop) !== undefined; + return getApplySync(x, prop) !== undefined; }, get(_target, prop) { - return getOrApplySync(x, prop); + return getApplySync(x, prop); }, }); Sync.get = makeSyncGetterProxy; From b90ae0835aec5484279eddcea4e9ccaa253d2db0 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Mon, 24 May 2021 22:28:33 -0600 Subject: [PATCH 04/31] fix: don't create new promise IDs and stall the pipeline --- packages/captp/lib/captp.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/captp/lib/captp.js b/packages/captp/lib/captp.js index f74aab94b0c..d1efc05cc58 100644 --- a/packages/captp/lib/captp.js +++ b/packages/captp/lib/captp.js @@ -188,6 +188,21 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { // eslint-disable-next-line no-use-before-define const pr = makeRemoteKit(questionID); questions.set(questionID, pr); + + // To fix #2846: + // 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'. + lastPromiseID += 1; + const promiseID = lastPromiseID; + const resultVPID = `p+${promiseID}`; + valToSlot.set(pr.p, resultVPID); + slotToVal.set(resultVPID, pr.p); + return [questionID, pr]; } From 14552986c6e47fde7eae720e449efce5aab23707 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Mon, 31 May 2021 12:33:07 -0600 Subject: [PATCH 05/31] fix: break up incoherent GetApply function into SyncImpl record --- packages/captp/lib/captp.js | 43 ++++++++++++++++--------- packages/captp/lib/sync.js | 55 ++++++++++++++++---------------- packages/captp/lib/types.js | 15 +++++---- packages/captp/test/test-sync.js | 27 ++++++++++------ 4 files changed, 82 insertions(+), 58 deletions(-) diff --git a/packages/captp/lib/captp.js b/packages/captp/lib/captp.js index d1efc05cc58..d8b0f4c1669 100644 --- a/packages/captp/lib/captp.js +++ b/packages/captp/lib/captp.js @@ -15,7 +15,7 @@ import { E, HandledPromise } from '@agoric/eventual-send'; import { isPromise } from '@agoric/promise-kit'; import { assert, details as X } from '@agoric/assert'; -import { makeSync, nearGetApplySync } from './sync.js'; +import { makeSync, nearSyncImpl } from './sync.js'; import './types.js'; @@ -34,7 +34,7 @@ export { E }; * @property {typeof defaultMakeMarshal} makeMarshal * @property {typeof defaultGetInterfaceOf} getInterfaceOf * @property {number} epoch - * @property {GetApplySync} getApplySync + * @property {SyncImpl} syncImpl */ /** * Create a CapTP connection. @@ -51,7 +51,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { makeMarshal = defaultMakeMarshal, getInterfaceOf = defaultGetInterfaceOf, epoch = 0, - getApplySync: rawGetApplySync, + syncImpl: rawSyncImpl, } = opts; const disconnectReason = id => @@ -457,9 +457,8 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { /** @type {{Sync?: Sync}} */ const addSync = {}; - if (rawGetApplySync) { - /** @type {GetApplySync} */ - const getApplySync = (target, prop, methodArgs = undefined) => { + if (rawSyncImpl) { + const makeSyncImpl = implMethod => (target, ...args) => { assert( Promise.resolve(target) !== target, X`Sync(${target}) target cannot be a promise`, @@ -475,13 +474,21 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { X`Sync(${target}) imported target was not exportAsSyncable`, ); assert( - rawGetApplySync, - X`Sync(${target}) failed; no opts.getApplySync supplied to makeCapTP`, + rawSyncImpl, + X`Sync(${target}) failed; no opts.syncImpl supplied to makeCapTP`, ); - return rawGetApplySync(slot, prop, methodArgs); + return rawSyncImpl[implMethod](slot, ...args); }; - addSync.Sync = makeSync(getApplySync); + /** @type {SyncImpl} */ + const syncImpl = { + applyFunction: makeSyncImpl('applyFunction'), + applyMethod: makeSyncImpl('applyMethod'), + get: makeSyncImpl('get'), + }; + harden(syncImpl); + + addSync.Sync = makeSync(syncImpl); } return harden({ @@ -529,6 +536,13 @@ export function makeLoopback(ourId, opts = {}) { index: 0, }); + const makeFarSyncImpl = implMethod => (slot, ...args) => { + // Cross the boundary to pull out the far object. + // eslint-disable-next-line no-use-before-define + const far = farUnserialize({ body: slotBody, slots: [slot] }); + return nearSyncImpl[implMethod](far, ...args); + }; + // Create the tunnel. let farDispatch; const { @@ -536,11 +550,10 @@ export function makeLoopback(ourId, opts = {}) { Sync, getBootstrap: getFarBootstrap, } = makeCapTP(`near-${ourId}`, o => farDispatch(o), bootstrap, { - getApplySync(slot, prop, methodArgs = undefined) { - // Cross the boundary to pull out the far object. - // eslint-disable-next-line no-use-before-define - const far = farUnserialize({ body: slotBody, slots: [slot] }); - return nearGetApplySync(far, prop, methodArgs); + syncImpl: { + applyFunction: makeFarSyncImpl('applyFunction'), + applyMethod: makeFarSyncImpl('applyMethod'), + get: makeFarSyncImpl('get'), }, }); assert(Sync); diff --git a/packages/captp/lib/sync.js b/packages/captp/lib/sync.js index bac58d6fcb9..9a25ce3b246 100644 --- a/packages/captp/lib/sync.js +++ b/packages/captp/lib/sync.js @@ -1,26 +1,24 @@ +// @ts-check // Lifted mostly from `@agoric/eventual-send/src/E.js`. import './types'; /** - * Default implementation of GetApplySync. + * Default implementation of Sync for near objects. * - * @type {GetApplySync} + * @type {SyncImpl} */ -export const nearGetApplySync = harden( - (target, prop, methodArgs = undefined) => { - if (Array.isArray(methodArgs)) { - if (prop === null) { - // Function application. - return target(...methodArgs); - } - // Method application. - return target[prop](...methodArgs); - } - // Property get. +export const nearSyncImpl = harden({ + applyFunction(target, args) { + return target(...args); + }, + applyMethod(target, prop, args) { + return target[prop](...args); + }, + get(target, prop) { return target[prop]; }, -); +}); const readOnlyProxyHandler = { set(_target, _prop, _value) { @@ -41,45 +39,48 @@ const readOnlyProxyHandler = { * A Proxy handler for Sync(x) * * @param {*} x Any value passed to Sync(x) - * @param {GetApplySync} getApplySync + * @param {SyncImpl} syncImpl * @returns {ProxyHandler} */ -function SyncProxyHandler(x, getApplySync) { +function SyncProxyHandler(x, syncImpl) { return harden({ ...readOnlyProxyHandler, get(_target, p, _receiver) { - return (...args) => getApplySync(x, p, args); + return (...args) => syncImpl.applyMethod(x, p, args); }, apply(_target, _thisArg, argArray = []) { - return getApplySync(x, null, argArray); + return syncImpl.applyFunction(x, argArray); }, has(_target, _p) { - // We just pretend everything exists. + // TODO: has property is not yet transferrable over captp. return true; }, }); } /** - * @param {GetApplySync} getApplySync + * @param {SyncImpl} syncImpl * @returns {Sync} */ -export function makeSync(getApplySync) { +export function makeSync(syncImpl) { function Sync(x) { - const handler = SyncProxyHandler(x, getApplySync); + const handler = SyncProxyHandler(x, syncImpl); return harden(new Proxy(() => {}, handler)); } - const makeSyncGetterProxy = x => - new Proxy(Object.create(null), { + const makeSyncGetterProxy = x => { + const handler = harden({ ...readOnlyProxyHandler, - has(_target, prop) { - return getApplySync(x, prop) !== undefined; + has(_target, _prop) { + // TODO: has property is not yet transferrable over captp. + return true; }, get(_target, prop) { - return getApplySync(x, prop); + return syncImpl.get(x, prop); }, }); + return new Proxy(Object.create(null), handler); + }; Sync.get = makeSyncGetterProxy; return harden(Sync); diff --git a/packages/captp/lib/types.js b/packages/captp/lib/types.js index ba1e411802d..b1795288080 100644 --- a/packages/captp/lib/types.js +++ b/packages/captp/lib/types.js @@ -1,14 +1,17 @@ /** - * @callback GetApplySync - * @param {any} target - * @param {string | symbol | number | null} prop the property or method name, - * null if this is a function call of target - * @param {Array} [methodArgs] any function or method arguments + * @typedef {Object} SyncImpl + * @property {(target: any, args: Array) => any} applyFunction function + * application + * @property {(target: any, method: string | symbol | number, args: Array) + * => any} applyMethod method invocation, which is an atomic lookup of method and + * apply + * @property {(target: any, prop: string | symbol | number) => any} get property + * lookup */ /** @typedef {import('./ts-types').Sync} Sync */ /** * @template T - * @typedef {import('./ts-types').Syncable} Syncable + * @typedef {import('./ts-types').Syncable} Syncable */ diff --git a/packages/captp/test/test-sync.js b/packages/captp/test/test-sync.js index cb9d7bc34b2..4c951171f09 100644 --- a/packages/captp/test/test-sync.js +++ b/packages/captp/test/test-sync.js @@ -1,7 +1,7 @@ import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava'; import { Far } from '@agoric/marshal'; import { E, makeCapTP, makeLoopback } from '../lib/captp'; -import { nearGetApplySync } from '../lib/sync'; +import { nearSyncImpl } from '../lib/sync'; function createFarBootstrap(exportAsSyncable) { // Create a remotable that has a syncable return value. @@ -52,21 +52,28 @@ test('try loopback syncable', async t => { }); test('try explicit syncable', async t => { + const makeFarSyncImpl = implMethod => (slot, ...args) => { + // Cross the boundary to pull out the far object. + const body = JSON.stringify({ + '@qclass': 'slot', + index: 0, + }); + // eslint-disable-next-line no-use-before-define + const far = farUnserialize({ body, slots: [slot] }); + return nearSyncImpl[implMethod](far, ...args); + }; + let farDispatch; const { dispatch: nearDispatch, getBootstrap, Sync } = makeCapTP( 'near', o => farDispatch(o), undefined, { - getApplySync(slot, prop, methodArgs = undefined) { - // Cross the boundary to pull out the far object. - const body = JSON.stringify({ - '@qclass': 'slot', - index: 0, - }); - // eslint-disable-next-line no-use-before-define - const far = farUnserialize({ body, slots: [slot] }); - return nearGetApplySync(far, prop, methodArgs); + syncImpl: { + applyFunction: makeFarSyncImpl('applyFunction'), + applyMethod: makeFarSyncImpl('applyMethod'), + get: makeFarSyncImpl('get'), + has: makeFarSyncImpl('has'), }, }, ); From c838e918164fc136b0bcbd83029489c6893ea381 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Tue, 1 Jun 2021 10:55:36 -0600 Subject: [PATCH 06/31] feat(captp): return Sync replies via arbitrary comm protocol The takeSyncReply Generator drives the giveSyncReply AsyncGenerator --- packages/captp/lib/captp.js | 283 +++++++++++++++++++++++++------ packages/captp/lib/types.js | 36 ++++ packages/captp/test/synclib.js | 127 ++++++++++++++ packages/captp/test/test-sync.js | 122 +++++-------- packages/captp/test/worker.cjs | 2 + packages/captp/test/worker.js | 23 +++ 6 files changed, 454 insertions(+), 139 deletions(-) create mode 100644 packages/captp/test/synclib.js create mode 100644 packages/captp/test/worker.cjs create mode 100644 packages/captp/test/worker.js diff --git a/packages/captp/lib/captp.js b/packages/captp/lib/captp.js index d8b0f4c1669..3b60cc75c6e 100644 --- a/packages/captp/lib/captp.js +++ b/packages/captp/lib/captp.js @@ -21,6 +21,9 @@ import './types.js'; export { E }; +// Something to make our reply sequences more interesting. +const STARTING_SEQUENCE_NUMBER = 23; + /** * @template T * @typedef {import('@agoric/eventual-send').ERef} ERef @@ -33,8 +36,13 @@ export { E }; * @property {typeof defaultFar} Far * @property {typeof defaultMakeMarshal} makeMarshal * @property {typeof defaultGetInterfaceOf} getInterfaceOf - * @property {number} epoch - * @property {SyncImpl} syncImpl + * @property {number} epoch an integer tag to attach to all messages in order to + * assist in ignoring earlier defunct instance's messages + * @property {TakeSyncReply} takeSyncReply if specified, enable this CapTP end + * to use Sync(target) to block while the recipient resolves and communicates + * the message + * @property {GiveSyncReply} giveSyncReply if specified, enable this CapTP to + * serve objects marked with exportAsSyncable to synchronous clients */ /** * Create a CapTP connection. @@ -51,7 +59,8 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { makeMarshal = defaultMakeMarshal, getInterfaceOf = defaultGetInterfaceOf, epoch = 0, - syncImpl: rawSyncImpl, + takeSyncReply, + giveSyncReply, } = opts; const disconnectReason = id => @@ -120,6 +129,8 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { const questions = new Map(); // chosen by us /** @type {Map} */ const answers = new Map(); // chosen by our peer + /** @type {Map void>} */ + const answerNext = new Map(); /** @type {Map} */ const imports = new Map(); // chosen by our peer @@ -226,6 +237,20 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { }); return harden(pr.p); }, + applyFunction(_o, args) { + if (unplug !== false) { + return quietReject(unplug); + } + const [questionID, pr] = makeQuestion(); + send({ + type: 'CTP_CALL', + epoch, + questionID, + target, + method: serialize(harden([null, args])), + }); + return harden(pr.p); + }, applyMethod(_o, prop, args) { if (unplug !== false) { return quietReject(unplug); @@ -314,7 +339,8 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { // to fulfill // target: Slot id of the target to be invoked. Checks against // answers first; otherwise goes through unserializer - const { questionID, target } = obj; + const { questionID, target, sync } = obj; + const [prop, args] = unserialize(obj.method); let val; if (answers.has(target)) { @@ -328,34 +354,89 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { slots: [target], }); } - // If `args` is supplied, we're applying a method... otherwise this is + + /** @type {(isReject: boolean, value: any) => void} */ + let sendReturn = (isReject, value) => { + send({ + type: 'CTP_RETURN', + epoch, + answerID: questionID, + [isReject ? 'exception' : 'result']: serialize(harden(value)), + }); + }; + if (sync) { + try { + assert( + syncableExported.has(val), + X`Refused Sync(${val}) because target has not been exported as syncable`, + ); + assert.typeof( + giveSyncReply, + 'function', + X`CapTP internal error; cannot make synchronous answer without opts.giveSyncReply`, + ); + } catch (e) { + sendReturn(true, e); + throw e; + } + sendReturn = async (isReject, value) => { + // sendSync may return an iterator. + const serialized = serialize(harden(value)); + const it = giveSyncReply(isReject, serialized); + + // Allow the caller to iterate via `CTP_NEXT` messages. + let nextSeq = STARTING_SEQUENCE_NUMBER; + let pendingP; + const nextInSequence = async (seq, data) => { + await pendingP; + assert.equal( + seq, + nextSeq, + X`Invalid CTP_NEXT; seq ${seq} is not ${nextSeq}`, + ); + nextSeq += 1; + pendingP = it.next(data); + const status = await pendingP; + if (status.done) { + // No more "nexts" are required. + answerNext.delete(questionID); + } + }; + answerNext.set(questionID, nextInSequence); + + // Prime the pump with the first iteration. + nextInSequence(nextSeq, undefined); + }; + } + + // If `args` is supplied, we're applying a method or function... otherwise this is // property access - const hp = args - ? HandledPromise.applyMethod(val, prop, args) - : HandledPromise.get(val, prop); + let hp; + if (!args) { + hp = HandledPromise.get(val, prop); + } else if (prop === null) { + hp = HandledPromise.applyFunction(val, args); + } else { + hp = HandledPromise.applyMethod(val, prop, args); + } + // Answer with our handled promise answers.set(questionID, hp); + // Set up promise resolver for this handled promise to send // message to other vat when fulfilled/broken. return hp - .then(res => - send({ - type: 'CTP_RETURN', - epoch, - answerID: questionID, - result: serialize(harden(res)), - }), - ) - .catch(rej => - send({ - type: 'CTP_RETURN', - epoch, - answerID: questionID, - exception: serialize(harden(rej)), - }), - ) + .then(res => sendReturn(false, res)) + .catch(rej => sendReturn(true, rej)) .catch(rej => quietReject(rej, false)); }, + // Send the next part of a "sync" call's result. + async CTP_NEXT(obj) { + const { questionID, data, seq } = obj; + const nextInSequence = answerNext.get(questionID); + assert(nextInSequence); + return nextInSequence(seq, data); + }, // Answer to one of our questions. async CTP_RETURN(obj) { const { result, exception, answerID } = obj; @@ -454,11 +535,20 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { return obj; }; - /** @type {{Sync?: Sync}} */ - const addSync = {}; + // Put together our return value. + const rets = { + abort, + dispatch, + getBootstrap, + serialize, + unserialize, + exportAsSyncable, + Sync: /** @type {Sync | undefined} */ (undefined), + }; - if (rawSyncImpl) { - const makeSyncImpl = implMethod => (target, ...args) => { + if (takeSyncReply) { + // Create the Sync proxy maker. + const makeSyncImpl = implMethod => (target, ...implArgs) => { assert( Promise.resolve(target) !== target, X`Sync(${target}) target cannot be a promise`, @@ -474,10 +564,90 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { X`Sync(${target}) imported target was not exportAsSyncable`, ); assert( - rawSyncImpl, - X`Sync(${target}) failed; no opts.syncImpl supplied to makeCapTP`, + takeSyncReply, + X`Sync(${target}) failed; no opts.takeSyncReply supplied to makeCapTP`, ); - return rawSyncImpl[implMethod](slot, ...args); + + // Send a "sync" message. + lastQuestionID += 1; + const questionID = lastQuestionID; + + // Encode the "method" parameter of the CTP_CALL. + let method; + switch (implMethod) { + case 'get': { + const [prop] = implArgs; + method = serialize(harden([prop])); + break; + } + case 'applyFunction': { + const [args] = implArgs; + method = serialize(harden([null, args])); + break; + } + case 'applyMethod': { + const [prop, args] = implArgs; + method = serialize(harden([prop, args])); + break; + } + default: { + assert.fail(X`Internal error; unrecognized implMethod ${implMethod}`); + } + } + + // Set up the sync call with its identifying information. + const it = takeSyncReply(implMethod, slot, implArgs); + const nextSync = prep => { + const status = it.next(prep); + assert.typeof( + /** @type {any} */ (status).then, + 'undefined', + X`takeSyncReply must be a synchronous Generator but it.next() returned a Thenable ${status}`, + ); + return status; + }; + + // Prepare the reply, in case there needs to be something initialized + // before the call is ready. + let firstTime = true; + let status = nextSync(firstTime); + let seq = STARTING_SEQUENCE_NUMBER; + while (!status.done) { + if (firstTime) { + // Mark the send as "sync", but handle it asynchronously on the other + // side. + send({ + type: 'CTP_CALL', + epoch, + sync: true, // This is the magic marker. + questionID, + target: slot, + method, + }); + firstTime = false; + } else { + // We weren't done fetching the entire result in the first iteration, + // so thread through the iterator values into the "next" messages to + // retrieve the entire serialized data. + seq += 1; + send({ + type: 'CTP_NEXT', + epoch, + questionID, + seq, + data: status.value, + }); + } + status = nextSync(false); + } + + // We've finished claiming the return value. + const [isReject, serialized] = status.value; + const value = unserialize(serialized); + if (isReject) { + throw value; + } + return value; }; /** @type {SyncImpl} */ @@ -488,18 +658,10 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { }; harden(syncImpl); - addSync.Sync = makeSync(syncImpl); + rets.Sync = makeSync(syncImpl); } - return harden({ - abort, - dispatch, - getBootstrap, - serialize, - unserialize, - exportAsSyncable, - ...addSync, - }); + return harden(rets); } /** @@ -513,7 +675,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { * @param {string} ourId * @param {Partial} [opts] * @returns {{ makeFar(x: T): ERef, makeNear(x: T): ERef, - * exportAsSyncable(x: T): T, Sync: Sync }} + * exportAsSyncable(x: T): T, Sync: Sync }} */ export function makeLoopback(ourId, opts = {}) { const { Far = defaultFar } = opts; @@ -536,34 +698,43 @@ export function makeLoopback(ourId, opts = {}) { index: 0, }); - const makeFarSyncImpl = implMethod => (slot, ...args) => { - // Cross the boundary to pull out the far object. - // eslint-disable-next-line no-use-before-define - const far = farUnserialize({ body: slotBody, slots: [slot] }); - return nearSyncImpl[implMethod](far, ...args); - }; - // Create the tunnel. let farDispatch; const { - dispatch: nearDispatch, Sync, + dispatch: nearDispatch, getBootstrap: getFarBootstrap, } = makeCapTP(`near-${ourId}`, o => farDispatch(o), bootstrap, { - syncImpl: { - applyFunction: makeFarSyncImpl('applyFunction'), - applyMethod: makeFarSyncImpl('applyMethod'), - get: makeFarSyncImpl('get'), + // eslint-disable-next-line require-yield + *takeSyncReply(implMethod, slot, implArgs) { + let value; + let isException = false; + try { + // Cross the boundary to pull out the far object. + // eslint-disable-next-line no-use-before-define + const far = farUnserialize({ body: slotBody, slots: [slot] }); + value = nearSyncImpl[implMethod](far, implArgs[0], implArgs[1]); + } catch (e) { + isException = true; + value = e; + } + // eslint-disable-next-line no-use-before-define + return [isException, farSerialize(value)]; }, }); assert(Sync); const { - dispatch, exportAsSyncable, + dispatch, getBootstrap: getNearBootstrap, unserialize: farUnserialize, - } = makeCapTP(`far-${ourId}`, nearDispatch, bootstrap); + serialize: farSerialize, + } = makeCapTP(`far-${ourId}`, nearDispatch, bootstrap, { + giveSyncReply(_isReject, _serialized) { + throw Error(`makeLoopback giveSyncReply is not expected to be called`); + }, + }); farDispatch = dispatch; const farGetter = E.get(getFarBootstrap()).refGetter; diff --git a/packages/captp/lib/types.js b/packages/captp/lib/types.js index b1795288080..57adebcf7ae 100644 --- a/packages/captp/lib/types.js +++ b/packages/captp/lib/types.js @@ -9,6 +9,42 @@ * lookup */ +/** + * @typedef {[boolean, any]} SyncCompleted The head of the pair is the + * `isRejected` value indicating whether the sync call was an exception, and + * tail of the pair is the serialized value (or error). + */ + +/** + * @callback TakeSyncReply Return a Generator to use in a tight loop until it + * returns a SyncCompleted value indicating the final results of a Sync call. + * + * The rules are that the call request is only sent if the first iteration + * yields instead of returning a result. For each yield, the other end of the + * connection's GiveSyncReply async iterator is called. When the reassembled + * SyncCompleted result is returned, the Sync() call either returns or throws an + * exception. + * + * @param {keyof SyncImpl} implMethod the SyncImpl method that was called + * @param {string} slot the target slot + * @param {Array} implArgs arguments to the SyncImpl method + * @returns {Generator} + */ + +/** + * @callback GiveSyncReply Return an AsyncGenerator which is synchronously + * iterated by TakeSyncReply's generator to signal readiness of parts of the + * reply (there may be only one). These two generators must be written to + * cooperate over a specific CapTP connection, and via any out-of-band + * mechanisms as well. + * + * @param {SyncCompleted[0]} isReject whether the reply to communicate was a + * rejection or a regular return + * @param {SyncCompleted[1]} serialized the marshal-serialized (JSONable) data + * to be communicated to the other side + * @returns {AsyncGenerator} + */ + /** @typedef {import('./ts-types').Sync} Sync */ /** diff --git a/packages/captp/test/synclib.js b/packages/captp/test/synclib.js new file mode 100644 index 00000000000..f7a7c042c67 --- /dev/null +++ b/packages/captp/test/synclib.js @@ -0,0 +1,127 @@ +import { assert, details as X } from '@agoric/assert'; +import { Far } from '@agoric/marshal'; +import { E, makeCapTP } from '../lib/captp'; + +export function createHostBootstrap(exportAsSyncable) { + // Create a remotable that has a syncable return value. + return Far('test sync', { + getSyncable(n) { + const syncable = exportAsSyncable( + Far('getN', { + getN() { + return n; + }, + }), + ); + return syncable; + }, + }); +} + +export async function runSyncTests(t, Sync, bs) { + // Demonstrate async compatibility of syncable. + const pn = E(E(bs).getSyncable(3)).getN(); + t.is(Promise.resolve(pn), pn); + t.is(await pn, 3); + + // Demonstrate Sync cannot be used on a promise. + const ps = E(bs).getSyncable(4); + t.throws(() => Sync(ps).getN(), { + instanceOf: Error, + message: /target cannot be a promise/, + }); + + // Demonstrate Sync used on a remotable. + const s = await ps; + t.is(Sync(s).getN(), 4); + + // Demonstrate Sync fails on an unmarked remotable. + const b = await bs; + t.throws(() => Sync(b).getSyncable(5), { + instanceOf: Error, + message: /imported target was not exportAsSyncable/, + }); +} + +function createGuestBootstrap(Sync, other) { + return Far('tests', { + async runSyncTests() { + const mockT = { + is(a, b) { + assert.equal(a, b, X`${a} !== ${b}`); + }, + throws(thunk, _spec) { + let ret; + try { + ret = thunk(); + } catch (e) { + return; + } + assert.fail(X`Thunk did not throw: ${ret}`); + }, + }; + await runSyncTests(mockT, Sync, other); + return true; + }, + }); +} + +const SEM_WAITING = 0; +const SEM_READY = 2; +const SEM_REJECT = 1; + +function makeBufs(sab) { + const sembuf = new Int32Array(sab, 0, 2 * Int32Array.BYTES_PER_ELEMENT); + const databuf = new Uint8Array(sab, sembuf.byteLength); + return { sembuf, databuf }; +} + +export function makeHost(send, sab) { + const { sembuf, databuf } = makeBufs(sab); + const te = new TextEncoder('utf-8'); + const { dispatch, getBootstrap, exportAsSyncable } = makeCapTP( + 'host', + send, + () => createHostBootstrap(exportAsSyncable), + { + // eslint-disable-next-line require-yield + async *giveSyncReply(isReject, ser) { + // We need a bufferable message. + const data = JSON.stringify(ser); + const { written } = te.encodeInto(data, databuf); + sembuf[1] = written; + sembuf[0] = SEM_READY + (isReject ? SEM_REJECT : 0); + Atomics.notify(sembuf, 0, +Infinity); + }, + }, + ); + + return { dispatch, getBootstrap }; +} + +export function makeGuest(send, sab) { + const { sembuf, databuf } = makeBufs(sab); + const td = new TextDecoder('utf-8'); + const { dispatch, getBootstrap, Sync } = makeCapTP( + 'guest', + send, + () => createGuestBootstrap(Sync, getBootstrap()), + { + *takeSyncReply() { + // Initialize the reply. + sembuf[0] = SEM_WAITING; + yield; + + // Wait for the reply to return. + Atomics.wait(sembuf, 0, SEM_WAITING); + + // eslint-disable-next-line no-bitwise + const isReject = !!(sembuf[0] & 1); + const data = td.decode(databuf.slice(0, sembuf[1])); + const ser = JSON.parse(data); + return [isReject, ser]; + }, + }, + ); + return { dispatch, getBootstrap, Sync }; +} diff --git a/packages/captp/test/test-sync.js b/packages/captp/test/test-sync.js index 4c951171f09..d2569e3266f 100644 --- a/packages/captp/test/test-sync.js +++ b/packages/captp/test/test-sync.js @@ -1,91 +1,47 @@ +/* global __dirname */ import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava'; -import { Far } from '@agoric/marshal'; -import { E, makeCapTP, makeLoopback } from '../lib/captp'; -import { nearSyncImpl } from '../lib/sync'; -function createFarBootstrap(exportAsSyncable) { - // Create a remotable that has a syncable return value. - return Far('test sync', { - getSyncable(n) { - const syncable = exportAsSyncable( - Far('getN', { - getN() { - return n; - }, - }), - ); - return syncable; - }, - }); -} - -async function runSyncTests(t, Sync, bs) { - // Demonstrate async compatibility of syncable. - const pn = E(E(bs).getSyncable(3)).getN(); - t.is(Promise.resolve(pn), pn); - t.is(await pn, 3); - - // Demonstrate Sync cannot be used on a promise. - const ps = E(bs).getSyncable(4); - t.throws(() => Sync(ps).getN(), { - instanceOf: Error, - message: /target cannot be a promise/, - }); - - // Demonstrate Sync used on a remotable. - const s = await ps; - t.is(Sync(s).getN(), 4); +import { Worker } from 'worker_threads'; + +import { E, makeLoopback } from '../lib/captp'; +import { + createHostBootstrap, + makeGuest, + makeHost, + runSyncTests, +} from './synclib'; + +const makeWorkerTests = isHost => async t => { + const sab = new SharedArrayBuffer(2048); + const worker = new Worker(`${__dirname}/worker.cjs`); + worker.addListener('error', err => t.fail(err)); + worker.postMessage({ type: 'TEST_INIT', sab, isGuest: isHost }); + + const initFn = isHost ? makeHost : makeGuest; + const { dispatch, getBootstrap, Sync } = initFn( + obj => worker.postMessage(obj), + sab, + ); - // Demonstrate Sync fails on an unmarked remotable. - const b = await bs; - t.throws(() => Sync(b).getSyncable(5), { - instanceOf: Error, - message: /imported target was not exportAsSyncable/, + worker.addListener('message', obj => { + // console.error('test received', obj); + dispatch(obj); }); -} - -test('try loopback syncable', async t => { - const { makeFar, Sync, exportAsSyncable } = makeLoopback('us'); - const bs = makeFar(createFarBootstrap(exportAsSyncable)); - - await runSyncTests(t, Sync, bs); -}); - -test('try explicit syncable', async t => { - const makeFarSyncImpl = implMethod => (slot, ...args) => { - // Cross the boundary to pull out the far object. - const body = JSON.stringify({ - '@qclass': 'slot', - index: 0, - }); - // eslint-disable-next-line no-use-before-define - const far = farUnserialize({ body, slots: [slot] }); - return nearSyncImpl[implMethod](far, ...args); - }; - - let farDispatch; - const { dispatch: nearDispatch, getBootstrap, Sync } = makeCapTP( - 'near', - o => farDispatch(o), - undefined, - { - syncImpl: { - applyFunction: makeFarSyncImpl('applyFunction'), - applyMethod: makeFarSyncImpl('applyMethod'), - get: makeFarSyncImpl('get'), - has: makeFarSyncImpl('has'), - }, - }, - ); - const { - dispatch, - exportAsSyncable, - unserialize: farUnserialize, - } = makeCapTP('far', nearDispatch, () => - createFarBootstrap(exportAsSyncable), - ); - farDispatch = dispatch; const bs = getBootstrap(); + // console.error('have bs', bs); + if (Sync) { + await runSyncTests(t, Sync, bs); + } else { + t.assert(await E(bs).runSyncTests()); + } +}; + +test('try Node.js worker syncable, main host', makeWorkerTests(true)); +test('try Node.js worker syncable, main guest', makeWorkerTests(false)); + +test('try restricted loopback syncable', async t => { + const { makeFar, Sync, exportAsSyncable } = makeLoopback('us'); + const bs = makeFar(createHostBootstrap(exportAsSyncable)); await runSyncTests(t, Sync, bs); }); diff --git a/packages/captp/test/worker.cjs b/packages/captp/test/worker.cjs new file mode 100644 index 00000000000..89a75f6c49b --- /dev/null +++ b/packages/captp/test/worker.cjs @@ -0,0 +1,2 @@ +/* global require module */ +require('esm')(module)('./worker.js'); diff --git a/packages/captp/test/worker.js b/packages/captp/test/worker.js new file mode 100644 index 00000000000..b854bf31fb5 --- /dev/null +++ b/packages/captp/test/worker.js @@ -0,0 +1,23 @@ +import '@agoric/install-ses'; + +import { parentPort } from 'worker_threads'; +import { makeGuest, makeHost } from './synclib'; + +let dispatch; +parentPort.addListener('message', obj => { + switch (obj.type) { + case 'TEST_INIT': { + const { sab, isGuest } = obj; + const initFn = isGuest ? makeGuest : makeHost; + const ret = initFn(o => parentPort.postMessage(o), sab); + dispatch = ret.dispatch; + break; + } + default: { + if (dispatch) { + dispatch(obj); + } + break; + } + } +}); From d642c414bd22036a72ab6db590d26393efd05568 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Wed, 2 Jun 2021 20:35:32 -0600 Subject: [PATCH 07/31] fix(captp): ensure Sync(x) never returns a thenable --- packages/captp/lib/captp.js | 24 ++++++++++++++++++------ packages/captp/test/synclib.js | 22 +++++++++++++++++++--- packages/captp/test/test-sync.js | 6 +++--- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/packages/captp/lib/captp.js b/packages/captp/lib/captp.js index 3b60cc75c6e..d5a2f064d73 100644 --- a/packages/captp/lib/captp.js +++ b/packages/captp/lib/captp.js @@ -44,6 +44,14 @@ const STARTING_SEQUENCE_NUMBER = 23; * @property {GiveSyncReply} giveSyncReply if specified, enable this CapTP to * serve objects marked with exportAsSyncable to synchronous clients */ + +/** + * @param {any} maybeThenable + * @returns {boolean} + */ +const isThenable = maybeThenable => + maybeThenable && typeof maybeThenable.then === 'function'; + /** * Create a CapTP connection. * @@ -599,10 +607,9 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { const it = takeSyncReply(implMethod, slot, implArgs); const nextSync = prep => { const status = it.next(prep); - assert.typeof( - /** @type {any} */ (status).then, - 'undefined', - X`takeSyncReply must be a synchronous Generator but it.next() returned a Thenable ${status}`, + assert( + !isThenable(status), + X`takeSyncReply must be a fully-synchronous Generator but it.next() returned a Thenable ${status}`, ); return status; }; @@ -616,6 +623,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { if (firstTime) { // Mark the send as "sync", but handle it asynchronously on the other // side. + firstTime = false; send({ type: 'CTP_CALL', epoch, @@ -624,7 +632,6 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { target: slot, method, }); - firstTime = false; } else { // We weren't done fetching the entire result in the first iteration, // so thread through the iterator values into the "next" messages to @@ -638,12 +645,16 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { data: status.value, }); } - status = nextSync(false); + status = nextSync(firstTime); } // We've finished claiming the return value. const [isReject, serialized] = status.value; const value = unserialize(serialized); + assert( + !isThenable(value), + X`Sync() reply must not be a Thenable; have ${value}`, + ); if (isReject) { throw value; } @@ -718,6 +729,7 @@ export function makeLoopback(ourId, opts = {}) { isException = true; value = e; } + harden(value); // eslint-disable-next-line no-use-before-define return [isException, farSerialize(value)]; }, diff --git a/packages/captp/test/synclib.js b/packages/captp/test/synclib.js index f7a7c042c67..7db6533faf7 100644 --- a/packages/captp/test/synclib.js +++ b/packages/captp/test/synclib.js @@ -1,3 +1,6 @@ +// @ts-check +/* global setTimeout */ + import { assert, details as X } from '@agoric/assert'; import { Far } from '@agoric/marshal'; import { E, makeCapTP } from '../lib/captp'; @@ -11,6 +14,9 @@ export function createHostBootstrap(exportAsSyncable) { getN() { return n; }, + getPromise() { + return new Promise(resolve => setTimeout(() => resolve(n), 10)); + }, }), ); return syncable; @@ -18,7 +24,7 @@ export function createHostBootstrap(exportAsSyncable) { }); } -export async function runSyncTests(t, Sync, bs) { +export async function runSyncTests(t, Sync, bs, unwrapsPromises) { // Demonstrate async compatibility of syncable. const pn = E(E(bs).getSyncable(3)).getN(); t.is(Promise.resolve(pn), pn); @@ -35,6 +41,16 @@ export async function runSyncTests(t, Sync, bs) { const s = await ps; t.is(Sync(s).getN(), 4); + // Try Sync unwrapping of a promise. + if (unwrapsPromises) { + t.is(Sync(s).getPromise(), 4); + } else { + t.throws(() => Sync(s).getPromise(), { + instanceOf: Error, + message: /reply must not be a Thenable/, + }); + } + // Demonstrate Sync fails on an unmarked remotable. const b = await bs; t.throws(() => Sync(b).getSyncable(5), { @@ -45,7 +61,7 @@ export async function runSyncTests(t, Sync, bs) { function createGuestBootstrap(Sync, other) { return Far('tests', { - async runSyncTests() { + async runSyncTests(unwrapsPromises) { const mockT = { is(a, b) { assert.equal(a, b, X`${a} !== ${b}`); @@ -60,7 +76,7 @@ function createGuestBootstrap(Sync, other) { assert.fail(X`Thunk did not throw: ${ret}`); }, }; - await runSyncTests(mockT, Sync, other); + await runSyncTests(mockT, Sync, other, unwrapsPromises); return true; }, }); diff --git a/packages/captp/test/test-sync.js b/packages/captp/test/test-sync.js index d2569e3266f..e6f64e6a81e 100644 --- a/packages/captp/test/test-sync.js +++ b/packages/captp/test/test-sync.js @@ -31,9 +31,9 @@ const makeWorkerTests = isHost => async t => { const bs = getBootstrap(); // console.error('have bs', bs); if (Sync) { - await runSyncTests(t, Sync, bs); + await runSyncTests(t, Sync, bs, true); } else { - t.assert(await E(bs).runSyncTests()); + t.assert(await E(bs).runSyncTests(true)); } }; @@ -43,5 +43,5 @@ test('try Node.js worker syncable, main guest', makeWorkerTests(false)); test('try restricted loopback syncable', async t => { const { makeFar, Sync, exportAsSyncable } = makeLoopback('us'); const bs = makeFar(createHostBootstrap(exportAsSyncable)); - await runSyncTests(t, Sync, bs); + await runSyncTests(t, Sync, bs, false); }); From 790735b3ef97c93e3a9354a4fbd5504cad1ffe13 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Thu, 3 Jun 2021 12:15:58 -0600 Subject: [PATCH 08/31] chore: pivot to TrapCaps to reflect guest/host relationship --- packages/captp/lib/captp.js | 167 +++++++++--------- packages/captp/lib/{sync.js => trap.js} | 30 ++-- packages/captp/lib/ts-types.d.ts | 24 +-- packages/captp/lib/types.js | 30 ++-- .../captp/test/{test-sync.js => test-trap.js} | 24 +-- .../captp/test/{synclib.js => traplib.js} | 78 ++++---- packages/captp/test/worker.js | 2 +- 7 files changed, 177 insertions(+), 178 deletions(-) rename packages/captp/lib/{sync.js => trap.js} (72%) rename packages/captp/test/{test-sync.js => test-trap.js} (59%) rename packages/captp/test/{synclib.js => traplib.js} (58%) diff --git a/packages/captp/lib/captp.js b/packages/captp/lib/captp.js index d5a2f064d73..45c1f9beb64 100644 --- a/packages/captp/lib/captp.js +++ b/packages/captp/lib/captp.js @@ -8,21 +8,20 @@ import { Remotable as defaultRemotable, Far as defaultFar, makeMarshal as defaultMakeMarshal, - getInterfaceOf as defaultGetInterfaceOf, QCLASS, } from '@agoric/marshal'; import { E, HandledPromise } from '@agoric/eventual-send'; import { isPromise } from '@agoric/promise-kit'; import { assert, details as X } from '@agoric/assert'; -import { makeSync, nearSyncImpl } from './sync.js'; +import { makeTrap, nearTrapImpl } from './trap.js'; import './types.js'; export { E }; // Something to make our reply sequences more interesting. -const STARTING_SEQUENCE_NUMBER = 23; +const TRAP_FIRST_SEQUENCE_NUMBER = 23; /** * @template T @@ -35,14 +34,14 @@ const STARTING_SEQUENCE_NUMBER = 23; * @property {typeof defaultRemotable} Remotable * @property {typeof defaultFar} Far * @property {typeof defaultMakeMarshal} makeMarshal - * @property {typeof defaultGetInterfaceOf} getInterfaceOf * @property {number} epoch an integer tag to attach to all messages in order to * assist in ignoring earlier defunct instance's messages - * @property {TakeSyncReply} takeSyncReply if specified, enable this CapTP end - * to use Sync(target) to block while the recipient resolves and communicates - * the message - * @property {GiveSyncReply} giveSyncReply if specified, enable this CapTP to - * serve objects marked with exportAsSyncable to synchronous clients + * @property {TakeTrapReply} takeTrapReply if specified, enable this CapTP + * (guest) to use Trap(target) to block while the recipient (host) resolves and + * communicates the message + * @property {GiveTrapReply} giveTrapReply if specified, enable this CapTP + * (host) to serve objects marked with makeTrapHandler to synchronous clients + * (guests) */ /** @@ -65,10 +64,10 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { onReject = err => console.error('CapTP', ourId, 'exception:', err), Remotable = defaultRemotable, makeMarshal = defaultMakeMarshal, - getInterfaceOf = defaultGetInterfaceOf, + Far = defaultFar, epoch = 0, - takeSyncReply, - giveSyncReply, + takeTrapReply, + giveTrapReply, } = opts; const disconnectReason = id => @@ -122,7 +121,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { /** @type {WeakMap} */ const valToSlot = new WeakMap(); // exports looked up by val const slotToVal = new Map(); // reverse - const syncableExported = new WeakSet(); + const exportedTrapHandlers = new WeakSet(); // Used to construct slot names for promises/non-promises. // In this verison of CapTP we use strings for export/import slot names. @@ -138,7 +137,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { /** @type {Map} */ const answers = new Map(); // chosen by our peer /** @type {Map void>} */ - const answerNext = new Map(); + const answerToTrapNextBuffer = new Map(); /** @type {Map} */ const imports = new Map(); // chosen by our peer @@ -176,11 +175,11 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { } else { // Since this isn't a promise, we instead increment the lastExportId and // use that to construct the slot name. Non-promises are prefaced with - // 'o+' for normal objects, or `s+` for syncable. + // 'o+' for normal objects, or `t+` for syncable. lastExportID += 1; const exportID = lastExportID; - if (syncableExported.has(val)) { - slot = `s+${exportID}`; + if (exportedTrapHandlers.has(val)) { + slot = `t+${exportID}`; } else { slot = `o+${exportID}`; } @@ -305,7 +304,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { if (!slotToVal.has(slot)) { // Make a new handled promise for the slot. const pr = makeRemoteKit(slot); - if (slot[0] === 'o' || slot[0] === 's') { + if (slot[0] === 'o' || slot[0] === 't') { // A new remote presence const pres = pr.resPres(); if (iface === undefined) { @@ -347,7 +346,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { // to fulfill // target: Slot id of the target to be invoked. Checks against // answers first; otherwise goes through unserializer - const { questionID, target, sync } = obj; + const { questionID, target, trap } = obj; const [prop, args] = unserialize(obj.method); let val; @@ -372,48 +371,53 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { [isReject ? 'exception' : 'result']: serialize(harden(value)), }); }; - if (sync) { + if (trap) { try { assert( - syncableExported.has(val), - X`Refused Sync(${val}) because target has not been exported as syncable`, + exportedTrapHandlers.has(val), + X`Refused Trap(${val}) because target was not registered with makeTrapHandler`, ); assert.typeof( - giveSyncReply, + giveTrapReply, 'function', - X`CapTP internal error; cannot make synchronous answer without opts.giveSyncReply`, + X`CapTP internal error; cannot answer Trap(x) without opts.giveTrapReply`, ); } catch (e) { sendReturn(true, e); throw e; } sendReturn = async (isReject, value) => { - // sendSync may return an iterator. + // sendTrap may return an iterator. const serialized = serialize(harden(value)); - const it = giveSyncReply(isReject, serialized); + const it = giveTrapReply(isReject, serialized); - // Allow the caller to iterate via `CTP_NEXT` messages. - let nextSeq = STARTING_SEQUENCE_NUMBER; + // Allow the caller to iterate via `CTP_TRAP_NEXT_BUFFER` messages. + let nextSeq = TRAP_FIRST_SEQUENCE_NUMBER; let pendingP; + /** + * @param {number} seq + * @param {any} data + */ const nextInSequence = async (seq, data) => { + // Prevent parallel requests for a given sequence number. await pendingP; assert.equal( seq, nextSeq, - X`Invalid CTP_NEXT; seq ${seq} is not ${nextSeq}`, + X`Invalid CTP_TRAP_NEXT_BUFFER; seq ${seq} is not ${nextSeq}`, ); nextSeq += 1; pendingP = it.next(data); const status = await pendingP; if (status.done) { // No more "nexts" are required. - answerNext.delete(questionID); + answerToTrapNextBuffer.delete(questionID); } }; - answerNext.set(questionID, nextInSequence); + answerToTrapNextBuffer.set(questionID, nextInSequence); // Prime the pump with the first iteration. - nextInSequence(nextSeq, undefined); + await nextInSequence(nextSeq, undefined); }; } @@ -438,10 +442,10 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { .catch(rej => sendReturn(true, rej)) .catch(rej => quietReject(rej, false)); }, - // Send the next part of a "sync" call's result. - async CTP_NEXT(obj) { + // Send the next part of a "trap" call's result. + async CTP_TRAP_NEXT_BUFFER(obj) { const { questionID, data, seq } = obj; - const nextInSequence = answerNext.get(questionID); + const nextInSequence = answerToTrapNextBuffer.get(questionID); assert(nextInSequence); return nextInSequence(seq, data); }, @@ -534,13 +538,10 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { dispatch({ type: 'CTP_DISCONNECT', epoch, reason }); }; - const exportAsSyncable = obj => { - assert( - getInterfaceOf(obj) !== undefined, - X`exportAsSyncable(${obj}) argument must be Remotable`, - ); - syncableExported.add(obj); - return obj; + const makeTrapHandler = (name, obj) => { + const far = Far(name, obj); + exportedTrapHandlers.add(far); + return far; }; // Put together our return value. @@ -550,33 +551,33 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { getBootstrap, serialize, unserialize, - exportAsSyncable, - Sync: /** @type {Sync | undefined} */ (undefined), + makeTrapHandler, + Trap: /** @type {Trap | undefined} */ (undefined), }; - if (takeSyncReply) { - // Create the Sync proxy maker. - const makeSyncImpl = implMethod => (target, ...implArgs) => { + if (takeTrapReply) { + // Create the Trap proxy maker. + const makeTrapImpl = implMethod => (target, ...implArgs) => { assert( Promise.resolve(target) !== target, - X`Sync(${target}) target cannot be a promise`, + X`Trap(${target}) target cannot be a promise`, ); const slot = valToSlot.get(target); assert( slot && slot[1] === '-', - X`Sync(${target}) target was not imported`, + X`Trap(${target}) target was not imported`, ); assert( - slot[0] === 's', - X`Sync(${target}) imported target was not exportAsSyncable`, + slot[0] === 't', + X`Trap(${target}) imported target was not created with makeTrapHandler`, ); assert( - takeSyncReply, - X`Sync(${target}) failed; no opts.takeSyncReply supplied to makeCapTP`, + takeTrapReply, + X`Trap(${target}) failed; no opts.takeTrapReply supplied to makeCapTP`, ); - // Send a "sync" message. + // Send a "trap" message. lastQuestionID += 1; const questionID = lastQuestionID; @@ -603,13 +604,13 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { } } - // Set up the sync call with its identifying information. - const it = takeSyncReply(implMethod, slot, implArgs); - const nextSync = prep => { + // Set up the trap call with its identifying information. + const it = takeTrapReply(implMethod, slot, implArgs); + const nextTrap = prep => { const status = it.next(prep); assert( !isThenable(status), - X`takeSyncReply must be a fully-synchronous Generator but it.next() returned a Thenable ${status}`, + X`takeTrapReply must be a fully-synchronous Generator but it.next() returned a Thenable ${status}`, ); return status; }; @@ -617,17 +618,17 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { // Prepare the reply, in case there needs to be something initialized // before the call is ready. let firstTime = true; - let status = nextSync(firstTime); - let seq = STARTING_SEQUENCE_NUMBER; + let status = nextTrap(firstTime); + let seq = TRAP_FIRST_SEQUENCE_NUMBER; while (!status.done) { if (firstTime) { - // Mark the send as "sync", but handle it asynchronously on the other - // side. + // Mark the send as a "trap", but handle it asynchronously on the + // other side. firstTime = false; send({ type: 'CTP_CALL', epoch, - sync: true, // This is the magic marker. + trap: true, // This is the magic marker. questionID, target: slot, method, @@ -638,14 +639,14 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { // retrieve the entire serialized data. seq += 1; send({ - type: 'CTP_NEXT', + type: 'CTP_TRAP_NEXT_BUFFER', epoch, questionID, seq, data: status.value, }); } - status = nextSync(firstTime); + status = nextTrap(firstTime); } // We've finished claiming the return value. @@ -653,7 +654,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { const value = unserialize(serialized); assert( !isThenable(value), - X`Sync() reply must not be a Thenable; have ${value}`, + X`Trap() reply cannot be a Thenable; have ${value}`, ); if (isReject) { throw value; @@ -661,15 +662,15 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { return value; }; - /** @type {SyncImpl} */ - const syncImpl = { - applyFunction: makeSyncImpl('applyFunction'), - applyMethod: makeSyncImpl('applyMethod'), - get: makeSyncImpl('get'), + /** @type {TrapImpl} */ + const trapImpl = { + applyFunction: makeTrapImpl('applyFunction'), + applyMethod: makeTrapImpl('applyMethod'), + get: makeTrapImpl('get'), }; - harden(syncImpl); + harden(trapImpl); - rets.Sync = makeSync(syncImpl); + rets.Trap = makeTrap(trapImpl); } return harden(rets); @@ -686,7 +687,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { * @param {string} ourId * @param {Partial} [opts] * @returns {{ makeFar(x: T): ERef, makeNear(x: T): ERef, - * exportAsSyncable(x: T): T, Sync: Sync }} + * makeTrapHandler(x: T): T, Trap: Trap }} */ export function makeLoopback(ourId, opts = {}) { const { Far = defaultFar } = opts; @@ -712,19 +713,19 @@ export function makeLoopback(ourId, opts = {}) { // Create the tunnel. let farDispatch; const { - Sync, + Trap, dispatch: nearDispatch, getBootstrap: getFarBootstrap, } = makeCapTP(`near-${ourId}`, o => farDispatch(o), bootstrap, { // eslint-disable-next-line require-yield - *takeSyncReply(implMethod, slot, implArgs) { + *takeTrapReply(implMethod, slot, implArgs) { let value; let isException = false; try { // Cross the boundary to pull out the far object. // eslint-disable-next-line no-use-before-define const far = farUnserialize({ body: slotBody, slots: [slot] }); - value = nearSyncImpl[implMethod](far, implArgs[0], implArgs[1]); + value = nearTrapImpl[implMethod](far, implArgs[0], implArgs[1]); } catch (e) { isException = true; value = e; @@ -734,17 +735,17 @@ export function makeLoopback(ourId, opts = {}) { return [isException, farSerialize(value)]; }, }); - assert(Sync); + assert(Trap); const { - exportAsSyncable, + makeTrapHandler, dispatch, getBootstrap: getNearBootstrap, unserialize: farUnserialize, serialize: farSerialize, } = makeCapTP(`far-${ourId}`, nearDispatch, bootstrap, { - giveSyncReply(_isReject, _serialized) { - throw Error(`makeLoopback giveSyncReply is not expected to be called`); + giveTrapReply(_isReject, _serialized) { + throw Error(`makeLoopback giveTrapReply is not expected to be called`); }, }); farDispatch = dispatch; @@ -769,7 +770,7 @@ export function makeLoopback(ourId, opts = {}) { return { makeFar: makeRefMaker(farGetter), makeNear: makeRefMaker(nearGetter), - exportAsSyncable, - Sync, + makeTrapHandler, + Trap, }; } diff --git a/packages/captp/lib/sync.js b/packages/captp/lib/trap.js similarity index 72% rename from packages/captp/lib/sync.js rename to packages/captp/lib/trap.js index 9a25ce3b246..bb5120dcd2a 100644 --- a/packages/captp/lib/sync.js +++ b/packages/captp/lib/trap.js @@ -4,11 +4,11 @@ import './types'; /** - * Default implementation of Sync for near objects. + * Default implementation of Trap for near objects. * - * @type {SyncImpl} + * @type {TrapImpl} */ -export const nearSyncImpl = harden({ +export const nearTrapImpl = harden({ applyFunction(target, args) { return target(...args); }, @@ -36,13 +36,13 @@ const readOnlyProxyHandler = { }; /** - * A Proxy handler for Sync(x) + * A Proxy handler for Trap(x) * - * @param {*} x Any value passed to Sync(x) - * @param {SyncImpl} syncImpl + * @param {*} x Any value passed to Trap(x) + * @param {TrapImpl} syncImpl * @returns {ProxyHandler} */ -function SyncProxyHandler(x, syncImpl) { +function TrapProxyHandler(x, syncImpl) { return harden({ ...readOnlyProxyHandler, get(_target, p, _receiver) { @@ -59,16 +59,16 @@ function SyncProxyHandler(x, syncImpl) { } /** - * @param {SyncImpl} syncImpl - * @returns {Sync} + * @param {TrapImpl} syncImpl + * @returns {Trap} */ -export function makeSync(syncImpl) { - function Sync(x) { - const handler = SyncProxyHandler(x, syncImpl); +export function makeTrap(syncImpl) { + function Trap(x) { + const handler = TrapProxyHandler(x, syncImpl); return harden(new Proxy(() => {}, handler)); } - const makeSyncGetterProxy = x => { + const makeTrapGetterProxy = x => { const handler = harden({ ...readOnlyProxyHandler, has(_target, _prop) { @@ -81,7 +81,7 @@ export function makeSync(syncImpl) { }); return new Proxy(Object.create(null), handler); }; - Sync.get = makeSyncGetterProxy; + Trap.get = makeTrapGetterProxy; - return harden(Sync); + return harden(Trap); } diff --git a/packages/captp/lib/ts-types.d.ts b/packages/captp/lib/ts-types.d.ts index b0087e0643e..e0bac854041 100644 --- a/packages/captp/lib/ts-types.d.ts +++ b/packages/captp/lib/ts-types.d.ts @@ -4,7 +4,7 @@ type ERef = PromiseLike | T; type Unpromise = T extends ERef ? U : T; -export type Syncable = T extends (...args: infer P) => infer R +export type TrapHandler = T extends (...args: infer P) => infer R ? (...args: P) => Unpromise : T extends Record ? { @@ -12,39 +12,39 @@ export type Syncable = T extends (...args: infer P) => infer R } : T; -/* Types for Sync proxy calls. */ -type SyncSingleMethod = { +/* Types for Trap proxy calls. */ +type TrapSingleMethod = { readonly [P in keyof T]: ( ...args: Parameters ) => Unpromise>; } -type SyncSingleCall = T extends Function ? +type TrapSingleCall = T extends Function ? ((...args: Parameters) => Unpromise>) & ESingleMethod> : ESingleMethod>; -type SyncSingleGet = { +type TrapSingleGet = { readonly [P in keyof T]: Unpromise; } -export interface Sync { +export interface Trap { /** - * Sync(x) returns a proxy on which you can call arbitrary methods. Each of + * Trap(x) returns a proxy on which you can call arbitrary methods. Each of * these method calls will unwrap a promise result. The method will be * invoked on a remote 'x', and be synchronous from the perspective of this * caller. * * @param {*} x target for method/function call - * @returns {SyncSingleCall} method/function call proxy + * @returns {TrapSingleCall} method/function call proxy */ - (x: T): SyncSingleCall>; + (x: T): TrapSingleCall>; /** - * Sync.get(x) returns a proxy on which you can get arbitrary properties. + * Trap.get(x) returns a proxy on which you can get arbitrary properties. * Each of these properties unwraps a promise result. The value will be the * property fetched from a remote 'x', and be synchronous from the perspective * of this caller. * * @param {*} x target for property get - * @returns {SyncSingleGet} property get proxy + * @returns {TrapSingleGet} property get proxy */ - readonly get(x: T): SyncSingleGet>; + readonly get(x: T): TrapSingleGet>; } diff --git a/packages/captp/lib/types.js b/packages/captp/lib/types.js index 57adebcf7ae..e1df83fbbc8 100644 --- a/packages/captp/lib/types.js +++ b/packages/captp/lib/types.js @@ -1,5 +1,5 @@ /** - * @typedef {Object} SyncImpl + * @typedef {Object} TrapImpl * @property {(target: any, args: Array) => any} applyFunction function * application * @property {(target: any, method: string | symbol | number, args: Array) @@ -10,44 +10,44 @@ */ /** - * @typedef {[boolean, any]} SyncCompleted The head of the pair is the + * @typedef {[boolean, any]} TrapCompleted The head of the pair is the * `isRejected` value indicating whether the sync call was an exception, and * tail of the pair is the serialized value (or error). */ /** - * @callback TakeSyncReply Return a Generator to use in a tight loop until it - * returns a SyncCompleted value indicating the final results of a Sync call. + * @callback TakeTrapReply Return a Generator to use in a tight loop until it + * returns a TrapCompleted value indicating the final results of a Trap call. * * The rules are that the call request is only sent if the first iteration * yields instead of returning a result. For each yield, the other end of the - * connection's GiveSyncReply async iterator is called. When the reassembled - * SyncCompleted result is returned, the Sync() call either returns or throws an + * connection's GiveTrapReply async iterator is called. When the reassembled + * TrapCompleted result is returned, the Trap() call either returns or throws an * exception. * - * @param {keyof SyncImpl} implMethod the SyncImpl method that was called + * @param {keyof TrapImpl} implMethod the TrapImpl method that was called * @param {string} slot the target slot - * @param {Array} implArgs arguments to the SyncImpl method - * @returns {Generator} + * @param {Array} implArgs arguments to the TrapImpl method + * @returns {Generator} */ /** - * @callback GiveSyncReply Return an AsyncGenerator which is synchronously - * iterated by TakeSyncReply's generator to signal readiness of parts of the + * @callback GiveTrapReply Return an AsyncGenerator which is synchronously + * iterated by TakeTrapReply's generator to signal readiness of parts of the * reply (there may be only one). These two generators must be written to * cooperate over a specific CapTP connection, and via any out-of-band * mechanisms as well. * - * @param {SyncCompleted[0]} isReject whether the reply to communicate was a + * @param {TrapCompleted[0]} isReject whether the reply to communicate was a * rejection or a regular return - * @param {SyncCompleted[1]} serialized the marshal-serialized (JSONable) data + * @param {TrapCompleted[1]} serialized the marshal-serialized (JSONable) data * to be communicated to the other side * @returns {AsyncGenerator} */ -/** @typedef {import('./ts-types').Sync} Sync */ +/** @typedef {import('./ts-types').Trap} Trap */ /** * @template T - * @typedef {import('./ts-types').Syncable} Syncable + * @typedef {import('./ts-types').TrapHandler} TrapHandler */ diff --git a/packages/captp/test/test-sync.js b/packages/captp/test/test-trap.js similarity index 59% rename from packages/captp/test/test-sync.js rename to packages/captp/test/test-trap.js index e6f64e6a81e..defec8393ca 100644 --- a/packages/captp/test/test-sync.js +++ b/packages/captp/test/test-trap.js @@ -8,8 +8,8 @@ import { createHostBootstrap, makeGuest, makeHost, - runSyncTests, -} from './synclib'; + runTrapTests, +} from './traplib'; const makeWorkerTests = isHost => async t => { const sab = new SharedArrayBuffer(2048); @@ -18,7 +18,7 @@ const makeWorkerTests = isHost => async t => { worker.postMessage({ type: 'TEST_INIT', sab, isGuest: isHost }); const initFn = isHost ? makeHost : makeGuest; - const { dispatch, getBootstrap, Sync } = initFn( + const { dispatch, getBootstrap, Trap } = initFn( obj => worker.postMessage(obj), sab, ); @@ -30,18 +30,18 @@ const makeWorkerTests = isHost => async t => { const bs = getBootstrap(); // console.error('have bs', bs); - if (Sync) { - await runSyncTests(t, Sync, bs, true); + if (Trap) { + await runTrapTests(t, Trap, bs, true); } else { - t.assert(await E(bs).runSyncTests(true)); + t.assert(await E(bs).runTrapTests(true)); } }; -test('try Node.js worker syncable, main host', makeWorkerTests(true)); -test('try Node.js worker syncable, main guest', makeWorkerTests(false)); +test('try Node.js worker trap, main host', makeWorkerTests(true)); +test('try Node.js worker trap, main guest', makeWorkerTests(false)); -test('try restricted loopback syncable', async t => { - const { makeFar, Sync, exportAsSyncable } = makeLoopback('us'); - const bs = makeFar(createHostBootstrap(exportAsSyncable)); - await runSyncTests(t, Sync, bs, false); +test('try restricted loopback trap', async t => { + const { makeFar, Trap, makeTrapHandler } = makeLoopback('us'); + const bs = makeFar(createHostBootstrap(makeTrapHandler)); + await runTrapTests(t, Trap, bs, false); }); diff --git a/packages/captp/test/synclib.js b/packages/captp/test/traplib.js similarity index 58% rename from packages/captp/test/synclib.js rename to packages/captp/test/traplib.js index 7db6533faf7..48b7c2d14c8 100644 --- a/packages/captp/test/synclib.js +++ b/packages/captp/test/traplib.js @@ -5,63 +5,61 @@ import { assert, details as X } from '@agoric/assert'; import { Far } from '@agoric/marshal'; import { E, makeCapTP } from '../lib/captp'; -export function createHostBootstrap(exportAsSyncable) { +export function createHostBootstrap(makeTrapHandler) { // Create a remotable that has a syncable return value. - return Far('test sync', { - getSyncable(n) { - const syncable = exportAsSyncable( - Far('getN', { - getN() { - return n; - }, - getPromise() { - return new Promise(resolve => setTimeout(() => resolve(n), 10)); - }, - }), - ); - return syncable; + return Far('test traps', { + getTraps(n) { + return makeTrapHandler('getNTraps', { + getN() { + return n; + }, + getPromise() { + return new Promise(resolve => setTimeout(() => resolve(n), 10)); + }, + }); }, }); } -export async function runSyncTests(t, Sync, bs, unwrapsPromises) { - // Demonstrate async compatibility of syncable. - const pn = E(E(bs).getSyncable(3)).getN(); +export async function runTrapTests(t, Trap, bs, unwrapsPromises) { + // Demonstrate async compatibility of traps. + const pn = E(E(bs).getTraps(3)).getN(); t.is(Promise.resolve(pn), pn); t.is(await pn, 3); - // Demonstrate Sync cannot be used on a promise. - const ps = E(bs).getSyncable(4); - t.throws(() => Sync(ps).getN(), { + // Demonstrate Trap cannot be used on a promise. + const ps = E(bs).getTraps(4); + t.throws(() => Trap(ps).getN(), { instanceOf: Error, message: /target cannot be a promise/, }); - // Demonstrate Sync used on a remotable. + // Demonstrate Trap used on a remotable. const s = await ps; - t.is(Sync(s).getN(), 4); + t.is(Trap(s).getN(), 4); - // Try Sync unwrapping of a promise. + // Try Trap unwrapping of a promise. if (unwrapsPromises) { - t.is(Sync(s).getPromise(), 4); + t.is(Trap(s).getPromise(), 4); } else { - t.throws(() => Sync(s).getPromise(), { + // If it's not supported, verify that an exception is thrown. + t.throws(() => Trap(s).getPromise(), { instanceOf: Error, - message: /reply must not be a Thenable/, + message: /reply cannot be a Thenable/, }); } - // Demonstrate Sync fails on an unmarked remotable. + // Demonstrate Trap fails on an unmarked remotable. const b = await bs; - t.throws(() => Sync(b).getSyncable(5), { + t.throws(() => Trap(b).getTraps(5), { instanceOf: Error, - message: /imported target was not exportAsSyncable/, + message: /imported target was not created with makeTrapHandler/, }); } -function createGuestBootstrap(Sync, other) { +function createGuestBootstrap(Trap, other) { return Far('tests', { - async runSyncTests(unwrapsPromises) { + async runTrapTests(unwrapsPromises) { const mockT = { is(a, b) { assert.equal(a, b, X`${a} !== ${b}`); @@ -76,7 +74,7 @@ function createGuestBootstrap(Sync, other) { assert.fail(X`Thunk did not throw: ${ret}`); }, }; - await runSyncTests(mockT, Sync, other, unwrapsPromises); + await runTrapTests(mockT, Trap, other, unwrapsPromises); return true; }, }); @@ -94,14 +92,14 @@ function makeBufs(sab) { export function makeHost(send, sab) { const { sembuf, databuf } = makeBufs(sab); - const te = new TextEncoder('utf-8'); - const { dispatch, getBootstrap, exportAsSyncable } = makeCapTP( + const te = new TextEncoder(); + const { dispatch, getBootstrap, makeTrapHandler } = makeCapTP( 'host', send, - () => createHostBootstrap(exportAsSyncable), + () => createHostBootstrap(makeTrapHandler), { // eslint-disable-next-line require-yield - async *giveSyncReply(isReject, ser) { + async *giveTrapReply(isReject, ser) { // We need a bufferable message. const data = JSON.stringify(ser); const { written } = te.encodeInto(data, databuf); @@ -118,12 +116,12 @@ export function makeHost(send, sab) { export function makeGuest(send, sab) { const { sembuf, databuf } = makeBufs(sab); const td = new TextDecoder('utf-8'); - const { dispatch, getBootstrap, Sync } = makeCapTP( + const { dispatch, getBootstrap, Trap } = makeCapTP( 'guest', send, - () => createGuestBootstrap(Sync, getBootstrap()), + () => createGuestBootstrap(Trap, getBootstrap()), { - *takeSyncReply() { + *takeTrapReply() { // Initialize the reply. sembuf[0] = SEM_WAITING; yield; @@ -139,5 +137,5 @@ export function makeGuest(send, sab) { }, }, ); - return { dispatch, getBootstrap, Sync }; + return { dispatch, getBootstrap, Trap }; } diff --git a/packages/captp/test/worker.js b/packages/captp/test/worker.js index b854bf31fb5..623c43ab359 100644 --- a/packages/captp/test/worker.js +++ b/packages/captp/test/worker.js @@ -1,7 +1,7 @@ import '@agoric/install-ses'; import { parentPort } from 'worker_threads'; -import { makeGuest, makeHost } from './synclib'; +import { makeGuest, makeHost } from './traplib'; let dispatch; parentPort.addListener('message', obj => { From 1eb035c72d9b7f75824775f21fd3ae87b659b618 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Wed, 16 Jun 2021 14:59:11 -0600 Subject: [PATCH 09/31] refactor: separate CapTP communications from trap iterator driver --- packages/captp/lib/captp.js | 120 +++++----------------- packages/captp/lib/trap-driver.js | 161 ++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 96 deletions(-) create mode 100644 packages/captp/lib/trap-driver.js diff --git a/packages/captp/lib/captp.js b/packages/captp/lib/captp.js index 45c1f9beb64..435d4aea0e1 100644 --- a/packages/captp/lib/captp.js +++ b/packages/captp/lib/captp.js @@ -15,14 +15,12 @@ import { isPromise } from '@agoric/promise-kit'; import { assert, details as X } from '@agoric/assert'; import { makeTrap, nearTrapImpl } from './trap.js'; +import { makeTrapGuest, makeTrapHost } from './trap-driver.js'; import './types.js'; export { E }; -// Something to make our reply sequences more interesting. -const TRAP_FIRST_SEQUENCE_NUMBER = 23; - /** * @template T * @typedef {import('@agoric/eventual-send').ERef} ERef @@ -44,13 +42,6 @@ const TRAP_FIRST_SEQUENCE_NUMBER = 23; * (guests) */ -/** - * @param {any} maybeThenable - * @returns {boolean} - */ -const isThenable = maybeThenable => - maybeThenable && typeof maybeThenable.then === 'function'; - /** * Create a CapTP connection. * @@ -118,6 +109,9 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { }, ); + const trapHost = giveTrapReply && makeTrapHost(); + const trapGuest = takeTrapReply && makeTrapGuest(unserialize); + /** @type {WeakMap} */ const valToSlot = new WeakMap(); // exports looked up by val const slotToVal = new Map(); // reverse @@ -136,8 +130,6 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { const questions = new Map(); // chosen by us /** @type {Map} */ const answers = new Map(); // chosen by our peer - /** @type {Map void>} */ - const answerToTrapNextBuffer = new Map(); /** @type {Map} */ const imports = new Map(); // chosen by our peer @@ -380,45 +372,18 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { assert.typeof( giveTrapReply, 'function', - X`CapTP internal error; cannot answer Trap(x) without opts.giveTrapReply`, + X`CapTP cannot answer Trap(x) without opts.giveTrapReply`, ); + assert(trapHost, X`CatTP internal error; trapHost is not defined`); + sendReturn = async (isReject, value) => { + const serialized = serialize(harden(value)); + const it = giveTrapReply(isReject, serialized); + await trapHost.sendTrapReply(questionID, it); + }; } catch (e) { sendReturn(true, e); throw e; } - sendReturn = async (isReject, value) => { - // sendTrap may return an iterator. - const serialized = serialize(harden(value)); - const it = giveTrapReply(isReject, serialized); - - // Allow the caller to iterate via `CTP_TRAP_NEXT_BUFFER` messages. - let nextSeq = TRAP_FIRST_SEQUENCE_NUMBER; - let pendingP; - /** - * @param {number} seq - * @param {any} data - */ - const nextInSequence = async (seq, data) => { - // Prevent parallel requests for a given sequence number. - await pendingP; - assert.equal( - seq, - nextSeq, - X`Invalid CTP_TRAP_NEXT_BUFFER; seq ${seq} is not ${nextSeq}`, - ); - nextSeq += 1; - pendingP = it.next(data); - const status = await pendingP; - if (status.done) { - // No more "nexts" are required. - answerToTrapNextBuffer.delete(questionID); - } - }; - answerToTrapNextBuffer.set(questionID, nextInSequence); - - // Prime the pump with the first iteration. - await nextInSequence(nextSeq, undefined); - }; } // If `args` is supplied, we're applying a method or function... otherwise this is @@ -442,13 +407,9 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { .catch(rej => sendReturn(true, rej)) .catch(rej => quietReject(rej, false)); }, - // Send the next part of a "trap" call's result. - async CTP_TRAP_NEXT_BUFFER(obj) { - const { questionID, data, seq } = obj; - const nextInSequence = answerToTrapNextBuffer.get(questionID); - assert(nextInSequence); - return nextInSequence(seq, data); - }, + // Have the host serve the next buffer request. + CTP_TRAP_NEXT_BUFFER: + trapHost && (obj => trapHost.trapNextBuffer(obj.questionID, obj.trapBuf)), // Answer to one of our questions. async CTP_RETURN(obj) { const { result, exception, answerID } = obj; @@ -604,27 +565,13 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { } } - // Set up the trap call with its identifying information. - const it = takeTrapReply(implMethod, slot, implArgs); - const nextTrap = prep => { - const status = it.next(prep); - assert( - !isThenable(status), - X`takeTrapReply must be a fully-synchronous Generator but it.next() returned a Thenable ${status}`, - ); - return status; - }; + assert(trapGuest, X`Trap(${target}) internal error; no trapGuest`); - // Prepare the reply, in case there needs to be something initialized - // before the call is ready. - let firstTime = true; - let status = nextTrap(firstTime); - let seq = TRAP_FIRST_SEQUENCE_NUMBER; - while (!status.done) { - if (firstTime) { - // Mark the send as a "trap", but handle it asynchronously on the - // other side. - firstTime = false; + // Set up the trap call with its identifying information. + const takeIt = takeTrapReply(implMethod, slot, implArgs); + return trapGuest.doTrap( + takeIt, + () => send({ type: 'CTP_CALL', epoch, @@ -632,34 +579,15 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { questionID, target: slot, method, - }); - } else { - // We weren't done fetching the entire result in the first iteration, - // so thread through the iterator values into the "next" messages to - // retrieve the entire serialized data. - seq += 1; + }), + trapBuf => send({ type: 'CTP_TRAP_NEXT_BUFFER', epoch, questionID, - seq, - data: status.value, - }); - } - status = nextTrap(firstTime); - } - - // We've finished claiming the return value. - const [isReject, serialized] = status.value; - const value = unserialize(serialized); - assert( - !isThenable(value), - X`Trap() reply cannot be a Thenable; have ${value}`, + trapBuf, + }), ); - if (isReject) { - throw value; - } - return value; }; /** @type {TrapImpl} */ diff --git a/packages/captp/lib/trap-driver.js b/packages/captp/lib/trap-driver.js new file mode 100644 index 00000000000..5b536a478e9 --- /dev/null +++ b/packages/captp/lib/trap-driver.js @@ -0,0 +1,161 @@ +// @ts-check + +/** + * This file drives the trap takeReply and giveReply iterators. + * + * On the Guest side, the takeIt is given next(true). If it returns, that's the + * end of the cycle. + * + * If it yields, then the initTrap function is called. For all subsequent + * iterations, it is given next(false). yields, the nextBuffer function is + * called with buffer parameters. + * + * On the Host side, sendTrapReply should be called when the answer from the + * initTrap step has settled. Calling the trapNextBuffer function with the + * trapID and supplied buffer parameters will rev the giveIt to transfer buffers + * to the takeIt. + */ + +import { assert, details as X } from '@agoric/assert'; + +import './types'; + +// Something to make our reply sequences more interesting. +const TRAP_FIRST_SEQUENCE_NUMBER = 23; + +/** + * @param {any} maybeThenable + * @returns {boolean} + */ +const isThenable = maybeThenable => + maybeThenable && typeof maybeThenable.then === 'function'; + +/** + * Create the host side of the trap interface. + */ +export const makeTrapHost = () => { + /** @type {Map void>} */ + const trapIDToNextBuffer = new Map(); + + return { + /** + * @param {any} trapID + * @param {ReturnType} giveIt + */ + sendTrapReply: async (trapID, giveIt) => { + assert( + !trapIDToNextBuffer.has(trapID), + X`Trap ID ${trapID} is already in progress`, + ); + + // Allow the caller to iterate via `trapNextBuffer` messages. + let nextSeq = TRAP_FIRST_SEQUENCE_NUMBER; + let pendingP; + /** + * @param {number} seq + * @param {any} data + */ + const nextInSequence = async (seq, data) => { + // Prevent parallel requests for a given sequence number. + await pendingP; + assert.equal( + seq, + nextSeq, + X`Invalid trapNextBuffer; seq ${seq} is not ${nextSeq}`, + ); + nextSeq += 1; + pendingP = giveIt.next(data); + const status = await pendingP; + if (status.done) { + // No more "nexts" are required, so clean up. + trapIDToNextBuffer.delete(trapID); + } + }; + trapIDToNextBuffer.set(trapID, nextInSequence); + + // Prime the pump with the first iteration. + await nextInSequence(nextSeq, undefined); + }, + // Send the next part of a "trap" call's result. + trapNextBuffer: async (trapID, trapParams) => { + const { data, seq } = trapParams; + const nextInSequence = trapIDToNextBuffer.get(trapID); + assert(nextInSequence); + return nextInSequence(seq, data); + }, + }; +}; + +/** + * Create the guest side of the trap interface. + * + * @param {(ser: any) => any} unserialize + */ +export const makeTrapGuest = unserialize => { + return { + /** + * @param {ReturnType} takeIt + * @param {() => void} [initTrap] + * @param {(trapParams: any) => void} [nextBuffer] + */ + doTrap: (takeIt, initTrap = undefined, nextBuffer = undefined) => { + /** + * @param {boolean} firstTime + */ + const nextTrap = firstTime => { + const status = takeIt.next(firstTime); + assert( + !isThenable(status), + X`takeTrapReply must be a fully-synchronous Generator but it.next() returned a Thenable ${status}`, + ); + return status; + }; + + // Prepare the reply, in case there needs to be something initialized + // before the call is ready. + let firstTime = true; + let status = nextTrap(firstTime); + let seq = TRAP_FIRST_SEQUENCE_NUMBER; + while (!status.done) { + if (firstTime) { + // Mark the send as a "trap", but handle it asynchronously on the + // other side. + assert.typeof( + initTrap, + 'function', + X`Trap() initTrap function must be defined`, + ); + firstTime = false; + initTrap(); + } else { + // We weren't done fetching the entire result in the first iteration, + // so thread through the iterator values into the "next" messages to + // retrieve the entire serialized data. + assert.typeof( + nextBuffer, + 'function', + X`Trap() nextBuffer function must be defined`, + ); + seq += 1; + nextBuffer({ + seq, + data: status.value, + }); + } + status = nextTrap(firstTime); + } + + // We've finished claiming the return value. + const [isReject, serialized] = status.value; + const value = unserialize(serialized); + assert( + !isThenable(value), + X`Trap() reply cannot be a Thenable; have ${value}`, + ); + if (isReject) { + throw value; + } + return value; + }, + }; +}; From 4135491ac92fd108ca994ee1658a641dbc015d15 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Wed, 16 Jun 2021 15:28:27 -0600 Subject: [PATCH 10/31] docs(captp): fix up description of trap-driver.js --- packages/captp/lib/trap-driver.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/captp/lib/trap-driver.js b/packages/captp/lib/trap-driver.js index 5b536a478e9..bb87c2c1d13 100644 --- a/packages/captp/lib/trap-driver.js +++ b/packages/captp/lib/trap-driver.js @@ -7,13 +7,13 @@ * end of the cycle. * * If it yields, then the initTrap function is called. For all subsequent - * iterations, it is given next(false). yields, the nextBuffer function is - * called with buffer parameters. + * iterations, it is given next(false). If it yields, the nextBuffer function is + * called with buffer iteration parameters. * * On the Host side, sendTrapReply should be called when the answer from the * initTrap step has settled. Calling the trapNextBuffer function with the - * trapID and supplied buffer parameters will rev the giveIt to transfer buffers - * to the takeIt. + * trapID and supplied buffer iteration parameters will rev the giveIt to + * transfer buffers to the takeIt. */ import { assert, details as X } from '@agoric/assert'; From 55a8c8fc6286a635dae659ff150aab4af0aaf4d3 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Wed, 16 Jun 2021 18:56:27 -0600 Subject: [PATCH 11/31] chore(captp): add type linting --- packages/captp/jsconfig.json | 18 ++++++++++++++++++ packages/captp/lib/types.js | 2 ++ packages/captp/package.json | 7 +++++-- 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 packages/captp/jsconfig.json diff --git a/packages/captp/jsconfig.json b/packages/captp/jsconfig.json new file mode 100644 index 00000000000..bb589e78a4e --- /dev/null +++ b/packages/captp/jsconfig.json @@ -0,0 +1,18 @@ +// This file can contain .js-specific Typescript compiler config. +{ + "compilerOptions": { + "target": "esnext", + + "noEmit": true, +/* + // The following flags are for creating .d.ts files: + "noEmit": false, + "declaration": true, + "emitDeclarationOnly": true, +*/ + "downlevelIteration": true, + "strictNullChecks": true, + "moduleResolution": "node", + }, + "include": ["lib/**/*.js", "exported.js"], +} diff --git a/packages/captp/lib/types.js b/packages/captp/lib/types.js index e1df83fbbc8..6eab0c528f6 100644 --- a/packages/captp/lib/types.js +++ b/packages/captp/lib/types.js @@ -1,3 +1,5 @@ +// @ts-check + /** * @typedef {Object} TrapImpl * @property {(target: any, args: Array) => any} applyFunction function diff --git a/packages/captp/package.json b/packages/captp/package.json index d41f7057c0c..eae63730c68 100644 --- a/packages/captp/package.json +++ b/packages/captp/package.json @@ -31,11 +31,14 @@ "test": "ava", "test:xs": "exit 0", "lint-check": "yarn lint", - "lint-fix": "eslint --fix '**/*.js'", - "lint": "eslint 'lib/*.js'" + "lint-fix": "yarn lint:eslint --fix && yarn lint:types", + "lint": "yarn lint:eslint && yarn lint:types", + "lint:eslint": "eslint '**/*.js'", + "lint:types": "tsc -p jsconfig.json" }, "devDependencies": { "@agoric/install-ses": "^0.5.20", + "@agoric/swingset-vat": "^0.18.6", "ava": "^3.12.1" }, "dependencies": { From 254b7354ff763b79a7462aeef042664a9c8101fe Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Wed, 16 Jun 2021 19:00:41 -0600 Subject: [PATCH 12/31] chore(captp): rename lib -> src --- packages/captp/jsconfig.json | 2 +- packages/captp/package.json | 8 ++++---- packages/captp/{lib => src}/captp.js | 0 packages/captp/{lib => src}/index.js | 0 packages/captp/{lib => src}/trap-driver.js | 0 packages/captp/{lib => src}/trap.js | 0 packages/captp/{lib => src}/ts-types.d.ts | 0 packages/captp/{lib => src}/types.js | 0 packages/captp/test/test-crosstalk.js | 2 +- packages/captp/test/test-disco.js | 2 +- packages/captp/test/test-loopback.js | 2 +- packages/captp/test/test-trap.js | 2 +- packages/captp/test/traplib.js | 2 +- 13 files changed, 10 insertions(+), 10 deletions(-) rename packages/captp/{lib => src}/captp.js (100%) rename packages/captp/{lib => src}/index.js (100%) rename packages/captp/{lib => src}/trap-driver.js (100%) rename packages/captp/{lib => src}/trap.js (100%) rename packages/captp/{lib => src}/ts-types.d.ts (100%) rename packages/captp/{lib => src}/types.js (100%) diff --git a/packages/captp/jsconfig.json b/packages/captp/jsconfig.json index bb589e78a4e..6aa8279c4c4 100644 --- a/packages/captp/jsconfig.json +++ b/packages/captp/jsconfig.json @@ -14,5 +14,5 @@ "strictNullChecks": true, "moduleResolution": "node", }, - "include": ["lib/**/*.js", "exported.js"], + "include": ["src/**/*.js", "exported.js"], } diff --git a/packages/captp/package.json b/packages/captp/package.json index eae63730c68..2ec6b7b85b2 100644 --- a/packages/captp/package.json +++ b/packages/captp/package.json @@ -13,14 +13,14 @@ "author": "Michael FIG ", "homepage": "https://github.com/Agoric/agoric-sdk#readme", "license": "Apache-2.0", - "main": "lib/captp.js", - "module": "lib/captp.js", + "main": "src/captp.js", + "module": "src/captp.js", "directories": { - "lib": "lib", + "src": "src", "test": "test" }, "files": [ - "lib" + "src" ], "repository": { "type": "git", diff --git a/packages/captp/lib/captp.js b/packages/captp/src/captp.js similarity index 100% rename from packages/captp/lib/captp.js rename to packages/captp/src/captp.js diff --git a/packages/captp/lib/index.js b/packages/captp/src/index.js similarity index 100% rename from packages/captp/lib/index.js rename to packages/captp/src/index.js diff --git a/packages/captp/lib/trap-driver.js b/packages/captp/src/trap-driver.js similarity index 100% rename from packages/captp/lib/trap-driver.js rename to packages/captp/src/trap-driver.js diff --git a/packages/captp/lib/trap.js b/packages/captp/src/trap.js similarity index 100% rename from packages/captp/lib/trap.js rename to packages/captp/src/trap.js diff --git a/packages/captp/lib/ts-types.d.ts b/packages/captp/src/ts-types.d.ts similarity index 100% rename from packages/captp/lib/ts-types.d.ts rename to packages/captp/src/ts-types.d.ts diff --git a/packages/captp/lib/types.js b/packages/captp/src/types.js similarity index 100% rename from packages/captp/lib/types.js rename to packages/captp/src/types.js diff --git a/packages/captp/test/test-crosstalk.js b/packages/captp/test/test-crosstalk.js index da7dadc28f6..7749c4689cb 100644 --- a/packages/captp/test/test-crosstalk.js +++ b/packages/captp/test/test-crosstalk.js @@ -1,7 +1,7 @@ import '@agoric/install-ses'; import { Far } from '@agoric/marshal'; import test from 'ava'; -import { makeLoopback, E } from '../lib/captp.js'; +import { makeLoopback, E } from '../src/captp.js'; test('prevent crosstalk', async t => { const { makeFar } = makeLoopback('alice'); diff --git a/packages/captp/test/test-disco.js b/packages/captp/test/test-disco.js index 47271bcd352..e3328206ac9 100644 --- a/packages/captp/test/test-disco.js +++ b/packages/captp/test/test-disco.js @@ -1,7 +1,7 @@ import '@agoric/install-ses'; import { Far } from '@agoric/marshal'; import test from 'ava'; -import { E, makeCapTP } from '../lib/captp.js'; +import { E, makeCapTP } from '../src/captp.js'; test('try disconnecting captp', async t => { const objs = []; diff --git a/packages/captp/test/test-loopback.js b/packages/captp/test/test-loopback.js index dba56d0c57d..f30076b0e2e 100644 --- a/packages/captp/test/test-loopback.js +++ b/packages/captp/test/test-loopback.js @@ -2,7 +2,7 @@ import '@agoric/install-ses'; import { Far } from '@agoric/marshal'; import test from 'ava'; -import { E, makeLoopback } from '../lib/captp.js'; +import { E, makeLoopback } from '../src/captp.js'; test('try loopback captp', async t => { const pr = {}; diff --git a/packages/captp/test/test-trap.js b/packages/captp/test/test-trap.js index defec8393ca..abe47983752 100644 --- a/packages/captp/test/test-trap.js +++ b/packages/captp/test/test-trap.js @@ -3,7 +3,7 @@ import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava'; import { Worker } from 'worker_threads'; -import { E, makeLoopback } from '../lib/captp'; +import { E, makeLoopback } from '../src/captp'; import { createHostBootstrap, makeGuest, diff --git a/packages/captp/test/traplib.js b/packages/captp/test/traplib.js index 48b7c2d14c8..0f0fd6e5773 100644 --- a/packages/captp/test/traplib.js +++ b/packages/captp/test/traplib.js @@ -3,7 +3,7 @@ import { assert, details as X } from '@agoric/assert'; import { Far } from '@agoric/marshal'; -import { E, makeCapTP } from '../lib/captp'; +import { E, makeCapTP } from '../src/captp'; export function createHostBootstrap(makeTrapHandler) { // Create a remotable that has a syncable return value. From 8b20562b9cc3917818455ab7d85aa74c9efb3f56 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Wed, 16 Jun 2021 19:34:33 -0600 Subject: [PATCH 13/31] fix(solo): clean up unnecessary deep captp import --- packages/captp/src/index.js | 3 +-- packages/solo/src/captp.js | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/captp/src/index.js b/packages/captp/src/index.js index 4f2376ad22d..dac09a2122d 100644 --- a/packages/captp/src/index.js +++ b/packages/captp/src/index.js @@ -1,6 +1,5 @@ -import { Nat } from '@agoric/nat'; +export { Nat } from '@agoric/nat'; export * from '@agoric/marshal'; export * from './captp.js'; -export { Nat }; diff --git a/packages/solo/src/captp.js b/packages/solo/src/captp.js index abb775b04d0..cd260062ff0 100644 --- a/packages/solo/src/captp.js +++ b/packages/solo/src/captp.js @@ -1,7 +1,4 @@ -// Avoid importing the full captp bundle, which would carry -// in its own makeHardener, etc. -import { makeCapTP } from '@agoric/captp/lib/captp'; -import { E } from '@agoric/eventual-send'; +import { E, makeCapTP } from '@agoric/captp'; export const getCapTPHandler = ( send, From 9e28e9e4eb1ef455eb6786a266c29a61f160f57f Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Tue, 22 Jun 2021 17:06:49 -0600 Subject: [PATCH 14/31] refactor(captp): cleanup style and typing --- packages/captp/SETUP-DELETEME.md | 43 ---------------- packages/captp/src/captp.js | 85 +++++++++++++++++++------------ packages/captp/src/trap-driver.js | 7 +-- packages/captp/src/trap.js | 12 ++--- packages/captp/src/types.js | 23 ++++++--- packages/captp/test/traplib.js | 24 ++++----- packages/marshal/src/marshal.js | 8 --- packages/marshal/src/types.js | 6 +-- 8 files changed, 94 insertions(+), 114 deletions(-) delete mode 100644 packages/captp/SETUP-DELETEME.md diff --git a/packages/captp/SETUP-DELETEME.md b/packages/captp/SETUP-DELETEME.md deleted file mode 100644 index e125c47b1a5..00000000000 --- a/packages/captp/SETUP-DELETEME.md +++ /dev/null @@ -1,43 +0,0 @@ -# Burn after reading - -This file contains instructions for setting up a project based on new-repo. - -After you have completed these instructions, you should do `git rm SETUP-DELETEME.md` - -# Cloning the new-repo repository - -Clone it as: - -``` -$ git clone https://github.com/Agoric/new-repo MyPackageName -$ cd MyPackageName -$ git remote rename origin new-repo -``` - -## Create Agoric/MyPackageName on GitHub - -After creating Agoric/MyPackageName, you should run something like: - -``` -$ git remote add origin git@github.com:Agoric/MyPackageName -$ git push -u origin master -``` - -# Setting up package.json - -The package.json is already set up with organization-prefixed details. You just need to substitute -your package name (usually dash-separated) and your repository name (usually capitalized words -concatenated): - -1. `sed -i.bak -e 's/@PACKAGE@/my-package-name/g; s/@REPO@/MyPackageName/g' package.json` -2. `rm package.json.bak` - -# Setting up README.md - -You will definitely want to edit the `README.md` file, then begin committing and pushing as usual. - -# Setting up CircleCI - -1. Go to https://circleci.com/gh/Agoric -2. Click the "Add Project" button to the left -3. Make sure that your repo has the `.circleci/config.yml` file that new-repo has diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index 435d4aea0e1..370a068a161 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -21,11 +21,6 @@ import './types.js'; export { E }; -/** - * @template T - * @typedef {import('@agoric/eventual-send').ERef} ERef - */ - /** * @typedef {Object} CapTPOptions the options to makeCapTP * @property {(err: any) => void} onReject @@ -50,7 +45,12 @@ export { E }; * @param {any} bootstrapObj the object to export to the other side * @param {Partial} opts options to the connection */ -export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { +export const makeCapTP = ( + ourId, + rawSend, + bootstrapObj = undefined, + opts = {}, +) => { const { onReject = err => console.error('CapTP', ourId, 'exception:', err), Remotable = defaultRemotable, @@ -66,7 +66,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { /** @type {any} */ let unplug = false; - async function quietReject(reason = undefined, returnIt = true) { + const quietReject = async (reason = undefined, returnIt = true) => { if ((unplug === false || reason !== unplug) && reason !== undefined) { onReject(reason); } @@ -79,23 +79,25 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { const p = Promise.reject(reason); p.catch(_ => {}); return p; - } + }; /** * @param {Record} obj */ - function send(obj) { + const send = obj => { // Don't throw here if unplugged, just don't send. if (unplug === false) { rawSend(obj); } - } + }; - // convertValToSlot and convertSlotToVal both perform side effects, - // populating the c-lists (imports/exports/questions/answers) upon - // marshalling/unmarshalling. As we traverse the datastructure representing - // the message, we discover what we need to import/export and send relevant - // messages across the wire. + /** + * convertValToSlot and convertSlotToVal both perform side effects, + * populating the c-lists (imports/exports/questions/answers) upon + * marshalling/unmarshalling. As we traverse the datastructure representing + * the message, we discover what we need to import/export and send relevant + * messages across the wire. + */ const { serialize, unserialize } = makeMarshal( // eslint-disable-next-line no-use-before-define convertValToSlot, @@ -112,8 +114,9 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { const trapHost = giveTrapReply && makeTrapHost(); const trapGuest = takeTrapReply && makeTrapGuest(unserialize); - /** @type {WeakMap} */ + /** @type {WeakMap} */ const valToSlot = new WeakMap(); // exports looked up by val + /** @type {Map} */ const slotToVal = new Map(); // reverse const exportedTrapHandlers = new WeakSet(); @@ -133,13 +136,21 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { /** @type {Map} */ const imports = new Map(); // chosen by our peer - // Called at marshalling time. Either retrieves an existing export, or if - // not yet exported, records this exported object. If a promise, sets up a - // promise listener to inform the other side when the promise is - // fulfilled/broken. + /** + * Called at marshalling time. Either retrieves an existing export, or if + * not yet exported, records this exported object. If a promise, sets up a + * promise listener to inform the other side when the promise is + * fulfilled/broken. + * + * @type {ConvertValToSlot} + */ function convertValToSlot(val) { if (!valToSlot.has(val)) { - // new export + /** + * new export + * + * @type {CapTPSlot} + */ let slot; if (isPromise(val)) { // This is a promise, so we're going to increment the lastPromiseId @@ -183,7 +194,9 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { } // At this point, the value is guaranteed to be exported, so return the // associated slot number. - return valToSlot.get(val); + const slot = valToSlot.get(val); + assert.typeof(slot, 'string'); + return slot; } /** @@ -192,7 +205,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { * * @returns {[number, ReturnType]} */ - function makeQuestion() { + const makeQuestion = () => { lastQuestionID += 1; const questionID = lastQuestionID; // eslint-disable-next-line no-use-before-define @@ -214,10 +227,10 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { slotToVal.set(resultVPID, pr.p); return [questionID, pr]; - } + }; // Make a remote promise for `target` (an id in the questions table) - function makeRemoteKit(target) { + const makeRemoteKit = target => { // This handler is set up such that it will transform both // attribute access and method invocation of this remote promise // as also being questions / remote handled promises @@ -279,9 +292,13 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { pr.p.catch(e => quietReject(e, false)); return harden(pr); - } + }; - // Set up import + /** + * Set up import + * + * @type {ConvertSlotToVal} + */ function convertSlotToVal(theirSlot, iface = undefined) { let val; // Invert slot direction from other side. @@ -602,7 +619,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { } return harden(rets); -} +}; /** * @typedef {Object} LoopbackOpts @@ -614,10 +631,14 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) { * * @param {string} ourId * @param {Partial} [opts] - * @returns {{ makeFar(x: T): ERef, makeNear(x: T): ERef, - * makeTrapHandler(x: T): T, Trap: Trap }} + * @returns {{ + * makeFar(x: T): ERef, + * makeNear(x: T): ERef, + * makeTrapHandler(x: T): T, + * Trap: Trap + * }} */ -export function makeLoopback(ourId, opts = {}) { +export const makeLoopback = (ourId, opts = {}) => { const { Far = defaultFar } = opts; let nextNonce = 0; const nonceToRef = new Map(); @@ -701,4 +722,4 @@ export function makeLoopback(ourId, opts = {}) { makeTrapHandler, Trap, }; -} +}; diff --git a/packages/captp/src/trap-driver.js b/packages/captp/src/trap-driver.js index bb87c2c1d13..2cd2f7893b1 100644 --- a/packages/captp/src/trap-driver.js +++ b/packages/captp/src/trap-driver.js @@ -12,13 +12,14 @@ * * On the Host side, sendTrapReply should be called when the answer from the * initTrap step has settled. Calling the trapNextBuffer function with the - * trapID and supplied buffer iteration parameters will rev the giveIt to - * transfer buffers to the takeIt. + * trapID and supplied buffer iteration parameters will step through the giveIt + * to transfer buffers to the takeIt. */ import { assert, details as X } from '@agoric/assert'; import './types'; +import '@agoric/marshal/exported'; // Something to make our reply sequences more interesting. const TRAP_FIRST_SEQUENCE_NUMBER = 23; @@ -89,7 +90,7 @@ export const makeTrapHost = () => { /** * Create the guest side of the trap interface. * - * @param {(ser: any) => any} unserialize + * @param {Unserialize} unserialize */ export const makeTrapGuest = unserialize => { return { diff --git a/packages/captp/src/trap.js b/packages/captp/src/trap.js index bb5120dcd2a..df03b71ce7d 100644 --- a/packages/captp/src/trap.js +++ b/packages/captp/src/trap.js @@ -42,7 +42,7 @@ const readOnlyProxyHandler = { * @param {TrapImpl} syncImpl * @returns {ProxyHandler} */ -function TrapProxyHandler(x, syncImpl) { +const TrapProxyHandler = (x, syncImpl) => { return harden({ ...readOnlyProxyHandler, get(_target, p, _receiver) { @@ -56,17 +56,17 @@ function TrapProxyHandler(x, syncImpl) { return true; }, }); -} +}; /** * @param {TrapImpl} syncImpl * @returns {Trap} */ -export function makeTrap(syncImpl) { - function Trap(x) { +export const makeTrap = syncImpl => { + const Trap = x => { const handler = TrapProxyHandler(x, syncImpl); return harden(new Proxy(() => {}, handler)); - } + }; const makeTrapGetterProxy = x => { const handler = harden({ @@ -84,4 +84,4 @@ export function makeTrap(syncImpl) { Trap.get = makeTrapGetterProxy; return harden(Trap); -} +}; diff --git a/packages/captp/src/types.js b/packages/captp/src/types.js index 6eab0c528f6..66247817bf8 100644 --- a/packages/captp/src/types.js +++ b/packages/captp/src/types.js @@ -1,5 +1,12 @@ // @ts-check +/** + * @template T + * @typedef {import('@agoric/eventual-send').ERef} ERef + */ + +/** @typedef {string} CapTPSlot */ + /** * @typedef {Object} TrapImpl * @property {(target: any, args: Array) => any} applyFunction function @@ -12,9 +19,11 @@ */ /** - * @typedef {[boolean, any]} TrapCompleted The head of the pair is the + * @typedef {[boolean, CapData]} TrapCompletion The head of the pair is the * `isRejected` value indicating whether the sync call was an exception, and - * tail of the pair is the serialized value (or error). + * tail of the pair is the serialized fulfillment value or rejection reason. + * (The fulfillment value is a non-thenable. The rejection reason is normally + * an error.) */ /** @@ -28,9 +37,9 @@ * exception. * * @param {keyof TrapImpl} implMethod the TrapImpl method that was called - * @param {string} slot the target slot + * @param {CapTPSlot} slot the target slot * @param {Array} implArgs arguments to the TrapImpl method - * @returns {Generator} + * @returns {Generator} */ /** @@ -40,10 +49,10 @@ * cooperate over a specific CapTP connection, and via any out-of-band * mechanisms as well. * - * @param {TrapCompleted[0]} isReject whether the reply to communicate was a + * @param {TrapCompletion[0]} isReject whether the reply to communicate was a * rejection or a regular return - * @param {TrapCompleted[1]} serialized the marshal-serialized (JSONable) data - * to be communicated to the other side + * @param {TrapCompletion[1]} serialized the marshal-serialized data to be + * communicated to the other side. Note that the serialized data is JSONable. * @returns {AsyncGenerator} */ diff --git a/packages/captp/test/traplib.js b/packages/captp/test/traplib.js index 0f0fd6e5773..5220584fcd5 100644 --- a/packages/captp/test/traplib.js +++ b/packages/captp/test/traplib.js @@ -5,7 +5,7 @@ import { assert, details as X } from '@agoric/assert'; import { Far } from '@agoric/marshal'; import { E, makeCapTP } from '../src/captp'; -export function createHostBootstrap(makeTrapHandler) { +export const createHostBootstrap = makeTrapHandler => { // Create a remotable that has a syncable return value. return Far('test traps', { getTraps(n) { @@ -19,9 +19,9 @@ export function createHostBootstrap(makeTrapHandler) { }); }, }); -} +}; -export async function runTrapTests(t, Trap, bs, unwrapsPromises) { +export const runTrapTests = async (t, Trap, bs, unwrapsPromises) => { // Demonstrate async compatibility of traps. const pn = E(E(bs).getTraps(3)).getN(); t.is(Promise.resolve(pn), pn); @@ -55,9 +55,9 @@ export async function runTrapTests(t, Trap, bs, unwrapsPromises) { instanceOf: Error, message: /imported target was not created with makeTrapHandler/, }); -} +}; -function createGuestBootstrap(Trap, other) { +const createGuestBootstrap = (Trap, other) => { return Far('tests', { async runTrapTests(unwrapsPromises) { const mockT = { @@ -78,19 +78,19 @@ function createGuestBootstrap(Trap, other) { return true; }, }); -} +}; const SEM_WAITING = 0; const SEM_READY = 2; const SEM_REJECT = 1; -function makeBufs(sab) { +const makeBufs = sab => { const sembuf = new Int32Array(sab, 0, 2 * Int32Array.BYTES_PER_ELEMENT); const databuf = new Uint8Array(sab, sembuf.byteLength); return { sembuf, databuf }; -} +}; -export function makeHost(send, sab) { +export const makeHost = (send, sab) => { const { sembuf, databuf } = makeBufs(sab); const te = new TextEncoder(); const { dispatch, getBootstrap, makeTrapHandler } = makeCapTP( @@ -111,9 +111,9 @@ export function makeHost(send, sab) { ); return { dispatch, getBootstrap }; -} +}; -export function makeGuest(send, sab) { +export const makeGuest = (send, sab) => { const { sembuf, databuf } = makeBufs(sab); const td = new TextDecoder('utf-8'); const { dispatch, getBootstrap, Trap } = makeCapTP( @@ -138,4 +138,4 @@ export function makeGuest(send, sab) { }, ); return { dispatch, getBootstrap, Trap }; -} +}; diff --git a/packages/marshal/src/marshal.js b/packages/marshal/src/marshal.js index 4b5309ebeaa..b2f6b0d4855 100644 --- a/packages/marshal/src/marshal.js +++ b/packages/marshal/src/marshal.js @@ -148,15 +148,7 @@ const makeRemotableProto = (oldProto, iface) => { const QCLASS = '@qclass'; export { QCLASS }; -/** - * @template Slot - * @type {ConvertValToSlot} - */ const defaultValToSlotFn = x => x; -/** - * @template Slot - * @type {ConvertSlotToVal} - */ const defaultSlotToValFn = (x, _) => x; /** diff --git a/packages/marshal/src/types.js b/packages/marshal/src/types.js index 4a64f2ae3ca..9c455c36625 100644 --- a/packages/marshal/src/types.js +++ b/packages/marshal/src/types.js @@ -145,10 +145,10 @@ /** * @template Slot * @callback MakeMarshal - * @param {ConvertValToSlot=} convertValToSlot - * @param {ConvertSlotToVal=} convertSlotToVal + * @param {ConvertValToSlot=} convertValToSlot + * @param {ConvertSlotToVal=} convertSlotToVal * @param {MakeMarshalOptions=} options - * @returns {Marshal} + * @returns {Marshal} */ /** From 3800ad0deafd34b4f4de72980dd305397768c8ea Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Sun, 11 Jul 2021 17:49:42 -0600 Subject: [PATCH 15/31] refactor(captp): less opinionated trapGuest (part 1/2) --- packages/captp/src/captp.js | 119 +++++++++++++++++++----------- packages/captp/src/trap-driver.js | 81 -------------------- packages/captp/src/types.js | 30 +++++--- packages/captp/test/traplib.js | 24 +++--- 4 files changed, 105 insertions(+), 149 deletions(-) diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index 370a068a161..24651a0bf54 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -15,12 +15,19 @@ import { isPromise } from '@agoric/promise-kit'; import { assert, details as X } from '@agoric/assert'; import { makeTrap, nearTrapImpl } from './trap.js'; -import { makeTrapGuest, makeTrapHost } from './trap-driver.js'; +import { makeTrapHost } from './trap-driver.js'; import './types.js'; export { E }; +/** + * @param {any} maybeThenable + * @returns {boolean} + */ +const isThenable = maybeThenable => + maybeThenable && typeof maybeThenable.then === 'function'; + /** * @typedef {Object} CapTPOptions the options to makeCapTP * @property {(err: any) => void} onReject @@ -29,14 +36,19 @@ export { E }; * @property {typeof defaultMakeMarshal} makeMarshal * @property {number} epoch an integer tag to attach to all messages in order to * assist in ignoring earlier defunct instance's messages - * @property {TakeTrapReply} takeTrapReply if specified, enable this CapTP - * (guest) to use Trap(target) to block while the recipient (host) resolves and + * @property {TrapGuest} trapGuest if specified, enable this CapTP (guest) to + * use Trap(target) to block while the recipient (host) resolves and * communicates the message * @property {GiveTrapReply} giveTrapReply if specified, enable this CapTP * (host) to serve objects marked with makeTrapHandler to synchronous clients * (guests) */ +const TRAP_FIRST_CALL = { + toString: () => 'TRAP_FIRST_CALL', +}; +harden(TRAP_FIRST_CALL); + /** * Create a CapTP connection. * @@ -57,7 +69,7 @@ export const makeCapTP = ( makeMarshal = defaultMakeMarshal, Far = defaultFar, epoch = 0, - takeTrapReply, + trapGuest, giveTrapReply, } = opts; @@ -112,7 +124,6 @@ export const makeCapTP = ( ); const trapHost = giveTrapReply && makeTrapHost(); - const trapGuest = takeTrapReply && makeTrapGuest(unserialize); /** @type {WeakMap} */ const valToSlot = new WeakMap(); // exports looked up by val @@ -533,7 +544,7 @@ export const makeCapTP = ( Trap: /** @type {Trap | undefined} */ (undefined), }; - if (takeTrapReply) { + if (trapGuest) { // Create the Trap proxy maker. const makeTrapImpl = implMethod => (target, ...implArgs) => { assert( @@ -550,9 +561,11 @@ export const makeCapTP = ( slot[0] === 't', X`Trap(${target}) imported target was not created with makeTrapHandler`, ); - assert( - takeTrapReply, - X`Trap(${target}) failed; no opts.takeTrapReply supplied to makeCapTP`, + assert(trapGuest, X`Trap(${target}) internal error; no trapGuest`); + assert.typeof( + trapGuest.doTrap, + 'function', + X`Trap(${target}) failed; no opts.trapGuest.doTrap function supplied to makeCapTP`, ); // Send a "trap" message. @@ -582,29 +595,44 @@ export const makeCapTP = ( } } - assert(trapGuest, X`Trap(${target}) internal error; no trapGuest`); + // Set up the trap call with its identifying information and a way to send + // messages over the current CapTP data channel. + const [isException, serialized] = trapGuest.doTrap({ + implMethod, + slot, + implArgs, + trapSend: (data = TRAP_FIRST_CALL) => { + if (data === TRAP_FIRST_CALL) { + // Send the call metadata over the connection. + send({ + type: 'CTP_CALL', + epoch, + trap: true, // This is the magic marker. + questionID, + target: slot, + method, + }); + } else { + send({ + type: 'CTP_TRAP_NEXT_BUFFER', + epoch, + questionID, + trapBuf: data, + }); + } + }, + }); - // Set up the trap call with its identifying information. - const takeIt = takeTrapReply(implMethod, slot, implArgs); - return trapGuest.doTrap( - takeIt, - () => - send({ - type: 'CTP_CALL', - epoch, - trap: true, // This is the magic marker. - questionID, - target: slot, - method, - }), - trapBuf => - send({ - type: 'CTP_TRAP_NEXT_BUFFER', - epoch, - questionID, - trapBuf, - }), + const value = unserialize(serialized); + assert( + !isThenable(value), + X`Trap() reply cannot be a Thenable; have ${value}`, ); + + if (isException) { + throw value; + } + return value; }; /** @type {TrapImpl} */ @@ -666,22 +694,23 @@ export const makeLoopback = (ourId, opts = {}) => { dispatch: nearDispatch, getBootstrap: getFarBootstrap, } = makeCapTP(`near-${ourId}`, o => farDispatch(o), bootstrap, { - // eslint-disable-next-line require-yield - *takeTrapReply(implMethod, slot, implArgs) { - let value; - let isException = false; - try { - // Cross the boundary to pull out the far object. + trapGuest: { + doTrap: ({ implMethod, slot, implArgs }) => { + let value; + let isException = false; + try { + // Cross the boundary to pull out the far object. + // eslint-disable-next-line no-use-before-define + const far = farUnserialize({ body: slotBody, slots: [slot] }); + value = nearTrapImpl[implMethod](far, implArgs[0], implArgs[1]); + } catch (e) { + isException = true; + value = e; + } + harden(value); // eslint-disable-next-line no-use-before-define - const far = farUnserialize({ body: slotBody, slots: [slot] }); - value = nearTrapImpl[implMethod](far, implArgs[0], implArgs[1]); - } catch (e) { - isException = true; - value = e; - } - harden(value); - // eslint-disable-next-line no-use-before-define - return [isException, farSerialize(value)]; + return [isException, farSerialize(value)]; + }, }, }); assert(Trap); diff --git a/packages/captp/src/trap-driver.js b/packages/captp/src/trap-driver.js index 2cd2f7893b1..251ed4e3232 100644 --- a/packages/captp/src/trap-driver.js +++ b/packages/captp/src/trap-driver.js @@ -24,13 +24,6 @@ import '@agoric/marshal/exported'; // Something to make our reply sequences more interesting. const TRAP_FIRST_SEQUENCE_NUMBER = 23; -/** - * @param {any} maybeThenable - * @returns {boolean} - */ -const isThenable = maybeThenable => - maybeThenable && typeof maybeThenable.then === 'function'; - /** * Create the host side of the trap interface. */ @@ -86,77 +79,3 @@ export const makeTrapHost = () => { }, }; }; - -/** - * Create the guest side of the trap interface. - * - * @param {Unserialize} unserialize - */ -export const makeTrapGuest = unserialize => { - return { - /** - * @param {ReturnType} takeIt - * @param {() => void} [initTrap] - * @param {(trapParams: any) => void} [nextBuffer] - */ - doTrap: (takeIt, initTrap = undefined, nextBuffer = undefined) => { - /** - * @param {boolean} firstTime - */ - const nextTrap = firstTime => { - const status = takeIt.next(firstTime); - assert( - !isThenable(status), - X`takeTrapReply must be a fully-synchronous Generator but it.next() returned a Thenable ${status}`, - ); - return status; - }; - - // Prepare the reply, in case there needs to be something initialized - // before the call is ready. - let firstTime = true; - let status = nextTrap(firstTime); - let seq = TRAP_FIRST_SEQUENCE_NUMBER; - while (!status.done) { - if (firstTime) { - // Mark the send as a "trap", but handle it asynchronously on the - // other side. - assert.typeof( - initTrap, - 'function', - X`Trap() initTrap function must be defined`, - ); - firstTime = false; - initTrap(); - } else { - // We weren't done fetching the entire result in the first iteration, - // so thread through the iterator values into the "next" messages to - // retrieve the entire serialized data. - assert.typeof( - nextBuffer, - 'function', - X`Trap() nextBuffer function must be defined`, - ); - seq += 1; - nextBuffer({ - seq, - data: status.value, - }); - } - status = nextTrap(firstTime); - } - - // We've finished claiming the return value. - const [isReject, serialized] = status.value; - const value = unserialize(serialized); - assert( - !isThenable(value), - X`Trap() reply cannot be a Thenable; have ${value}`, - ); - if (isReject) { - throw value; - } - return value; - }, - }; -}; diff --git a/packages/captp/src/types.js b/packages/captp/src/types.js index 66247817bf8..aba9dc327e5 100644 --- a/packages/captp/src/types.js +++ b/packages/captp/src/types.js @@ -27,19 +27,25 @@ */ /** - * @callback TakeTrapReply Return a Generator to use in a tight loop until it - * returns a TrapCompleted value indicating the final results of a Trap call. - * - * The rules are that the call request is only sent if the first iteration - * yields instead of returning a result. For each yield, the other end of the - * connection's GiveTrapReply async iterator is called. When the reassembled - * TrapCompleted result is returned, the Trap() call either returns or throws an - * exception. + * @typedef DoTrapInfo the argument to DoTrap + * @property {keyof TrapImpl} implMethod the TrapImpl method that was called + * @property {CapTPSlot} slot the target slot + * @property {Array} implArgs arguments to the TrapImpl method + * @property {(obj?: any) => void} trapSend send a message over the existing + * CapTP data channel for the trapHost to handle + */ + +/** + * @callback DoTrap Use out-of-band communications to synchronously return a + * TrapCompleted value indicating the final results of a Trap call. * - * @param {keyof TrapImpl} implMethod the TrapImpl method that was called - * @param {CapTPSlot} slot the target slot - * @param {Array} implArgs arguments to the TrapImpl method - * @returns {Generator} + * @param {DoTrapInfo} info + * @returns {TrapCompletion} + */ + +/** + * @typedef {Object} TrapGuest + * @property {DoTrap} doTrap fulfill a Trap request */ /** diff --git a/packages/captp/test/traplib.js b/packages/captp/test/traplib.js index 5220584fcd5..bb8cecb7d1c 100644 --- a/packages/captp/test/traplib.js +++ b/packages/captp/test/traplib.js @@ -121,19 +121,21 @@ export const makeGuest = (send, sab) => { send, () => createGuestBootstrap(Trap, getBootstrap()), { - *takeTrapReply() { - // Initialize the reply. - sembuf[0] = SEM_WAITING; - yield; + trapGuest: { + doTrap: ({ trapSend }) => { + // Initialize the reply. + sembuf[0] = SEM_WAITING; + trapSend(); - // Wait for the reply to return. - Atomics.wait(sembuf, 0, SEM_WAITING); + // Wait for the reply to return. + Atomics.wait(sembuf, 0, SEM_WAITING); - // eslint-disable-next-line no-bitwise - const isReject = !!(sembuf[0] & 1); - const data = td.decode(databuf.slice(0, sembuf[1])); - const ser = JSON.parse(data); - return [isReject, ser]; + // eslint-disable-next-line no-bitwise + const isReject = !!(sembuf[0] & 1); + const data = td.decode(databuf.slice(0, sembuf[1])); + const ser = JSON.parse(data); + return [isReject, ser]; + }, }, }, ); From 75a9a6c46fb861e417ec30db883a78bc06c3847b Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Sun, 11 Jul 2021 18:45:17 -0600 Subject: [PATCH 16/31] refactor(captp): make continuations explicit (part 2/3) --- packages/captp/src/captp.js | 103 ++++++++++++++++++--------------- packages/captp/src/types.js | 18 +++--- packages/captp/test/traplib.js | 29 +++++----- 3 files changed, 81 insertions(+), 69 deletions(-) diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index 24651a0bf54..e6998d10640 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -15,7 +15,6 @@ import { isPromise } from '@agoric/promise-kit'; import { assert, details as X } from '@agoric/assert'; import { makeTrap, nearTrapImpl } from './trap.js'; -import { makeTrapHost } from './trap-driver.js'; import './types.js'; @@ -39,9 +38,8 @@ const isThenable = maybeThenable => * @property {TrapGuest} trapGuest if specified, enable this CapTP (guest) to * use Trap(target) to block while the recipient (host) resolves and * communicates the message - * @property {GiveTrapReply} giveTrapReply if specified, enable this CapTP - * (host) to serve objects marked with makeTrapHandler to synchronous clients - * (guests) + * @property {TrapHost} trapHost if specified, enable this CapTP (host) to serve + * objects marked with makeTrapHandler to synchronous clients (guests) */ const TRAP_FIRST_CALL = { @@ -70,12 +68,14 @@ export const makeCapTP = ( Far = defaultFar, epoch = 0, trapGuest, - giveTrapReply, + trapHost, } = opts; const disconnectReason = id => Error(`${JSON.stringify(id)} connection closed`); + const trapGiveMore = new Map(); + /** @type {any} */ let unplug = false; const quietReject = async (reason = undefined, returnIt = true) => { @@ -123,8 +123,6 @@ export const makeCapTP = ( }, ); - const trapHost = giveTrapReply && makeTrapHost(); - /** @type {WeakMap} */ const valToSlot = new WeakMap(); // exports looked up by val /** @type {Map} */ @@ -398,15 +396,21 @@ export const makeCapTP = ( X`Refused Trap(${val}) because target was not registered with makeTrapHandler`, ); assert.typeof( - giveTrapReply, + trapHost, 'function', - X`CapTP cannot answer Trap(x) without opts.giveTrapReply`, + X`CapTP cannot answer Trap(x) without a trapHost function`, ); - assert(trapHost, X`CatTP internal error; trapHost is not defined`); sendReturn = async (isReject, value) => { const serialized = serialize(harden(value)); - const it = giveTrapReply(isReject, serialized); - await trapHost.sendTrapReply(questionID, it); + const giveMore = trapHost([isReject, serialized]); + if (giveMore) { + assert.typeof( + giveMore, + 'function', + X`CapTP trapHost.reply result can only be a giveMore function`, + ); + trapGiveMore.set(questionID, giveMore); + } }; } catch (e) { sendReturn(true, e); @@ -435,9 +439,26 @@ export const makeCapTP = ( .catch(rej => sendReturn(true, rej)) .catch(rej => quietReject(rej, false)); }, - // Have the host serve the next buffer request. - CTP_TRAP_NEXT_BUFFER: - trapHost && (obj => trapHost.trapNextBuffer(obj.questionID, obj.trapBuf)), + // Have the host serve more of the reply. + CTP_TRAP_TAKE_MORE: obj => { + assert(trapHost, X`CTP_TRAP_TAKE_MORE is impossible without a trapHost`); + const giveMore = trapGiveMore.get(obj.questionID); + trapGiveMore.delete(obj.questionID); + assert.typeof( + giveMore, + 'function', + X`CTP_TRAP_TAKE_MORE did not expect ${obj.questionID}`, + ); + const nextGiveMore = giveMore(obj.data); + if (nextGiveMore) { + assert.typeof( + nextGiveMore, + 'function', + X`CapTP giveMore result can only be another giveMore function`, + ); + trapGiveMore.set(obj.questionID, nextGiveMore); + } + }, // Answer to one of our questions. async CTP_RETURN(obj) { const { result, exception, answerID } = obj; @@ -545,6 +566,8 @@ export const makeCapTP = ( }; if (trapGuest) { + assert.typeof(trapGuest, 'function', X`opts.trapGuest must be a function`); + // Create the Trap proxy maker. const makeTrapImpl = implMethod => (target, ...implArgs) => { assert( @@ -561,12 +584,6 @@ export const makeCapTP = ( slot[0] === 't', X`Trap(${target}) imported target was not created with makeTrapHandler`, ); - assert(trapGuest, X`Trap(${target}) internal error; no trapGuest`); - assert.typeof( - trapGuest.doTrap, - 'function', - X`Trap(${target}) failed; no opts.trapGuest.doTrap function supplied to makeCapTP`, - ); // Send a "trap" message. lastQuestionID += 1; @@ -597,11 +614,11 @@ export const makeCapTP = ( // Set up the trap call with its identifying information and a way to send // messages over the current CapTP data channel. - const [isException, serialized] = trapGuest.doTrap({ + const [isException, serialized] = trapGuest({ implMethod, slot, implArgs, - trapSend: (data = TRAP_FIRST_CALL) => { + takeMore: (data = TRAP_FIRST_CALL) => { if (data === TRAP_FIRST_CALL) { // Send the call metadata over the connection. send({ @@ -614,10 +631,10 @@ export const makeCapTP = ( }); } else { send({ - type: 'CTP_TRAP_NEXT_BUFFER', + type: 'CTP_TRAP_TAKE_MORE', epoch, questionID, - trapBuf: data, + data, }); } }, @@ -694,23 +711,21 @@ export const makeLoopback = (ourId, opts = {}) => { dispatch: nearDispatch, getBootstrap: getFarBootstrap, } = makeCapTP(`near-${ourId}`, o => farDispatch(o), bootstrap, { - trapGuest: { - doTrap: ({ implMethod, slot, implArgs }) => { - let value; - let isException = false; - try { - // Cross the boundary to pull out the far object. - // eslint-disable-next-line no-use-before-define - const far = farUnserialize({ body: slotBody, slots: [slot] }); - value = nearTrapImpl[implMethod](far, implArgs[0], implArgs[1]); - } catch (e) { - isException = true; - value = e; - } - harden(value); + trapGuest: ({ implMethod, slot, implArgs }) => { + let value; + let isException = false; + try { + // Cross the boundary to pull out the far object. // eslint-disable-next-line no-use-before-define - return [isException, farSerialize(value)]; - }, + const far = farUnserialize({ body: slotBody, slots: [slot] }); + value = nearTrapImpl[implMethod](far, implArgs[0], implArgs[1]); + } catch (e) { + isException = true; + value = e; + } + harden(value); + // eslint-disable-next-line no-use-before-define + return [isException, farSerialize(value)]; }, }); assert(Trap); @@ -721,11 +736,7 @@ export const makeLoopback = (ourId, opts = {}) => { getBootstrap: getNearBootstrap, unserialize: farUnserialize, serialize: farSerialize, - } = makeCapTP(`far-${ourId}`, nearDispatch, bootstrap, { - giveTrapReply(_isReject, _serialized) { - throw Error(`makeLoopback giveTrapReply is not expected to be called`); - }, - }); + } = makeCapTP(`far-${ourId}`, nearDispatch, bootstrap); farDispatch = dispatch; const farGetter = E.get(getFarBootstrap()).refGetter; diff --git a/packages/captp/src/types.js b/packages/captp/src/types.js index aba9dc327e5..2c005123f84 100644 --- a/packages/captp/src/types.js +++ b/packages/captp/src/types.js @@ -27,25 +27,29 @@ */ /** - * @typedef DoTrapInfo the argument to DoTrap + * @typedef TrapRequest the argument to TrapGuest * @property {keyof TrapImpl} implMethod the TrapImpl method that was called * @property {CapTPSlot} slot the target slot * @property {Array} implArgs arguments to the TrapImpl method - * @property {(obj?: any) => void} trapSend send a message over the existing - * CapTP data channel for the trapHost to handle + * @property {(data?: any) => void} takeMore send some data over the existing + * CapTP data channel for the trapHost to receive and supply us with more of the + * synchronous result */ /** - * @callback DoTrap Use out-of-band communications to synchronously return a + * @callback TrapGuest Use out-of-band communications to synchronously return a * TrapCompleted value indicating the final results of a Trap call. * - * @param {DoTrapInfo} info + * @param {TrapRequest} req * @returns {TrapCompletion} */ /** - * @typedef {Object} TrapGuest - * @property {DoTrap} doTrap fulfill a Trap request + * @callback TrapHost start the process of transferring the Trap request's + * results + * @param {TrapCompletion} completion + * @returns {void | ((data: any) => void)} If a function is returned, it will + * satisfy a future `takeMore`. */ /** diff --git a/packages/captp/test/traplib.js b/packages/captp/test/traplib.js index bb8cecb7d1c..8ccb9209bfb 100644 --- a/packages/captp/test/traplib.js +++ b/packages/captp/test/traplib.js @@ -85,7 +85,7 @@ const SEM_READY = 2; const SEM_REJECT = 1; const makeBufs = sab => { - const sembuf = new Int32Array(sab, 0, 2 * Int32Array.BYTES_PER_ELEMENT); + const sembuf = new Int32Array(sab, 0, 2); const databuf = new Uint8Array(sab, sembuf.byteLength); return { sembuf, databuf }; }; @@ -98,8 +98,7 @@ export const makeHost = (send, sab) => { send, () => createHostBootstrap(makeTrapHandler), { - // eslint-disable-next-line require-yield - async *giveTrapReply(isReject, ser) { + trapHost: ([isReject, ser]) => { // We need a bufferable message. const data = JSON.stringify(ser); const { written } = te.encodeInto(data, databuf); @@ -121,21 +120,19 @@ export const makeGuest = (send, sab) => { send, () => createGuestBootstrap(Trap, getBootstrap()), { - trapGuest: { - doTrap: ({ trapSend }) => { - // Initialize the reply. - sembuf[0] = SEM_WAITING; - trapSend(); + trapGuest: ({ takeMore: trapSend }) => { + // Initialize the reply. + sembuf[0] = SEM_WAITING; + trapSend(); - // Wait for the reply to return. - Atomics.wait(sembuf, 0, SEM_WAITING); + // Wait for the reply to return. + Atomics.wait(sembuf, 0, SEM_WAITING); - // eslint-disable-next-line no-bitwise - const isReject = !!(sembuf[0] & 1); - const data = td.decode(databuf.slice(0, sembuf[1])); - const ser = JSON.parse(data); - return [isReject, ser]; - }, + // eslint-disable-next-line no-bitwise + const isReject = !!(sembuf[0] & 1); + const data = td.decode(databuf.slice(0, sembuf[1])); + const ser = JSON.parse(data); + return [isReject, ser]; }, }, ); From 57ebb714069e4cb0296b9607595bfdf62bdd62d8 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Sun, 11 Jul 2021 19:40:32 -0600 Subject: [PATCH 17/31] test(captp): test takeMore implementation (part 3/3) --- packages/captp/src/captp.js | 13 +++---- packages/captp/src/types.js | 2 +- packages/captp/test/test-trap.js | 3 +- packages/captp/test/traplib.js | 65 ++++++++++++++++++++++++-------- 4 files changed, 57 insertions(+), 26 deletions(-) diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index e6998d10640..1d824d6008e 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -42,11 +42,6 @@ const isThenable = maybeThenable => * objects marked with makeTrapHandler to synchronous clients (guests) */ -const TRAP_FIRST_CALL = { - toString: () => 'TRAP_FIRST_CALL', -}; -harden(TRAP_FIRST_CALL); - /** * Create a CapTP connection. * @@ -440,7 +435,7 @@ export const makeCapTP = ( .catch(rej => quietReject(rej, false)); }, // Have the host serve more of the reply. - CTP_TRAP_TAKE_MORE: obj => { + CTP_TRAP_TAKE_MORE: async obj => { assert(trapHost, X`CTP_TRAP_TAKE_MORE is impossible without a trapHost`); const giveMore = trapGiveMore.get(obj.questionID); trapGiveMore.delete(obj.questionID); @@ -614,13 +609,15 @@ export const makeCapTP = ( // Set up the trap call with its identifying information and a way to send // messages over the current CapTP data channel. + let isFirst = true; const [isException, serialized] = trapGuest({ implMethod, slot, implArgs, - takeMore: (data = TRAP_FIRST_CALL) => { - if (data === TRAP_FIRST_CALL) { + takeMore: data => { + if (isFirst) { // Send the call metadata over the connection. + isFirst = false; send({ type: 'CTP_CALL', epoch, diff --git a/packages/captp/src/types.js b/packages/captp/src/types.js index 2c005123f84..7d76442e1b6 100644 --- a/packages/captp/src/types.js +++ b/packages/captp/src/types.js @@ -48,7 +48,7 @@ * @callback TrapHost start the process of transferring the Trap request's * results * @param {TrapCompletion} completion - * @returns {void | ((data: any) => void)} If a function is returned, it will + * @returns {undefined | ((data: any) => void)} If a function is returned, it will * satisfy a future `takeMore`. */ diff --git a/packages/captp/test/test-trap.js b/packages/captp/test/test-trap.js index abe47983752..782637e16c7 100644 --- a/packages/captp/test/test-trap.js +++ b/packages/captp/test/test-trap.js @@ -12,7 +12,8 @@ import { } from './traplib'; const makeWorkerTests = isHost => async t => { - const sab = new SharedArrayBuffer(2048); + // Ridiculously small shared array buffer to test continuations. + const sab = new SharedArrayBuffer(16); const worker = new Worker(`${__dirname}/worker.cjs`); worker.addListener('error', err => t.fail(err)); worker.postMessage({ type: 'TEST_INIT', sab, isGuest: isHost }); diff --git a/packages/captp/test/traplib.js b/packages/captp/test/traplib.js index 8ccb9209bfb..eab281a419e 100644 --- a/packages/captp/test/traplib.js +++ b/packages/captp/test/traplib.js @@ -80,9 +80,10 @@ const createGuestBootstrap = (Trap, other) => { }); }; -const SEM_WAITING = 0; -const SEM_READY = 2; const SEM_REJECT = 1; +const SEM_READY = 2; +const SEM_AGAIN = 4; +const SEM_WAITING = 8; const makeBufs = sab => { const sembuf = new Int32Array(sab, 0, 2); @@ -100,11 +101,30 @@ export const makeHost = (send, sab) => { { trapHost: ([isReject, ser]) => { // We need a bufferable message. - const data = JSON.stringify(ser); - const { written } = te.encodeInto(data, databuf); - sembuf[1] = written; - sembuf[0] = SEM_READY + (isReject ? SEM_REJECT : 0); - Atomics.notify(sembuf, 0, +Infinity); + const json = JSON.stringify(ser); + const encoded = te.encode(json); + let i = 0; + + // Send chunks in the data transfer buffer. + const sendChunk = () => { + const subenc = encoded.subarray(i, i + databuf.length); + databuf.set(subenc); + sembuf[1] = encoded.length - i; + i += subenc.length; + const done = i >= encoded.length; + sembuf[0] = + // eslint-disable-next-line no-bitwise + (done ? SEM_READY : SEM_AGAIN) | (isReject ? SEM_REJECT : 0); + Atomics.notify(sembuf, 0, +Infinity); + + if (done) { + // All done! + return undefined; + } + + return sendChunk; + }; + return sendChunk(); }, }, ); @@ -114,24 +134,37 @@ export const makeHost = (send, sab) => { export const makeGuest = (send, sab) => { const { sembuf, databuf } = makeBufs(sab); - const td = new TextDecoder('utf-8'); const { dispatch, getBootstrap, Trap } = makeCapTP( 'guest', send, () => createGuestBootstrap(Trap, getBootstrap()), { - trapGuest: ({ takeMore: trapSend }) => { + trapGuest: ({ takeMore }) => { + const td = new TextDecoder('utf-8'); + // Initialize the reply. - sembuf[0] = SEM_WAITING; - trapSend(); + const next = () => { + sembuf[0] = SEM_WAITING; + takeMore(); + // Wait for the reply to return. + Atomics.wait(sembuf, 0, SEM_WAITING); + }; - // Wait for the reply to return. - Atomics.wait(sembuf, 0, SEM_WAITING); + next(); + let json = ''; // eslint-disable-next-line no-bitwise - const isReject = !!(sembuf[0] & 1); - const data = td.decode(databuf.slice(0, sembuf[1])); - const ser = JSON.parse(data); + while (sembuf[0] & SEM_AGAIN) { + json += td.decode(databuf.subarray(0, sembuf[1]), { stream: true }); + next(); + } + + json += td.decode(databuf.subarray(0, sembuf[1])); + + // eslint-disable-next-line no-bitwise + const isReject = !!(sembuf[0] & SEM_REJECT); + + const ser = JSON.parse(json); return [isReject, ser]; }, }, From 67cc4cb7f1ea4698cc77b3bf9954cad831330468 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Sun, 11 Jul 2021 19:55:31 -0600 Subject: [PATCH 18/31] chore(captp): fix the lint --- packages/captp/src/captp.js | 5 ++ packages/captp/src/trap-driver.js | 81 ------------------------------- packages/captp/src/types.js | 5 -- 3 files changed, 5 insertions(+), 86 deletions(-) delete mode 100644 packages/captp/src/trap-driver.js diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index 1d824d6008e..6f0a2f108cd 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -20,6 +20,11 @@ import './types.js'; export { E }; +/** + * @template T + * @typedef {import('@agoric/eventual-send').ERef} ERef + */ + /** * @param {any} maybeThenable * @returns {boolean} diff --git a/packages/captp/src/trap-driver.js b/packages/captp/src/trap-driver.js deleted file mode 100644 index 251ed4e3232..00000000000 --- a/packages/captp/src/trap-driver.js +++ /dev/null @@ -1,81 +0,0 @@ -// @ts-check - -/** - * This file drives the trap takeReply and giveReply iterators. - * - * On the Guest side, the takeIt is given next(true). If it returns, that's the - * end of the cycle. - * - * If it yields, then the initTrap function is called. For all subsequent - * iterations, it is given next(false). If it yields, the nextBuffer function is - * called with buffer iteration parameters. - * - * On the Host side, sendTrapReply should be called when the answer from the - * initTrap step has settled. Calling the trapNextBuffer function with the - * trapID and supplied buffer iteration parameters will step through the giveIt - * to transfer buffers to the takeIt. - */ - -import { assert, details as X } from '@agoric/assert'; - -import './types'; -import '@agoric/marshal/exported'; - -// Something to make our reply sequences more interesting. -const TRAP_FIRST_SEQUENCE_NUMBER = 23; - -/** - * Create the host side of the trap interface. - */ -export const makeTrapHost = () => { - /** @type {Map void>} */ - const trapIDToNextBuffer = new Map(); - - return { - /** - * @param {any} trapID - * @param {ReturnType} giveIt - */ - sendTrapReply: async (trapID, giveIt) => { - assert( - !trapIDToNextBuffer.has(trapID), - X`Trap ID ${trapID} is already in progress`, - ); - - // Allow the caller to iterate via `trapNextBuffer` messages. - let nextSeq = TRAP_FIRST_SEQUENCE_NUMBER; - let pendingP; - /** - * @param {number} seq - * @param {any} data - */ - const nextInSequence = async (seq, data) => { - // Prevent parallel requests for a given sequence number. - await pendingP; - assert.equal( - seq, - nextSeq, - X`Invalid trapNextBuffer; seq ${seq} is not ${nextSeq}`, - ); - nextSeq += 1; - pendingP = giveIt.next(data); - const status = await pendingP; - if (status.done) { - // No more "nexts" are required, so clean up. - trapIDToNextBuffer.delete(trapID); - } - }; - trapIDToNextBuffer.set(trapID, nextInSequence); - - // Prime the pump with the first iteration. - await nextInSequence(nextSeq, undefined); - }, - // Send the next part of a "trap" call's result. - trapNextBuffer: async (trapID, trapParams) => { - const { data, seq } = trapParams; - const nextInSequence = trapIDToNextBuffer.get(trapID); - assert(nextInSequence); - return nextInSequence(seq, data); - }, - }; -}; diff --git a/packages/captp/src/types.js b/packages/captp/src/types.js index 7d76442e1b6..acdbbc306d5 100644 --- a/packages/captp/src/types.js +++ b/packages/captp/src/types.js @@ -1,10 +1,5 @@ // @ts-check -/** - * @template T - * @typedef {import('@agoric/eventual-send').ERef} ERef - */ - /** @typedef {string} CapTPSlot */ /** From a8e0e965f7640dc1a1e75b15d4788916e9cd563e Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Wed, 14 Jul 2021 22:54:45 -0600 Subject: [PATCH 19/31] feat(captp): take suggestion in #3289 to prefix questionIDs --- packages/captp/src/captp.js | 14 +++++++------- packages/captp/test/test-disco.js | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index 6f0a2f108cd..987ec0319be 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -130,17 +130,17 @@ export const makeCapTP = ( const exportedTrapHandlers = new WeakSet(); // Used to construct slot names for promises/non-promises. - // In this verison of CapTP we use strings for export/import slot names. + // In this version of CapTP we use strings for export/import slot names. // prefixed with 'p' if promises and 'o' otherwise; let lastPromiseID = 0; let lastExportID = 0; - // Since we decide the numbers for questions, we use this to increment - // the question key + // Since we decide the ids for questions, we use this to increment the + // question key let lastQuestionID = 0; - /** @type {Map} */ + /** @type {Map} */ const questions = new Map(); // chosen by us - /** @type {Map} */ + /** @type {Map} */ const answers = new Map(); // chosen by our peer /** @type {Map} */ const imports = new Map(); // chosen by our peer @@ -212,11 +212,11 @@ export const makeCapTP = ( * Generate a new question in the questions table and set up a new * remote handled promise. * - * @returns {[number, ReturnType]} + * @returns {[string, ReturnType]} */ const makeQuestion = () => { lastQuestionID += 1; - const questionID = lastQuestionID; + const questionID = `${ourId}#${lastQuestionID}`; // eslint-disable-next-line no-use-before-define const pr = makeRemoteKit(questionID); questions.set(questionID, pr); diff --git a/packages/captp/test/test-disco.js b/packages/captp/test/test-disco.js index e3328206ac9..3cb033dd0c3 100644 --- a/packages/captp/test/test-disco.js +++ b/packages/captp/test/test-disco.js @@ -39,7 +39,7 @@ test('try disconnecting captp', async t => { ); t.deepEqual( objs, - [{ type: 'CTP_BOOTSTRAP', questionID: 1, epoch: 0 }], + [{ type: 'CTP_BOOTSTRAP', questionID: 'us#1', epoch: 0 }], 'expected bootstrap messages', ); ps.push( @@ -54,7 +54,7 @@ test('try disconnecting captp', async t => { t.deepEqual( objs, [ - { type: 'CTP_BOOTSTRAP', questionID: 1, epoch: 0 }, + { type: 'CTP_BOOTSTRAP', questionID: 'us#1', epoch: 0 }, { type: 'CTP_DISCONNECT', reason: undefined, epoch: 0 }, ], 'expected clean disconnect', @@ -98,7 +98,7 @@ test('try aborting captp with reason', async t => { ); t.deepEqual( objs, - [{ type: 'CTP_BOOTSTRAP', questionID: 1, epoch: 0 }], + [{ type: 'CTP_BOOTSTRAP', questionID: 'us#1', epoch: 0 }], 'expected bootstrap messages', ); ps.push( @@ -117,7 +117,7 @@ test('try aborting captp with reason', async t => { ); t.deepEqual( objs, - [{ type: 'CTP_BOOTSTRAP', questionID: 1, epoch: 0 }, aborted], + [{ type: 'CTP_BOOTSTRAP', questionID: 'us#1', epoch: 0 }, aborted], 'expected unclean disconnect', ); await Promise.all(ps); From a350b9d4688bd156655e519dec9fe291b7353427 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Thu, 15 Jul 2021 11:12:30 -0600 Subject: [PATCH 20/31] feat(captp): leverage makeSubscriptionKit to drive trapHost --- packages/captp/package.json | 1 + packages/captp/src/captp.js | 114 ++++++++++++++++++++------------- packages/captp/src/types.js | 23 ++----- packages/captp/test/traplib.js | 55 ++++++++-------- 4 files changed, 102 insertions(+), 91 deletions(-) diff --git a/packages/captp/package.json b/packages/captp/package.json index 2ec6b7b85b2..de2ef045e05 100644 --- a/packages/captp/package.json +++ b/packages/captp/package.json @@ -46,6 +46,7 @@ "@agoric/eventual-send": "^0.13.22", "@agoric/marshal": "^0.4.19", "@agoric/nat": "^4.1.0", + "@agoric/notifier": "^0.3.22", "@agoric/promise-kit": "^0.2.20", "esm": "agoric-labs/esm#Agoric-built" }, diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index 987ec0319be..5979399cabd 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -11,6 +11,7 @@ import { QCLASS, } from '@agoric/marshal'; import { E, HandledPromise } from '@agoric/eventual-send'; +import { makeSubscriptionKit, observeIteration } from '@agoric/notifier'; import { isPromise } from '@agoric/promise-kit'; import { assert, details as X } from '@agoric/assert'; @@ -74,7 +75,8 @@ export const makeCapTP = ( const disconnectReason = id => Error(`${JSON.stringify(id)} connection closed`); - const trapGiveMore = new Map(); + /** @type {Map>} */ + const trapPublishers = new Map(); /** @type {any} */ let unplug = false; @@ -400,17 +402,34 @@ export const makeCapTP = ( 'function', X`CapTP cannot answer Trap(x) without a trapHost function`, ); + + // We need to create the publication right now to prevent a race with + // the other side. + const { subscription, publication } = makeSubscriptionKit(); + trapPublishers.set(questionID, publication); + sendReturn = async (isReject, value) => { const serialized = serialize(harden(value)); - const giveMore = trapHost([isReject, serialized]); - if (giveMore) { - assert.typeof( - giveMore, - 'function', - X`CapTP trapHost.reply result can only be a giveMore function`, - ); - trapGiveMore.set(questionID, giveMore); + const it = trapHost([isReject, serialized]); + if (!it) { + trapPublishers.delete(questionID); + return; } + + // Drive the trapHost async iterator via the subscription. + observeIteration(subscription, { + updateState(nonFinalValue) { + it.next(nonFinalValue); + }, + finish(completion) { + trapPublishers.delete(questionID); + it.return && it.return(completion); + }, + fail(reason) { + trapPublishers.delete(questionID); + it.throw && it.throw(reason); + }, + }).catch(e => console.error('error observing', e)); }; } catch (e) { sendReturn(true, e); @@ -440,23 +459,26 @@ export const makeCapTP = ( .catch(rej => quietReject(rej, false)); }, // Have the host serve more of the reply. - CTP_TRAP_TAKE_MORE: async obj => { - assert(trapHost, X`CTP_TRAP_TAKE_MORE is impossible without a trapHost`); - const giveMore = trapGiveMore.get(obj.questionID); - trapGiveMore.delete(obj.questionID); - assert.typeof( - giveMore, - 'function', - X`CTP_TRAP_TAKE_MORE did not expect ${obj.questionID}`, - ); - const nextGiveMore = giveMore(obj.data); - if (nextGiveMore) { - assert.typeof( - nextGiveMore, - 'function', - X`CapTP giveMore result can only be another giveMore function`, - ); - trapGiveMore.set(obj.questionID, nextGiveMore); + CTP_TRAP_ITERATE: async obj => { + assert(trapHost, X`CTP_TRAP_ITERATE is impossible without a trapHost`); + const { questionID, serialized } = obj; + const pub = trapPublishers.get(questionID); + assert(pub, X`CTP_TRAP_ITERATE did not expect ${questionID}`); + const [method, args] = unserialize(serialized); + try { + switch (method) { + case 'updateState': + case 'finish': + case 'fail': { + pub[method](...args); + break; + } + default: { + assert.fail(X`Unexpected CTP_TRAP_ITERATE method ${method}`); + } + } + } catch (e) { + pub.fail(e); } }, // Answer to one of our questions. @@ -587,7 +609,7 @@ export const makeCapTP = ( // Send a "trap" message. lastQuestionID += 1; - const questionID = lastQuestionID; + const questionID = `${ourId}#${lastQuestionID}`; // Encode the "method" parameter of the CTP_CALL. let method; @@ -614,31 +636,35 @@ export const makeCapTP = ( // Set up the trap call with its identifying information and a way to send // messages over the current CapTP data channel. - let isFirst = true; const [isException, serialized] = trapGuest({ implMethod, slot, implArgs, - takeMore: data => { - if (isFirst) { - // Send the call metadata over the connection. - isFirst = false; - send({ - type: 'CTP_CALL', - epoch, - trap: true, // This is the magic marker. - questionID, - target: slot, - method, - }); - } else { + trapToHost: () => { + // Send the call metadata over the connection. + send({ + type: 'CTP_CALL', + epoch, + trap: true, // This is the magic marker. + questionID, + target: slot, + method, + }); + + // Return an IterationObserver. + const makeObserverMethod = observerMethod => (...args) => { send({ - type: 'CTP_TRAP_TAKE_MORE', + type: 'CTP_TRAP_ITERATE', epoch, questionID, - data, + serialized: serialize(harden([observerMethod, args])), }); - } + }; + return harden({ + updateState: makeObserverMethod('updateState'), + fail: makeObserverMethod('fail'), + finish: makeObserverMethod('finish'), + }); }, }); diff --git a/packages/captp/src/types.js b/packages/captp/src/types.js index acdbbc306d5..2f09838cd07 100644 --- a/packages/captp/src/types.js +++ b/packages/captp/src/types.js @@ -26,9 +26,8 @@ * @property {keyof TrapImpl} implMethod the TrapImpl method that was called * @property {CapTPSlot} slot the target slot * @property {Array} implArgs arguments to the TrapImpl method - * @property {(data?: any) => void} takeMore send some data over the existing - * CapTP data channel for the trapHost to receive and supply us with more of the - * synchronous result + * @property {() => IterationObserver} trapToHost start the trap process on + * the trapHost, and drive the other side. */ /** @@ -43,22 +42,8 @@ * @callback TrapHost start the process of transferring the Trap request's * results * @param {TrapCompletion} completion - * @returns {undefined | ((data: any) => void)} If a function is returned, it will - * satisfy a future `takeMore`. - */ - -/** - * @callback GiveTrapReply Return an AsyncGenerator which is synchronously - * iterated by TakeTrapReply's generator to signal readiness of parts of the - * reply (there may be only one). These two generators must be written to - * cooperate over a specific CapTP connection, and via any out-of-band - * mechanisms as well. - * - * @param {TrapCompletion[0]} isReject whether the reply to communicate was a - * rejection or a regular return - * @param {TrapCompletion[1]} serialized the marshal-serialized data to be - * communicated to the other side. Note that the serialized data is JSONable. - * @returns {AsyncGenerator} + * @returns {AsyncIterator | undefined} If an AsyncIterator is returned, it + * will satisfy a future guest IterationObserver. */ /** @typedef {import('./ts-types').Trap} Trap */ diff --git a/packages/captp/test/traplib.js b/packages/captp/test/traplib.js index eab281a419e..4aa868a3cc3 100644 --- a/packages/captp/test/traplib.js +++ b/packages/captp/test/traplib.js @@ -81,7 +81,7 @@ const createGuestBootstrap = (Trap, other) => { }; const SEM_REJECT = 1; -const SEM_READY = 2; +const SEM_DONE = 2; const SEM_AGAIN = 4; const SEM_WAITING = 8; @@ -99,32 +99,30 @@ export const makeHost = (send, sab) => { send, () => createHostBootstrap(makeTrapHandler), { - trapHost: ([isReject, ser]) => { - // We need a bufferable message. + async *trapHost([isReject, ser]) { + // Get the complete encoded message buffer. const json = JSON.stringify(ser); const encoded = te.encode(json); - let i = 0; // Send chunks in the data transfer buffer. - const sendChunk = () => { + let i = 0; + let done = false; + while (!done) { const subenc = encoded.subarray(i, i + databuf.length); databuf.set(subenc); sembuf[1] = encoded.length - i; + i += subenc.length; - const done = i >= encoded.length; + done = i >= encoded.length; + sembuf[0] = // eslint-disable-next-line no-bitwise - (done ? SEM_READY : SEM_AGAIN) | (isReject ? SEM_REJECT : 0); + (done ? SEM_DONE : SEM_AGAIN) | (isReject ? SEM_REJECT : 0); Atomics.notify(sembuf, 0, +Infinity); - if (done) { - // All done! - return undefined; - } - - return sendChunk; - }; - return sendChunk(); + // Wait until the next call. + yield; + } }, }, ); @@ -139,27 +137,28 @@ export const makeGuest = (send, sab) => { send, () => createGuestBootstrap(Trap, getBootstrap()), { - trapGuest: ({ takeMore }) => { + trapGuest: ({ trapToHost }) => { const td = new TextDecoder('utf-8'); - // Initialize the reply. - const next = () => { + let json = ''; + + // Start by sending the trap to the host. + const pub = trapToHost(); + + let done = false; + while (!done) { sembuf[0] = SEM_WAITING; - takeMore(); + pub.updateState(); + // Wait for the reply to return. Atomics.wait(sembuf, 0, SEM_WAITING); - }; - next(); - - let json = ''; - // eslint-disable-next-line no-bitwise - while (sembuf[0] & SEM_AGAIN) { - json += td.decode(databuf.subarray(0, sembuf[1]), { stream: true }); - next(); + // eslint-disable-next-line no-bitwise + done = (sembuf[0] & SEM_DONE) !== 0; + json += td.decode(databuf.subarray(0, sembuf[1]), { stream: !done }); } - json += td.decode(databuf.subarray(0, sembuf[1])); + pub.finish(); // eslint-disable-next-line no-bitwise const isReject = !!(sembuf[0] & SEM_REJECT); From 4aa941ca2f9ae869f3d544d926a30844a779a8bd Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Fri, 16 Jul 2021 17:17:18 -0600 Subject: [PATCH 21/31] refactor(captp): rename and simplify TrapCaps --- packages/captp/package.json | 1 - packages/captp/src/captp.js | 200 +++++++------------------- packages/captp/src/index.js | 1 + packages/captp/src/loopback.js | 95 ++++++++++++ packages/captp/src/trap.js | 16 +-- packages/captp/src/types.js | 6 +- packages/captp/test/test-crosstalk.js | 2 +- packages/captp/test/test-loopback.js | 2 +- packages/captp/test/test-trap.js | 3 +- packages/captp/test/traplib.js | 12 +- 10 files changed, 165 insertions(+), 173 deletions(-) create mode 100644 packages/captp/src/loopback.js diff --git a/packages/captp/package.json b/packages/captp/package.json index de2ef045e05..2ec6b7b85b2 100644 --- a/packages/captp/package.json +++ b/packages/captp/package.json @@ -46,7 +46,6 @@ "@agoric/eventual-send": "^0.13.22", "@agoric/marshal": "^0.4.19", "@agoric/nat": "^4.1.0", - "@agoric/notifier": "^0.3.22", "@agoric/promise-kit": "^0.2.20", "esm": "agoric-labs/esm#Agoric-built" }, diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index 5979399cabd..374c8a1eac7 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -4,18 +4,12 @@ // This logic was mostly lifted from @agoric/swingset-vat liveSlots.js // Defects in it are mfig's fault. -import { - Remotable as defaultRemotable, - Far as defaultFar, - makeMarshal as defaultMakeMarshal, - QCLASS, -} from '@agoric/marshal'; +import { Remotable, Far, makeMarshal, QCLASS } from '@agoric/marshal'; import { E, HandledPromise } from '@agoric/eventual-send'; -import { makeSubscriptionKit, observeIteration } from '@agoric/notifier'; -import { isPromise } from '@agoric/promise-kit'; +import { isPromise, makePromiseKit } from '@agoric/promise-kit'; import { assert, details as X } from '@agoric/assert'; -import { makeTrap, nearTrapImpl } from './trap.js'; +import { makeTrap } from './trap.js'; import './types.js'; @@ -36,9 +30,6 @@ const isThenable = maybeThenable => /** * @typedef {Object} CapTPOptions the options to makeCapTP * @property {(err: any) => void} onReject - * @property {typeof defaultRemotable} Remotable - * @property {typeof defaultFar} Far - * @property {typeof defaultMakeMarshal} makeMarshal * @property {number} epoch an integer tag to attach to all messages in order to * assist in ignoring earlier defunct instance's messages * @property {TrapGuest} trapGuest if specified, enable this CapTP (guest) to @@ -64,9 +55,6 @@ export const makeCapTP = ( ) => { const { onReject = err => console.error('CapTP', ourId, 'exception:', err), - Remotable = defaultRemotable, - makeMarshal = defaultMakeMarshal, - Far = defaultFar, epoch = 0, trapGuest, trapHost, @@ -75,8 +63,10 @@ export const makeCapTP = ( const disconnectReason = id => Error(`${JSON.stringify(id)} connection closed`); - /** @type {Map>} */ - const trapPublishers = new Map(); + /** @type {Map>>} */ + const trapIteratorResultP = new Map(); + /** @type {Map>} */ + const trapIterator = new Map(); /** @type {any} */ let unplug = false; @@ -403,33 +393,23 @@ export const makeCapTP = ( X`CapTP cannot answer Trap(x) without a trapHost function`, ); - // We need to create the publication right now to prevent a race with - // the other side. - const { subscription, publication } = makeSubscriptionKit(); - trapPublishers.set(questionID, publication); + // We need to create a promise for the "isDone" iteration right now to + // prevent a race with the other side. + const resultPK = makePromiseKit(); + trapIteratorResultP.set(questionID, resultPK.promise); sendReturn = async (isReject, value) => { const serialized = serialize(harden(value)); - const it = trapHost([isReject, serialized]); - if (!it) { - trapPublishers.delete(questionID); + const ait = trapHost([isReject, serialized]); + if (!ait) { + // One-shot, no async iterator. + resultPK.resolve({ done: true }); return; } - // Drive the trapHost async iterator via the subscription. - observeIteration(subscription, { - updateState(nonFinalValue) { - it.next(nonFinalValue); - }, - finish(completion) { - trapPublishers.delete(questionID); - it.return && it.return(completion); - }, - fail(reason) { - trapPublishers.delete(questionID); - it.throw && it.throw(reason); - }, - }).catch(e => console.error('error observing', e)); + // We're ready for them to drive the iterator. + trapIterator.set(questionID, ait); + resultPK.resolve({ done: false }); }; } catch (e) { sendReturn(true, e); @@ -462,23 +442,36 @@ export const makeCapTP = ( CTP_TRAP_ITERATE: async obj => { assert(trapHost, X`CTP_TRAP_ITERATE is impossible without a trapHost`); const { questionID, serialized } = obj; - const pub = trapPublishers.get(questionID); - assert(pub, X`CTP_TRAP_ITERATE did not expect ${questionID}`); + + const resultP = trapIteratorResultP.get(questionID); + assert(resultP, X`CTP_TRAP_ITERATE did not expect ${questionID}`); + const [method, args] = unserialize(serialized); + + const resultPK = makePromiseKit(); + trapIteratorResultP.set(questionID, resultPK.promise); + + const { done } = await resultP; + if (done) { + trapIterator.delete(questionID); + return; + } + const ait = trapIterator.get(questionID); + try { switch (method) { - case 'updateState': - case 'finish': - case 'fail': { - pub[method](...args); + case 'next': + case 'return': + case 'throw': { + resultPK.resolve(ait && ait[method] && ait[method](...args)); break; } default: { - assert.fail(X`Unexpected CTP_TRAP_ITERATE method ${method}`); + assert.fail(X`Unrecognized iteration method ${method}`); } } } catch (e) { - pub.fail(e); + resultPK.reject(e); } }, // Answer to one of our questions. @@ -637,10 +630,10 @@ export const makeCapTP = ( // Set up the trap call with its identifying information and a way to send // messages over the current CapTP data channel. const [isException, serialized] = trapGuest({ - implMethod, + trapMethod: implMethod, slot, - implArgs, - trapToHost: () => { + trapArgs: implArgs, + startTrap: () => { // Send the call metadata over the connection. send({ type: 'CTP_CALL', @@ -652,18 +645,19 @@ export const makeCapTP = ( }); // Return an IterationObserver. - const makeObserverMethod = observerMethod => (...args) => { + const makeIteratorMethod = (iteratorMethod, done) => (...args) => { send({ type: 'CTP_TRAP_ITERATE', epoch, questionID, - serialized: serialize(harden([observerMethod, args])), + serialized: serialize(harden([iteratorMethod, args])), }); + return { done, value: undefined }; }; return harden({ - updateState: makeObserverMethod('updateState'), - fail: makeObserverMethod('fail'), - finish: makeObserverMethod('finish'), + next: makeIteratorMethod('next', false), + return: makeIteratorMethod('return', true), + throw: makeIteratorMethod('throw', true), }); }, }); @@ -693,101 +687,3 @@ export const makeCapTP = ( return harden(rets); }; - -/** - * @typedef {Object} LoopbackOpts - * @property {typeof defaultFar} Far - */ - -/** - * Create an async-isolated channel to an object. - * - * @param {string} ourId - * @param {Partial} [opts] - * @returns {{ - * makeFar(x: T): ERef, - * makeNear(x: T): ERef, - * makeTrapHandler(x: T): T, - * Trap: Trap - * }} - */ -export const makeLoopback = (ourId, opts = {}) => { - const { Far = defaultFar } = opts; - let nextNonce = 0; - const nonceToRef = new Map(); - - const bootstrap = harden({ - refGetter: Far('refGetter', { - getRef(nonce) { - // Find the local ref for the specified nonce. - const xFar = nonceToRef.get(nonce); - nonceToRef.delete(nonce); - return xFar; - }, - }), - }); - - const slotBody = JSON.stringify({ - '@qclass': 'slot', - index: 0, - }); - - // Create the tunnel. - let farDispatch; - const { - Trap, - dispatch: nearDispatch, - getBootstrap: getFarBootstrap, - } = makeCapTP(`near-${ourId}`, o => farDispatch(o), bootstrap, { - trapGuest: ({ implMethod, slot, implArgs }) => { - let value; - let isException = false; - try { - // Cross the boundary to pull out the far object. - // eslint-disable-next-line no-use-before-define - const far = farUnserialize({ body: slotBody, slots: [slot] }); - value = nearTrapImpl[implMethod](far, implArgs[0], implArgs[1]); - } catch (e) { - isException = true; - value = e; - } - harden(value); - // eslint-disable-next-line no-use-before-define - return [isException, farSerialize(value)]; - }, - }); - assert(Trap); - - const { - makeTrapHandler, - dispatch, - getBootstrap: getNearBootstrap, - unserialize: farUnserialize, - serialize: farSerialize, - } = makeCapTP(`far-${ourId}`, nearDispatch, bootstrap); - farDispatch = dispatch; - - const farGetter = E.get(getFarBootstrap()).refGetter; - const nearGetter = E.get(getNearBootstrap()).refGetter; - - /** - * @param {ERef<{ getRef(nonce: number): any }>} refGetter - */ - const makeRefMaker = refGetter => - /** - * @param {any} x - */ - async x => { - const myNonce = nextNonce; - nextNonce += 1; - nonceToRef.set(myNonce, harden(x)); - return E(refGetter).getRef(myNonce); - }; - - return { - makeFar: makeRefMaker(farGetter), - makeNear: makeRefMaker(nearGetter), - makeTrapHandler, - Trap, - }; -}; diff --git a/packages/captp/src/index.js b/packages/captp/src/index.js index dac09a2122d..b9b0031f53a 100644 --- a/packages/captp/src/index.js +++ b/packages/captp/src/index.js @@ -3,3 +3,4 @@ export { Nat } from '@agoric/nat'; export * from '@agoric/marshal'; export * from './captp.js'; +export * from './loopback.js'; diff --git a/packages/captp/src/loopback.js b/packages/captp/src/loopback.js new file mode 100644 index 00000000000..ca308fbfb7f --- /dev/null +++ b/packages/captp/src/loopback.js @@ -0,0 +1,95 @@ +import { Far } from '@agoric/marshal'; +import { E, makeCapTP } from './captp.js'; +import { nearTrapImpl } from './trap.js'; + +export { E }; + +/** + * Create an async-isolated channel to an object. + * + * @param {string} ourId + * @returns {{ + * makeFar(x: T): ERef, + * makeNear(x: T): ERef, + * makeTrapHandler(x: T): T, + * Trap: Trap + * }} + */ +export const makeLoopback = ourId => { + let nextNonce = 0; + const nonceToRef = new Map(); + + const bootstrap = harden({ + refGetter: Far('refGetter', { + getRef(nonce) { + // Find the local ref for the specified nonce. + const xFar = nonceToRef.get(nonce); + nonceToRef.delete(nonce); + return xFar; + }, + }), + }); + + const slotBody = JSON.stringify({ + '@qclass': 'slot', + index: 0, + }); + + // Create the tunnel. + const { + Trap, + dispatch: nearDispatch, + getBootstrap: getFarBootstrap, + // eslint-disable-next-line no-use-before-define + } = makeCapTP(`near-${ourId}`, o => farDispatch(o), bootstrap, { + trapGuest: ({ trapMethod, slot, trapArgs }) => { + let value; + let isException = false; + try { + // Cross the boundary to pull out the far object. + // eslint-disable-next-line no-use-before-define + const far = farUnserialize({ body: slotBody, slots: [slot] }); + value = nearTrapImpl[trapMethod](far, trapArgs[0], trapArgs[1]); + } catch (e) { + isException = true; + value = e; + } + harden(value); + // eslint-disable-next-line no-use-before-define + return [isException, farSerialize(value)]; + }, + }); + assert(Trap); + + const { + makeTrapHandler, + dispatch: farDispatch, + getBootstrap: getNearBootstrap, + unserialize: farUnserialize, + serialize: farSerialize, + } = makeCapTP(`far-${ourId}`, nearDispatch, bootstrap); + + const farGetter = E.get(getFarBootstrap()).refGetter; + const nearGetter = E.get(getNearBootstrap()).refGetter; + + /** + * @param {ERef<{ getRef(nonce: number): any }>} refGetter + */ + const makeRefMaker = refGetter => + /** + * @param {any} x + */ + async x => { + const myNonce = nextNonce; + nextNonce += 1; + nonceToRef.set(myNonce, harden(x)); + return E(refGetter).getRef(myNonce); + }; + + return { + makeFar: makeRefMaker(farGetter), + makeNear: makeRefMaker(nearGetter), + makeTrapHandler, + Trap, + }; +}; diff --git a/packages/captp/src/trap.js b/packages/captp/src/trap.js index df03b71ce7d..52db67027ab 100644 --- a/packages/captp/src/trap.js +++ b/packages/captp/src/trap.js @@ -39,17 +39,17 @@ const readOnlyProxyHandler = { * A Proxy handler for Trap(x) * * @param {*} x Any value passed to Trap(x) - * @param {TrapImpl} syncImpl + * @param {TrapImpl} trapImpl * @returns {ProxyHandler} */ -const TrapProxyHandler = (x, syncImpl) => { +const TrapProxyHandler = (x, trapImpl) => { return harden({ ...readOnlyProxyHandler, get(_target, p, _receiver) { - return (...args) => syncImpl.applyMethod(x, p, args); + return (...args) => trapImpl.applyMethod(x, p, args); }, apply(_target, _thisArg, argArray = []) { - return syncImpl.applyFunction(x, argArray); + return trapImpl.applyFunction(x, argArray); }, has(_target, _p) { // TODO: has property is not yet transferrable over captp. @@ -59,12 +59,12 @@ const TrapProxyHandler = (x, syncImpl) => { }; /** - * @param {TrapImpl} syncImpl + * @param {TrapImpl} trapImpl * @returns {Trap} */ -export const makeTrap = syncImpl => { +export const makeTrap = trapImpl => { const Trap = x => { - const handler = TrapProxyHandler(x, syncImpl); + const handler = TrapProxyHandler(x, trapImpl); return harden(new Proxy(() => {}, handler)); }; @@ -76,7 +76,7 @@ export const makeTrap = syncImpl => { return true; }, get(_target, prop) { - return syncImpl.get(x, prop); + return trapImpl.get(x, prop); }, }); return new Proxy(Object.create(null), handler); diff --git a/packages/captp/src/types.js b/packages/captp/src/types.js index 2f09838cd07..7c76d1e9702 100644 --- a/packages/captp/src/types.js +++ b/packages/captp/src/types.js @@ -23,10 +23,10 @@ /** * @typedef TrapRequest the argument to TrapGuest - * @property {keyof TrapImpl} implMethod the TrapImpl method that was called + * @property {keyof TrapImpl} trapMethod the TrapImpl method that was called * @property {CapTPSlot} slot the target slot - * @property {Array} implArgs arguments to the TrapImpl method - * @property {() => IterationObserver} trapToHost start the trap process on + * @property {Array} trapArgs arguments to the TrapImpl method + * @property {() => Iterator} startTrap start the trap process on * the trapHost, and drive the other side. */ diff --git a/packages/captp/test/test-crosstalk.js b/packages/captp/test/test-crosstalk.js index 7749c4689cb..324b772fced 100644 --- a/packages/captp/test/test-crosstalk.js +++ b/packages/captp/test/test-crosstalk.js @@ -1,7 +1,7 @@ import '@agoric/install-ses'; import { Far } from '@agoric/marshal'; import test from 'ava'; -import { makeLoopback, E } from '../src/captp.js'; +import { makeLoopback, E } from '../src/loopback.js'; test('prevent crosstalk', async t => { const { makeFar } = makeLoopback('alice'); diff --git a/packages/captp/test/test-loopback.js b/packages/captp/test/test-loopback.js index f30076b0e2e..ce1ce714d27 100644 --- a/packages/captp/test/test-loopback.js +++ b/packages/captp/test/test-loopback.js @@ -2,7 +2,7 @@ import '@agoric/install-ses'; import { Far } from '@agoric/marshal'; import test from 'ava'; -import { E, makeLoopback } from '../src/captp.js'; +import { E, makeLoopback } from '../src/loopback.js'; test('try loopback captp', async t => { const pr = {}; diff --git a/packages/captp/test/test-trap.js b/packages/captp/test/test-trap.js index 782637e16c7..14f20927196 100644 --- a/packages/captp/test/test-trap.js +++ b/packages/captp/test/test-trap.js @@ -3,7 +3,8 @@ import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava'; import { Worker } from 'worker_threads'; -import { E, makeLoopback } from '../src/captp'; +import { E, makeLoopback } from '../src/loopback'; + import { createHostBootstrap, makeGuest, diff --git a/packages/captp/test/traplib.js b/packages/captp/test/traplib.js index 4aa868a3cc3..ebb5946e350 100644 --- a/packages/captp/test/traplib.js +++ b/packages/captp/test/traplib.js @@ -99,9 +99,9 @@ export const makeHost = (send, sab) => { send, () => createHostBootstrap(makeTrapHandler), { - async *trapHost([isReject, ser]) { + async *trapHost([isReject, serialized]) { // Get the complete encoded message buffer. - const json = JSON.stringify(ser); + const json = JSON.stringify(serialized); const encoded = te.encode(json); // Send chunks in the data transfer buffer. @@ -137,18 +137,18 @@ export const makeGuest = (send, sab) => { send, () => createGuestBootstrap(Trap, getBootstrap()), { - trapGuest: ({ trapToHost }) => { + trapGuest: ({ startTrap }) => { const td = new TextDecoder('utf-8'); let json = ''; // Start by sending the trap to the host. - const pub = trapToHost(); + const it = startTrap(); let done = false; while (!done) { sembuf[0] = SEM_WAITING; - pub.updateState(); + it.next(); // Wait for the reply to return. Atomics.wait(sembuf, 0, SEM_WAITING); @@ -158,7 +158,7 @@ export const makeGuest = (send, sab) => { json += td.decode(databuf.subarray(0, sembuf[1]), { stream: !done }); } - pub.finish(); + it.return(); // eslint-disable-next-line no-bitwise const isReject = !!(sembuf[0] & SEM_REJECT); From ec5dfbe047a2afd68c285ba5cc69b3757c617ef3 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Fri, 16 Jul 2021 19:13:25 -0600 Subject: [PATCH 22/31] refactor(captp): clarify the Atomics trapGuest/trapHost protocol --- packages/captp/src/atomics.js | 149 +++++++++++++++++++++++++++++++ packages/captp/src/index.js | 3 +- packages/captp/src/types.js | 2 +- packages/captp/test/test-trap.js | 18 ++-- packages/captp/test/traplib.js | 74 ++------------- packages/captp/test/worker.js | 6 +- 6 files changed, 174 insertions(+), 78 deletions(-) create mode 100644 packages/captp/src/atomics.js diff --git a/packages/captp/src/atomics.js b/packages/captp/src/atomics.js new file mode 100644 index 00000000000..1eca551e3d7 --- /dev/null +++ b/packages/captp/src/atomics.js @@ -0,0 +1,149 @@ +// @ts-check +/* global BigUint64Array */ + +import { assert, details as X } from '@agoric/assert'; + +// This is a pathological minimum, but exercised by the unit test. +// transfer. +export const MIN_DATA_BUFFER_LENGTH = 1; + +// Calculate how big the transfer buffer needs to be. +export const TRANSFER_OVERHEAD_LENGTH = + BigUint64Array.BYTES_PER_ELEMENT + Int32Array.BYTES_PER_ELEMENT; +export const MIN_TRANSFER_BUFFER_LENGTH = + MIN_DATA_BUFFER_LENGTH + TRANSFER_OVERHEAD_LENGTH; + +// These are bit flags for the status element of the transfer buffer. +const STATUS_WAITING = 1; +const STATUS_FLAG_DONE = 2; +const STATUS_FLAG_REJECT = 4; + +/** + * Return a status buffer, length buffer, and data buffer backed by transferBuffer. + * + * @param {SharedArrayBuffer} transferBuffer the backing buffer + */ +const splitTransferBuffer = transferBuffer => { + assert( + transferBuffer.byteLength >= MIN_TRANSFER_BUFFER_LENGTH, + X`Transfer buffer of ${transferBuffer.byteLength} bytes is smaller than MIN_TRANSFER_BUFFER_LENGTH ${MIN_TRANSFER_BUFFER_LENGTH}`, + ); + const lenbuf = new BigUint64Array(transferBuffer, 0, 1); + const statusbuf = new Int32Array(transferBuffer, lenbuf.byteLength, 1); + const overheadLength = lenbuf.byteLength + statusbuf.byteLength; + assert.equal( + overheadLength, + TRANSFER_OVERHEAD_LENGTH, + X`Internal error; actual overhead ${overheadLength} of bytes is not TRANSFER_OVERHEAD_LENGTH ${TRANSFER_OVERHEAD_LENGTH}`, + ); + const databuf = new Uint8Array( + transferBuffer, + lenbuf.byteLength + statusbuf.byteLength, + ); + assert( + databuf.byteLength >= MIN_DATA_BUFFER_LENGTH, + X`Transfer buffer of size ${transferBuffer.byteLength} only supports ${databuf.byteLength} data bytes; need at least ${MIN_DATA_BUFFER_LENGTH}`, + ); + return { statusbuf, lenbuf, databuf }; +}; + +/** + * Create a trapHost that can be paired with makeAtomicsTrapGuest. + * + * This host encodes the transfer buffer and returns it in consecutive slices + * when the guest iterates over it. + * + * @param {SharedArrayBuffer} transferBuffer + * @returns {TrapHost} + */ +export const makeAtomicsTrapHost = transferBuffer => { + const { statusbuf, lenbuf, databuf } = splitTransferBuffer(transferBuffer); + + const te = new TextEncoder(); + + return async function* trapHost([isReject, serialized]) { + // Get the complete encoded message buffer. + const json = JSON.stringify(serialized); + const encoded = te.encode(json); + + // Send chunks in the data transfer buffer. + let i = 0; + let done = false; + while (!done) { + // Copy the next slice of the encoded arry to the data buffer. + const subenc = encoded.subarray(i, i + databuf.length); + databuf.set(subenc); + + // Save the length of the remaining data. + lenbuf[0] = BigInt(encoded.length - i); + + // Calculate the next slice, and whether this is the last one. + i += subenc.length; + done = i >= encoded.length; + + // Find bitflags to represent the rejected and finished state. + const rejectFlag = isReject ? STATUS_FLAG_REJECT : 0; + const doneFlag = done ? STATUS_FLAG_DONE : 0; + + // Notify our guest for this data buffer. + + // eslint-disable-next-line no-bitwise + statusbuf[0] = rejectFlag | doneFlag; + Atomics.notify(statusbuf, 0, +Infinity); + + // Wait until the next call. + yield; + } + }; +}; + +/** + * Create a trapGuest that can be paired with makeAtomicsTrapHost. + * + * This guest iterates through the consecutive slices of the JSON-encoded data, + * then returns it. + * + * @param {SharedArrayBuffer} transferBuffer + * @returns {TrapGuest} + */ +export const makeAtomicsTrapGuest = transferBuffer => { + const { statusbuf, lenbuf, databuf } = splitTransferBuffer(transferBuffer); + + return ({ startTrap }) => { + const td = new TextDecoder('utf-8'); + + let json = ''; + + // Start by sending the trap call to the host. + const it = startTrap(); + + let done = false; + while (!done) { + // Tell that we are ready for another buffer. + statusbuf[0] = STATUS_WAITING; + it.next(); + + // Wait for the host to wake us. + Atomics.wait(statusbuf, 0, STATUS_WAITING); + + // Determine whether this is the last buffer. + // eslint-disable-next-line no-bitwise + done = (statusbuf[0] & STATUS_FLAG_DONE) !== 0; + + // Decode the next part of the data buffer. + json += td.decode(databuf.subarray(0, Number(lenbuf[0])), { + stream: !done, + }); + } + + // Tell the host we're finished. + it.return(); + + // eslint-disable-next-line no-bitwise + const isReject = !!(statusbuf[0] & STATUS_FLAG_REJECT); + + // Parse the JSON data into marshalled form. + const serialized = JSON.parse(json); + return [isReject, serialized]; + }; +}; diff --git a/packages/captp/src/index.js b/packages/captp/src/index.js index b9b0031f53a..3eaf55e4828 100644 --- a/packages/captp/src/index.js +++ b/packages/captp/src/index.js @@ -3,4 +3,5 @@ export { Nat } from '@agoric/nat'; export * from '@agoric/marshal'; export * from './captp.js'; -export * from './loopback.js'; +export { makeLoopback } from './loopback.js'; +export * from './atomics.js'; diff --git a/packages/captp/src/types.js b/packages/captp/src/types.js index 7c76d1e9702..6d90357b30e 100644 --- a/packages/captp/src/types.js +++ b/packages/captp/src/types.js @@ -26,7 +26,7 @@ * @property {keyof TrapImpl} trapMethod the TrapImpl method that was called * @property {CapTPSlot} slot the target slot * @property {Array} trapArgs arguments to the TrapImpl method - * @property {() => Iterator} startTrap start the trap process on + * @property {() => Required>} startTrap start the trap process on * the trapHost, and drive the other side. */ diff --git a/packages/captp/test/test-trap.js b/packages/captp/test/test-trap.js index 14f20927196..22238b074c9 100644 --- a/packages/captp/test/test-trap.js +++ b/packages/captp/test/test-trap.js @@ -2,6 +2,7 @@ import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava'; import { Worker } from 'worker_threads'; +import { MIN_TRANSFER_BUFFER_LENGTH } from '../src/atomics.js'; import { E, makeLoopback } from '../src/loopback'; @@ -13,16 +14,23 @@ import { } from './traplib'; const makeWorkerTests = isHost => async t => { - // Ridiculously small shared array buffer to test continuations. - const sab = new SharedArrayBuffer(16); + const initFn = isHost ? makeHost : makeGuest; + for (let len = 0; len < MIN_TRANSFER_BUFFER_LENGTH; len += 1) { + t.throws(() => initFn(() => {}, new SharedArrayBuffer(len)), { + message: /^Transfer buffer/, + instanceOf: Error, + }); + } + + // Small shared array buffer to test iterator. + const transferBuffer = new SharedArrayBuffer(MIN_TRANSFER_BUFFER_LENGTH); const worker = new Worker(`${__dirname}/worker.cjs`); worker.addListener('error', err => t.fail(err)); - worker.postMessage({ type: 'TEST_INIT', sab, isGuest: isHost }); + worker.postMessage({ type: 'TEST_INIT', transferBuffer, isGuest: isHost }); - const initFn = isHost ? makeHost : makeGuest; const { dispatch, getBootstrap, Trap } = initFn( obj => worker.postMessage(obj), - sab, + transferBuffer, ); worker.addListener('message', obj => { diff --git a/packages/captp/test/traplib.js b/packages/captp/test/traplib.js index ebb5946e350..7a653b207b4 100644 --- a/packages/captp/test/traplib.js +++ b/packages/captp/test/traplib.js @@ -3,7 +3,9 @@ import { assert, details as X } from '@agoric/assert'; import { Far } from '@agoric/marshal'; -import { E, makeCapTP } from '../src/captp'; +import { E, makeCapTP } from '../src/captp.js'; + +import { makeAtomicsTrapGuest, makeAtomicsTrapHost } from '../src/atomics.js'; export const createHostBootstrap = makeTrapHandler => { // Create a remotable that has a syncable return value. @@ -80,50 +82,13 @@ const createGuestBootstrap = (Trap, other) => { }); }; -const SEM_REJECT = 1; -const SEM_DONE = 2; -const SEM_AGAIN = 4; -const SEM_WAITING = 8; - -const makeBufs = sab => { - const sembuf = new Int32Array(sab, 0, 2); - const databuf = new Uint8Array(sab, sembuf.byteLength); - return { sembuf, databuf }; -}; - export const makeHost = (send, sab) => { - const { sembuf, databuf } = makeBufs(sab); - const te = new TextEncoder(); const { dispatch, getBootstrap, makeTrapHandler } = makeCapTP( 'host', send, () => createHostBootstrap(makeTrapHandler), { - async *trapHost([isReject, serialized]) { - // Get the complete encoded message buffer. - const json = JSON.stringify(serialized); - const encoded = te.encode(json); - - // Send chunks in the data transfer buffer. - let i = 0; - let done = false; - while (!done) { - const subenc = encoded.subarray(i, i + databuf.length); - databuf.set(subenc); - sembuf[1] = encoded.length - i; - - i += subenc.length; - done = i >= encoded.length; - - sembuf[0] = - // eslint-disable-next-line no-bitwise - (done ? SEM_DONE : SEM_AGAIN) | (isReject ? SEM_REJECT : 0); - Atomics.notify(sembuf, 0, +Infinity); - - // Wait until the next call. - yield; - } - }, + trapHost: makeAtomicsTrapHost(sab), }, ); @@ -131,41 +96,12 @@ export const makeHost = (send, sab) => { }; export const makeGuest = (send, sab) => { - const { sembuf, databuf } = makeBufs(sab); const { dispatch, getBootstrap, Trap } = makeCapTP( 'guest', send, () => createGuestBootstrap(Trap, getBootstrap()), { - trapGuest: ({ startTrap }) => { - const td = new TextDecoder('utf-8'); - - let json = ''; - - // Start by sending the trap to the host. - const it = startTrap(); - - let done = false; - while (!done) { - sembuf[0] = SEM_WAITING; - it.next(); - - // Wait for the reply to return. - Atomics.wait(sembuf, 0, SEM_WAITING); - - // eslint-disable-next-line no-bitwise - done = (sembuf[0] & SEM_DONE) !== 0; - json += td.decode(databuf.subarray(0, sembuf[1]), { stream: !done }); - } - - it.return(); - - // eslint-disable-next-line no-bitwise - const isReject = !!(sembuf[0] & SEM_REJECT); - - const ser = JSON.parse(json); - return [isReject, ser]; - }, + trapGuest: makeAtomicsTrapGuest(sab), }, ); return { dispatch, getBootstrap, Trap }; diff --git a/packages/captp/test/worker.js b/packages/captp/test/worker.js index 623c43ab359..bb3200a1fbb 100644 --- a/packages/captp/test/worker.js +++ b/packages/captp/test/worker.js @@ -1,4 +1,5 @@ import '@agoric/install-ses'; +import { assert, details as X } from '@agoric/assert'; import { parentPort } from 'worker_threads'; import { makeGuest, makeHost } from './traplib'; @@ -7,9 +8,10 @@ let dispatch; parentPort.addListener('message', obj => { switch (obj.type) { case 'TEST_INIT': { - const { sab, isGuest } = obj; + assert(!dispatch, X`Internal error; duplicate initialization`); + const { transferBuffer, isGuest } = obj; const initFn = isGuest ? makeGuest : makeHost; - const ret = initFn(o => parentPort.postMessage(o), sab); + const ret = initFn(o => parentPort.postMessage(o), transferBuffer); dispatch = ret.dispatch; break; } From 1b60eb6f7d3688674f57e2430ddf5d4a83bf8776 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Fri, 16 Jul 2021 21:09:55 -0600 Subject: [PATCH 23/31] style(captp): clarify documentation and review changes --- packages/captp/NEWS.md | 4 ++++ packages/captp/README.md | 40 ++++++++++++++++++++++++++++++++ packages/captp/jsconfig.json | 2 +- packages/captp/src/captp.js | 7 +----- packages/captp/src/loopback.js | 5 ++++ packages/captp/src/ts-types.d.ts | 22 +++++++++++++----- packages/captp/src/types.js | 11 +++++---- 7 files changed, 74 insertions(+), 17 deletions(-) diff --git a/packages/captp/NEWS.md b/packages/captp/NEWS.md index 84fe77e73be..6bb069f9efd 100644 --- a/packages/captp/NEWS.md +++ b/packages/captp/NEWS.md @@ -7,3 +7,7 @@ User-visible changes in captp: Moved from https://github.com/Agoric/captp into the `packages/captp/` directory in the monorepo at https://github.com/Agoric/agoric-sdk . +## Release 1.8.0 + +* introduce TrapCaps for synchronous "kernel trap" interfaces (see the + README.md). diff --git a/packages/captp/README.md b/packages/captp/README.md index dabfda6eedf..23a8a0ef99b 100644 --- a/packages/captp/README.md +++ b/packages/captp/README.md @@ -23,3 +23,43 @@ E(getBootstrap()).method(args).then(res => console.log('got res', res)); // Tear down the CapTP connection if it fails (e.g. connection is closed). abort(Error('Connection aborted by user.')); ``` + +## Loopback + +The `makeLoopback()` function creates an async barrier between "near" and "far" +objects. This is useful for testing and isolation within the same address +space. + +## TrapCaps + +In addition to the normal CapTP facilities, this library also has the notion of +"TrapCaps", which enable a "guest" endpoint to call a "host" object (which may +resolve an answer promise at its convenience), but the guest synchronously +blocks until it receives the resolved answer. + +This is a specialized and advanced use case, not for mutually-suspicious CapTP +parties, but instead for clear "guest"/"host" relationship, such as user-space +code and synchronous devices. + +1. Supply the `trapHost` and `trapGuest` protocol implementation (such as the + one based on `SharedArrayBuffers` in `src/atomics.js`) to the host and guest + `makeCapTP` calls. +2. On the host side, use the returned `makeTrapHandler(target)` to mark a target + as synchronous-enabled. +3. On the guest side, use the returned `Trap(target)` proxy maker much like + `E(target)`, but it will return a synchronous result. `Trap` will throw an + error if `target` was not marked as a TrapHandler by the host. + +To understand how `trapHost` and `trapGuest` relate, consider the `trapHost` as +a maker of AsyncIterators which don't return any useful value. These specific +iterators are used to drive the transfer of serialized data back to the guest. + +`trapGuest` receives arguments to describe the specific trap request, including +`startTrap()` which sends data to the host to perform the actual work of the +trap. The returned (synchronous) iterator from `startTrap()` drives the async +iterator of the host until it fully transfers the trap results to the guest, and +the guest unblocks. + +The Loopback implementation provides partial support for TrapCaps, except it +cannot unwrap promises. Loopback TrapHandlers must return synchronously, or an +exception will be thrown. diff --git a/packages/captp/jsconfig.json b/packages/captp/jsconfig.json index 6aa8279c4c4..4d4c6b8dcf0 100644 --- a/packages/captp/jsconfig.json +++ b/packages/captp/jsconfig.json @@ -14,5 +14,5 @@ "strictNullChecks": true, "moduleResolution": "node", }, - "include": ["src/**/*.js", "exported.js"], + "include": ["src/**/*.js", "src/**/*.d.ts", "exported.js"], } diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index 374c8a1eac7..aaf9efb3992 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -15,11 +15,6 @@ import './types.js'; export { E }; -/** - * @template T - * @typedef {import('@agoric/eventual-send').ERef} ERef - */ - /** * @param {any} maybeThenable * @returns {boolean} @@ -34,7 +29,7 @@ const isThenable = maybeThenable => * assist in ignoring earlier defunct instance's messages * @property {TrapGuest} trapGuest if specified, enable this CapTP (guest) to * use Trap(target) to block while the recipient (host) resolves and - * communicates the message + * communicates the response to the message * @property {TrapHost} trapHost if specified, enable this CapTP (host) to serve * objects marked with makeTrapHandler to synchronous clients (guests) */ diff --git a/packages/captp/src/loopback.js b/packages/captp/src/loopback.js index ca308fbfb7f..a59de6bd6c2 100644 --- a/packages/captp/src/loopback.js +++ b/packages/captp/src/loopback.js @@ -4,6 +4,11 @@ import { nearTrapImpl } from './trap.js'; export { E }; +/** + * @template T + * @typedef {import('@agoric/eventual-send').ERef} ERef + */ + /** * Create an async-isolated channel to an object. * diff --git a/packages/captp/src/ts-types.d.ts b/packages/captp/src/ts-types.d.ts index e0bac854041..65f366d664e 100644 --- a/packages/captp/src/ts-types.d.ts +++ b/packages/captp/src/ts-types.d.ts @@ -1,9 +1,15 @@ /* eslint-disable */ // eslint-disable-next-line spaced-comment -type ERef = PromiseLike | T; -type Unpromise = T extends ERef ? U : T; +import type { Unpromise } from '@agoric/eventual-send'; +/** + * In order to type using Trap with a handler TrapHandler, this template type + * examines its parameter, and transforms any Promise function return types + * or Promise object property types into the corresponding resolved type R. + * + * That correctly describes that Trap(target)... "unpromises" any results. + */ export type TrapHandler = T extends (...args: infer P) => infer R ? (...args: P) => Unpromise : T extends Record @@ -27,24 +33,28 @@ type TrapSingleGet = { export interface Trap { /** + * @template T + * * Trap(x) returns a proxy on which you can call arbitrary methods. Each of * these method calls will unwrap a promise result. The method will be * invoked on a remote 'x', and be synchronous from the perspective of this * caller. * - * @param {*} x target for method/function call - * @returns {TrapSingleCall} method/function call proxy + * @param {T} x target for method/function call + * @returns {TrapSingleCall>} method/function call proxy */ (x: T): TrapSingleCall>; /** + * @template T + * * Trap.get(x) returns a proxy on which you can get arbitrary properties. * Each of these properties unwraps a promise result. The value will be the * property fetched from a remote 'x', and be synchronous from the perspective * of this caller. * - * @param {*} x target for property get - * @returns {TrapSingleGet} property get proxy + * @param {T} x target for property get + * @returns {TrapSingleGet>} property get proxy */ readonly get(x: T): TrapSingleGet>; } diff --git a/packages/captp/src/types.js b/packages/captp/src/types.js index 6d90357b30e..479e32d1d1a 100644 --- a/packages/captp/src/types.js +++ b/packages/captp/src/types.js @@ -6,8 +6,11 @@ * @typedef {Object} TrapImpl * @property {(target: any, args: Array) => any} applyFunction function * application - * @property {(target: any, method: string | symbol | number, args: Array) - * => any} applyMethod method invocation, which is an atomic lookup of method and + * @property {( + * target: any, + * method: string | symbol | number, + * args: Array + * ) => any} applyMethod method invocation, which is an atomic lookup of method and * apply * @property {(target: any, prop: string | symbol | number) => any} get property * lookup @@ -42,8 +45,8 @@ * @callback TrapHost start the process of transferring the Trap request's * results * @param {TrapCompletion} completion - * @returns {AsyncIterator | undefined} If an AsyncIterator is returned, it - * will satisfy a future guest IterationObserver. + * @returns {AsyncIterator | undefined} If an AsyncIterator is + * returned, it will satisfy a future guest IterationObserver. */ /** @typedef {import('./ts-types').Trap} Trap */ From 933d5a7d875ad3c29b88edd0e7d0abae5583c117 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Fri, 16 Jul 2021 21:39:21 -0600 Subject: [PATCH 24/31] style(captp): the ProxyHandler is not readOnly, it's freezable --- packages/captp/src/trap.js | 7 ++++--- packages/eventual-send/src/E.js | 9 +++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/captp/src/trap.js b/packages/captp/src/trap.js index 52db67027ab..850bd8149fb 100644 --- a/packages/captp/src/trap.js +++ b/packages/captp/src/trap.js @@ -20,7 +20,8 @@ export const nearTrapImpl = harden({ }, }); -const readOnlyProxyHandler = { +/** @type {ProxyHandler} */ +const baseFreezableProxyHandler = { set(_target, _prop, _value) { return false; }, @@ -44,7 +45,7 @@ const readOnlyProxyHandler = { */ const TrapProxyHandler = (x, trapImpl) => { return harden({ - ...readOnlyProxyHandler, + ...baseFreezableProxyHandler, get(_target, p, _receiver) { return (...args) => trapImpl.applyMethod(x, p, args); }, @@ -70,7 +71,7 @@ export const makeTrap = trapImpl => { const makeTrapGetterProxy = x => { const handler = harden({ - ...readOnlyProxyHandler, + ...baseFreezableProxyHandler, has(_target, _prop) { // TODO: has property is not yet transferrable over captp. return true; diff --git a/packages/eventual-send/src/E.js b/packages/eventual-send/src/E.js index 2a24e2b1464..6128a8cc916 100644 --- a/packages/eventual-send/src/E.js +++ b/packages/eventual-send/src/E.js @@ -3,7 +3,8 @@ import { trackTurns } from './track-turns.js'; // eslint-disable-next-line spaced-comment /// -const readOnlyProxyHandler = { +/** @type {ProxyHandler} */ +const baseFreezableProxyHandler = { set(_target, _prop, _value) { return false; }, @@ -27,7 +28,7 @@ const readOnlyProxyHandler = { */ function EProxyHandler(x, HandledPromise) { return harden({ - ...readOnlyProxyHandler, + ...baseFreezableProxyHandler, get(_target, p, _receiver) { // Harden this Promise because it's our only opportunity to ensure // p1=E(x).foo() is hardened. The Handled Promise API does not (yet) @@ -56,7 +57,7 @@ function EProxyHandler(x, HandledPromise) { */ function EsendOnlyProxyHandler(x, HandledPromise) { return harden({ - ...readOnlyProxyHandler, + ...baseFreezableProxyHandler, get(_target, p, _receiver) { return (...args) => { HandledPromise.applyMethod(x, p, args); @@ -82,7 +83,7 @@ export default function makeE(HandledPromise) { const makeEGetterProxy = x => new Proxy(Object.create(null), { - ...readOnlyProxyHandler, + ...baseFreezableProxyHandler, has(_target, _prop) { return true; }, From 592f0b78b6adcd2956c925b8294ed9452ff4c9bb Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Fri, 16 Jul 2021 22:02:57 -0600 Subject: [PATCH 25/31] fix(captp): properly export src/index.js --- packages/captp/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/captp/package.json b/packages/captp/package.json index 2ec6b7b85b2..a53bc3a24c0 100644 --- a/packages/captp/package.json +++ b/packages/captp/package.json @@ -13,8 +13,8 @@ "author": "Michael FIG ", "homepage": "https://github.com/Agoric/agoric-sdk#readme", "license": "Apache-2.0", - "main": "src/captp.js", - "module": "src/captp.js", + "main": "src/index.js", + "module": "src/index.js", "directories": { "src": "src", "test": "test" From feda6c8510f56385c2becec40412223b4acf109d Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Sat, 17 Jul 2021 11:40:33 -0600 Subject: [PATCH 26/31] fix(captp): ensure trapcap reply iteration is serial --- packages/captp/src/captp.js | 45 ++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index aaf9efb3992..1762c0ad5ba 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -443,31 +443,30 @@ export const makeCapTP = ( const [method, args] = unserialize(serialized); - const resultPK = makePromiseKit(); - trapIteratorResultP.set(questionID, resultPK.promise); + const getNextResultP = async () => { + const result = await resultP; + if (!result || result.done) { + // We're done! + trapIterator.delete(questionID); + trapIteratorResultP.delete(questionID); + return result; + } - const { done } = await resultP; - if (done) { - trapIterator.delete(questionID); - return; - } - const ait = trapIterator.get(questionID); - - try { - switch (method) { - case 'next': - case 'return': - case 'throw': { - resultPK.resolve(ait && ait[method] && ait[method](...args)); - break; - } - default: { - assert.fail(X`Unrecognized iteration method ${method}`); - } + const ait = trapIterator.get(questionID); + if (ait && ait[method]) { + // Drive the next iteration. + return ait[method](...args); } - } catch (e) { - resultPK.reject(e); - } + + return result; + }; + + // Store the next result promise. + const nextResultP = getNextResultP(); + trapIteratorResultP.set(questionID, nextResultP); + + // Wait for the next iteration so that we properly report errors. + await nextResultP; }, // Answer to one of our questions. async CTP_RETURN(obj) { From 003c3d16dc2301ae171d9cc60ab30509fa7ee9ea Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Sat, 17 Jul 2021 13:06:46 -0600 Subject: [PATCH 27/31] fix(captp): more robust CTP_TRAP_ITERATE error handling --- packages/captp/src/captp.js | 41 ++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index 1762c0ad5ba..f7d84670f49 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -443,30 +443,43 @@ export const makeCapTP = ( const [method, args] = unserialize(serialized); + const DONE_PUMPKIN = { toString: () => 'DONE_PUMPKIN' }; const getNextResultP = async () => { const result = await resultP; - if (!result || result.done) { - // We're done! - trapIterator.delete(questionID); - trapIteratorResultP.delete(questionID); - return result; - } + try { + if (!result || result.done) { + throw DONE_PUMPKIN; + } + + const ait = trapIterator.get(questionID); + if (!ait) { + // The iterator is done, so we're done. + throw DONE_PUMPKIN; + } - const ait = trapIterator.get(questionID); - if (ait && ait[method]) { // Drive the next iteration. - return ait[method](...args); + return await ait[method](...args); + } catch (e) { + // Done with this trap iterator. + trapIterator.delete(questionID); + trapIteratorResultP.delete(questionID); + if (e !== DONE_PUMPKIN) { + // We had an exception. + throw e; + } + return harden({ done: true }); } - - return result; }; // Store the next result promise. const nextResultP = getNextResultP(); trapIteratorResultP.set(questionID, nextResultP); - // Wait for the next iteration so that we properly report errors. - await nextResultP; + // Ensure that something handles any rejection. + nextResultP.catch(e => quietReject(e, false)); + + // We shouldn't just return the next result promise, because our caller + // isn't waiting for our answer. }, // Answer to one of our questions. async CTP_RETURN(obj) { @@ -646,7 +659,7 @@ export const makeCapTP = ( questionID, serialized: serialize(harden([iteratorMethod, args])), }); - return { done, value: undefined }; + return harden({ done, value: undefined }); }; return harden({ next: makeIteratorMethod('next', false), From 5a370a8404124409e5bbdf60c4ccf494fde8b103 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Mon, 19 Jul 2021 14:02:06 -0600 Subject: [PATCH 28/31] fix(captp): don't rely on TextDecoder stream flag --- packages/captp/src/atomics.js | 65 +++++++++++++++++++++++++---------- packages/captp/src/captp.js | 31 ++++++++++------- 2 files changed, 65 insertions(+), 31 deletions(-) diff --git a/packages/captp/src/atomics.js b/packages/captp/src/atomics.js index 1eca551e3d7..0ae2b0cb69e 100644 --- a/packages/captp/src/atomics.js +++ b/packages/captp/src/atomics.js @@ -4,7 +4,6 @@ import { assert, details as X } from '@agoric/assert'; // This is a pathological minimum, but exercised by the unit test. -// transfer. export const MIN_DATA_BUFFER_LENGTH = 1; // Calculate how big the transfer buffer needs to be. @@ -29,6 +28,10 @@ const splitTransferBuffer = transferBuffer => { X`Transfer buffer of ${transferBuffer.byteLength} bytes is smaller than MIN_TRANSFER_BUFFER_LENGTH ${MIN_TRANSFER_BUFFER_LENGTH}`, ); const lenbuf = new BigUint64Array(transferBuffer, 0, 1); + + // The documentation says that this needs to be an Int32Array for use with + // Atomics.notify: + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Atomics/notify#syntax const statusbuf = new Int32Array(transferBuffer, lenbuf.byteLength, 1); const overheadLength = lenbuf.byteLength + statusbuf.byteLength; assert.equal( @@ -36,10 +39,7 @@ const splitTransferBuffer = transferBuffer => { TRANSFER_OVERHEAD_LENGTH, X`Internal error; actual overhead ${overheadLength} of bytes is not TRANSFER_OVERHEAD_LENGTH ${TRANSFER_OVERHEAD_LENGTH}`, ); - const databuf = new Uint8Array( - transferBuffer, - lenbuf.byteLength + statusbuf.byteLength, - ); + const databuf = new Uint8Array(transferBuffer, overheadLength); assert( databuf.byteLength >= MIN_DATA_BUFFER_LENGTH, X`Transfer buffer of size ${transferBuffer.byteLength} only supports ${databuf.byteLength} data bytes; need at least ${MIN_DATA_BUFFER_LENGTH}`, @@ -75,7 +75,8 @@ export const makeAtomicsTrapHost = transferBuffer => { databuf.set(subenc); // Save the length of the remaining data. - lenbuf[0] = BigInt(encoded.length - i); + const remaining = BigInt(encoded.length - i); + lenbuf[0] = remaining; // Calculate the next slice, and whether this is the last one. i += subenc.length; @@ -91,8 +92,12 @@ export const makeAtomicsTrapHost = transferBuffer => { statusbuf[0] = rejectFlag | doneFlag; Atomics.notify(statusbuf, 0, +Infinity); - // Wait until the next call. - yield; + if (!done) { + // Wait until the next call to `it.next()`. If the guest calls + // `it.return()` or `it.throw()`, then this yield will return or throw, + // terminating the generator function early. + yield; + } } }; }; @@ -110,18 +115,18 @@ export const makeAtomicsTrapGuest = transferBuffer => { const { statusbuf, lenbuf, databuf } = splitTransferBuffer(transferBuffer); return ({ startTrap }) => { - const td = new TextDecoder('utf-8'); - - let json = ''; - // Start by sending the trap call to the host. const it = startTrap(); + /** @type {Uint8Array} */ + let encoded; + let i = 0; let done = false; while (!done) { // Tell that we are ready for another buffer. statusbuf[0] = STATUS_WAITING; - it.next(); + const { done: itDone } = it.next(); + assert(!itDone, X`Internal error; it.next() returned done=${itDone}`); // Wait for the host to wake us. Atomics.wait(statusbuf, 0, STATUS_WAITING); @@ -130,18 +135,40 @@ export const makeAtomicsTrapGuest = transferBuffer => { // eslint-disable-next-line no-bitwise done = (statusbuf[0] & STATUS_FLAG_DONE) !== 0; - // Decode the next part of the data buffer. - json += td.decode(databuf.subarray(0, Number(lenbuf[0])), { - stream: !done, - }); + // Accumulate the encoded buffer. + const remaining = Number(lenbuf[0]); + const datalen = Math.min(remaining, databuf.byteLength); + if (i === 0) { + if (done) { + // Special case: we are done on first try, so we don't need to copy + // anything. + encoded = databuf.subarray(0, datalen); + break; + } + // Allocate our buffer for the remaining data. + encoded = new Uint8Array(remaining); + } + + // Copy the next buffer. + encoded.set(databuf.subarray(0, datalen), i); + i += datalen; } - // Tell the host we're finished. - it.return(); + // This throw is harmless if the host iterator has already finished, and + // if not finished, captp will correctly raise an error. + // + // TODO: It would be nice to use an error type, but captp is just too + // noisy with spurious "Temporary logging of sent error" messages. + // it.throw(assert.error(X`Trap host has not finished`)); + it.throw(undefined); // eslint-disable-next-line no-bitwise const isReject = !!(statusbuf[0] & STATUS_FLAG_REJECT); + // Decode the accumulated encoded buffer. + const td = new TextDecoder('utf-8'); + const json = td.decode(encoded); + // Parse the JSON data into marshalled form. const serialized = JSON.parse(json); return [isReject, serialized]; diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index f7d84670f49..1fc0b802195 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -385,7 +385,7 @@ export const makeCapTP = ( assert.typeof( trapHost, 'function', - X`CapTP cannot answer Trap(x) without a trapHost function`, + X`CapTP cannot answer Trap(${val}) without a trapHost function`, ); // We need to create a promise for the "isDone" iteration right now to @@ -443,31 +443,38 @@ export const makeCapTP = ( const [method, args] = unserialize(serialized); - const DONE_PUMPKIN = { toString: () => 'DONE_PUMPKIN' }; const getNextResultP = async () => { const result = await resultP; + + // Done with this trap iterator. + const cleanup = () => { + trapIterator.delete(questionID); + trapIteratorResultP.delete(questionID); + return harden({ done: true }); + }; + try { if (!result || result.done) { - throw DONE_PUMPKIN; + return cleanup(); } const ait = trapIterator.get(questionID); if (!ait) { // The iterator is done, so we're done. - throw DONE_PUMPKIN; + return cleanup(); } // Drive the next iteration. return await ait[method](...args); } catch (e) { - // Done with this trap iterator. - trapIterator.delete(questionID); - trapIteratorResultP.delete(questionID); - if (e !== DONE_PUMPKIN) { - // We had an exception. - throw e; + cleanup(); + if (e === undefined) { + assert.fail( + X`trapGuest expected trapHost AsyncIterator(${questionID}) to be done, but it wasn't`, + ); } - return harden({ done: true }); + assert.note(e, X`trapHost AsyncIterator(${questionID}) threw`); + throw e; } }; @@ -672,7 +679,7 @@ export const makeCapTP = ( const value = unserialize(serialized); assert( !isThenable(value), - X`Trap() reply cannot be a Thenable; have ${value}`, + X`Trap(${target}) reply cannot be a Thenable; have ${value}`, ); if (isException) { From 6fc842cc3160f134455a250c8a13418e07301848 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Mon, 19 Jul 2021 14:40:53 -0600 Subject: [PATCH 29/31] fix(captp): relax it.throw signature --- packages/captp/src/atomics.js | 6 +++--- packages/captp/src/captp.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/captp/src/atomics.js b/packages/captp/src/atomics.js index 0ae2b0cb69e..76e53102db1 100644 --- a/packages/captp/src/atomics.js +++ b/packages/captp/src/atomics.js @@ -118,7 +118,7 @@ export const makeAtomicsTrapGuest = transferBuffer => { // Start by sending the trap call to the host. const it = startTrap(); - /** @type {Uint8Array} */ + /** @type {Uint8Array | undefined} */ let encoded; let i = 0; let done = false; @@ -138,7 +138,7 @@ export const makeAtomicsTrapGuest = transferBuffer => { // Accumulate the encoded buffer. const remaining = Number(lenbuf[0]); const datalen = Math.min(remaining, databuf.byteLength); - if (i === 0) { + if (!encoded) { if (done) { // Special case: we are done on first try, so we don't need to copy // anything. @@ -160,7 +160,7 @@ export const makeAtomicsTrapGuest = transferBuffer => { // TODO: It would be nice to use an error type, but captp is just too // noisy with spurious "Temporary logging of sent error" messages. // it.throw(assert.error(X`Trap host has not finished`)); - it.throw(undefined); + it.throw(null); // eslint-disable-next-line no-bitwise const isReject = !!(statusbuf[0] & STATUS_FLAG_REJECT); diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index 1fc0b802195..d9c51235536 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -468,7 +468,7 @@ export const makeCapTP = ( return await ait[method](...args); } catch (e) { cleanup(); - if (e === undefined) { + if (!e) { assert.fail( X`trapGuest expected trapHost AsyncIterator(${questionID}) to be done, but it wasn't`, ); From 15292fe064147f0bc7d6c8598be5a43d41552ab4 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Mon, 19 Jul 2021 16:45:50 -0600 Subject: [PATCH 30/31] style(captp): fix types line wrapping --- packages/captp/src/types.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/captp/src/types.js b/packages/captp/src/types.js index 479e32d1d1a..dc73c3860ad 100644 --- a/packages/captp/src/types.js +++ b/packages/captp/src/types.js @@ -10,16 +10,16 @@ * target: any, * method: string | symbol | number, * args: Array - * ) => any} applyMethod method invocation, which is an atomic lookup of method and - * apply + * ) => any} applyMethod method invocation, which is an atomic lookup of method + * and apply * @property {(target: any, prop: string | symbol | number) => any} get property * lookup */ /** - * @typedef {[boolean, CapData]} TrapCompletion The head of the pair is the - * `isRejected` value indicating whether the sync call was an exception, and - * tail of the pair is the serialized fulfillment value or rejection reason. + * @typedef {[boolean, CapData]} TrapCompletion The head of the pair + * is the `isRejected` value indicating whether the sync call was an exception, + * and tail of the pair is the serialized fulfillment value or rejection reason. * (The fulfillment value is a non-thenable. The rejection reason is normally * an error.) */ @@ -29,13 +29,13 @@ * @property {keyof TrapImpl} trapMethod the TrapImpl method that was called * @property {CapTPSlot} slot the target slot * @property {Array} trapArgs arguments to the TrapImpl method - * @property {() => Required>} startTrap start the trap process on - * the trapHost, and drive the other side. + * @property {() => Required>} startTrap start the + * trap process on the trapHost, and drive the other side. */ /** * @callback TrapGuest Use out-of-band communications to synchronously return a - * TrapCompleted value indicating the final results of a Trap call. + * TrapCompletion value indicating the final results of a Trap call. * * @param {TrapRequest} req * @returns {TrapCompletion} From 21b72cd54ec95e9fcc86638086c7c0c09a3e71cf Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Mon, 19 Jul 2021 17:23:12 -0600 Subject: [PATCH 31/31] fix(captp): clarify error handling --- packages/captp/src/captp.js | 97 +++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 47 deletions(-) diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index d9c51235536..9bfd68a6cd6 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -55,6 +55,15 @@ export const makeCapTP = ( trapHost, } = opts; + // It's a hazard to have trapGuest and trapHost both enabled, as we may + // encounter deadlock. Without a lot more bookkeeping, we can't detect it for + // more general networks of CapTPs, but we are conservative for at least this + // one case. + assert( + !(trapHost && trapGuest), + X`CapTP ${ourId} can only be one of either trapGuest or trapHost`, + ); + const disconnectReason = id => Error(`${JSON.stringify(id)} connection closed`); @@ -368,7 +377,7 @@ export const makeCapTP = ( } /** @type {(isReject: boolean, value: any) => void} */ - let sendReturn = (isReject, value) => { + let processResult = (isReject, value) => { send({ type: 'CTP_RETURN', epoch, @@ -377,43 +386,38 @@ export const makeCapTP = ( }); }; if (trap) { - try { - assert( - exportedTrapHandlers.has(val), - X`Refused Trap(${val}) because target was not registered with makeTrapHandler`, - ); - assert.typeof( - trapHost, - 'function', - X`CapTP cannot answer Trap(${val}) without a trapHost function`, - ); - - // We need to create a promise for the "isDone" iteration right now to - // prevent a race with the other side. - const resultPK = makePromiseKit(); - trapIteratorResultP.set(questionID, resultPK.promise); - - sendReturn = async (isReject, value) => { - const serialized = serialize(harden(value)); - const ait = trapHost([isReject, serialized]); - if (!ait) { - // One-shot, no async iterator. - resultPK.resolve({ done: true }); - return; - } - - // We're ready for them to drive the iterator. - trapIterator.set(questionID, ait); - resultPK.resolve({ done: false }); - }; - } catch (e) { - sendReturn(true, e); - throw e; - } + assert( + exportedTrapHandlers.has(val), + X`Refused Trap(${val}) because target was not registered with makeTrapHandler`, + ); + assert.typeof( + trapHost, + 'function', + X`CapTP cannot answer Trap(${val}) without a trapHost function`, + ); + + // We need to create a promise for the "isDone" iteration right now to + // prevent a race with the other side. + const resultPK = makePromiseKit(); + trapIteratorResultP.set(questionID, resultPK.promise); + + processResult = async (isReject, value) => { + const serialized = serialize(harden(value)); + const ait = trapHost([isReject, serialized]); + if (!ait) { + // One-shot, no async iterator. + resultPK.resolve({ done: true }); + return; + } + + // We're ready for them to drive the iterator. + trapIterator.set(questionID, ait); + resultPK.resolve({ done: false }); + }; } - // If `args` is supplied, we're applying a method or function... otherwise this is - // property access + // If `args` is supplied, we're applying a method or function... + // otherwise this is property access let hp; if (!args) { hp = HandledPromise.get(val, prop); @@ -426,12 +430,13 @@ export const makeCapTP = ( // Answer with our handled promise answers.set(questionID, hp); - // Set up promise resolver for this handled promise to send - // message to other vat when fulfilled/broken. - return hp - .then(res => sendReturn(false, res)) - .catch(rej => sendReturn(true, rej)) - .catch(rej => quietReject(rej, false)); + // We let rejections bubble up to our caller, `dispatch`. + await hp + // Process this handled promise method's result when settled. + .then( + fulfilment => processResult(false, fulfilment), + reason => processResult(true, reason), + ); }, // Have the host serve more of the reply. CTP_TRAP_ITERATE: async obj => { @@ -453,6 +458,7 @@ export const makeCapTP = ( return harden({ done: true }); }; + // We want to ensure we clean up the iterator in case of any failure. try { if (!result || result.done) { return cleanup(); @@ -482,11 +488,8 @@ export const makeCapTP = ( const nextResultP = getNextResultP(); trapIteratorResultP.set(questionID, nextResultP); - // Ensure that something handles any rejection. - nextResultP.catch(e => quietReject(e, false)); - - // We shouldn't just return the next result promise, because our caller - // isn't waiting for our answer. + // Ensure that our caller handles any rejection. + await nextResultP; }, // Answer to one of our questions. async CTP_RETURN(obj) {