From 15a18d77529254ddc3a4f6b13b71c492dcfd7bb0 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Wed, 20 Nov 2024 13:06:08 -0500 Subject: [PATCH 1/2] chore(SwingSet): Improve config documentation and typing --- packages/SwingSet/docs/configuration.md | 50 +++++++++--------- .../src/controller/initializeKernel.js | 1 - .../src/controller/initializeSwingset.js | 33 ++++++------ packages/SwingSet/src/typeGuards.js | 44 ++++++++-------- packages/SwingSet/src/types-external.js | 51 +++++++++---------- packages/SwingSet/src/types-internal.js | 3 +- packages/SwingSet/test/gc/gc-vat.test.js | 2 + .../virtualObjects/weakcollections.test.js | 1 + 8 files changed, 91 insertions(+), 94 deletions(-) diff --git a/packages/SwingSet/docs/configuration.md b/packages/SwingSet/docs/configuration.md index 9c7614ea1c6..4cf9860ec56 100644 --- a/packages/SwingSet/docs/configuration.md +++ b/packages/SwingSet/docs/configuration.md @@ -22,6 +22,7 @@ The configuration object takes this form: config = { defaultManagerType: MANAGERTYPESTRING? includeDevDependencies: BOOL? + defaultReapGCKrefs: NUMBER|'never'? defaultReapInterval: NUMBER|'never'? relaxDurabilityRules: BOOL? snapshotInitial: NUMBER? @@ -38,15 +39,15 @@ config = { endowments: DATAOBJECT? creationOptions: { enablePipelining: BOOL? - name: STRING? enableDisavow: BOOL? managerType: MANAGERTYPESTRING? - metered: BOOL? + neverReap: BOOL? + nodeOptions: STRING[]? + reapGCKrefs: NUMBER|'never'? reapInterval: NUMBER|'never'? enableSetup: BOOL? critical: BOOL? useTranscript: BOOL? - virtualObjectCacheSize: NUMBER? }? }+ } @@ -126,13 +127,18 @@ creates bundles to include the SwingSet package's dev dependencies as well as its regular runtime depencies in any bundles that it creates. Defaults to `false`. -The `defaultReapInterval` property specifies the default frequency with which -the kernel should prompt each vat (by delivering it a `bringOutYourDead` -directive) to perform garbage collection and notify the kernel of any dropped or -released references that result. This should either be an integer, indicating -the number of deliveries that should be made to the vat before reaping, or the -string `'never'`, indicating that such activity should never happen. If -defaults (yes, the default has a default) to 1. +The `defaultReapGCKrefs` property specifies the default count of krefs to be +received by each vat in GC deliveries (dropImports/retireImports/retireExports) +before the kernel should deliver a "bringOutYourDead" to trigger garbage +collection. It should either be an integer, or the string `'never'` to indicate +that such counts do not affect when to deliver a bringOutYourDead. It defaults +to 20 (cf. `DEFAULT_GC_KREFS_PER_BOYD`). + +The `defaultReapInterval` property specifies the default count of deliveries to +be received by each vat before the kernel should deliver a "bringOutYourDead" to +trigger garbage collection. It should either be an integer, or the string +`'never'` to indicate that such counts do not affect when to deliver a +bringOutYourDead. It defaults to 1. The `relaxDurabilityRules` property, if `true`, allows vat code running inside this swingset to store non-durable references inside durable objects and stores @@ -197,10 +203,6 @@ not yet supported. - `enablePipelining` specifies whether the vat should be launched with promise pipelining enabled. Defaults to `false`. -- `name` is an optional short label string used to identify the vat in log and - debug outputs. If omitted, it defaults to the property name that was used for - the vat descriptor. - - `enableDisavow`, if `true`, adds `vatPowers.disavow()`, which allows vat code to explicitly disavow interest in an imported Presence. This will trigger a `syscall.dropImports` of the associated object ID. By default, this @@ -209,11 +211,17 @@ not yet supported. - `managerType` indicates what sort of vat worker to run the associated vat in. If omitted, it defaults to the swingset's default manager type. -- `metered` specifies whether the vat should be have metering turned - on or not. Defaults to `false`. +- `neverReap` disables automatic bringOutYourDead deliveries to this vat. + +- `nodeOptions` is valid only with `managerType` `'node-subprocess'`, for which + it specifies additional command-line options to include when spawning (e.g., + `['--inspect']` to support interactive debugging). -- `reapInterval` specifies the reap interval for this particular vat. It - defaults to the swingset's default reap interval. +- `reapGCKrefs` overrides the swingset's `defaultReapGCKrefs` for this + particular vat. + +- `reapInterval`, overrides the swingset's `defaultReapInterval` for this + particular vat. - `enableSetup` indicates that the vat module may use the older, lower level `setup()` API, which allows a vat to be defined independent of @@ -230,10 +238,6 @@ not yet supported. deliveries made to it so that these can be replayed at a later time to reconstruct the internal state of the vat. Defaults to `true`. -- `virtualObjectCacheSize` tells the vat's virtual object manager how many - virtual objects should have their state kept live in memory at any given - time. Defaults to an implementation defined value. - ## Dynamic option setting Some swingset and vat configuration options can be dynamically updated within a @@ -271,8 +275,6 @@ vats, i.e., vats configured using the config object described above). Currently supported vat options that you can change this way are: -- `virtualObjectCacheSize` - - `reapInterval` Other options may be added in the future. diff --git a/packages/SwingSet/src/controller/initializeKernel.js b/packages/SwingSet/src/controller/initializeKernel.js index dfe3ba00157..6977500b35f 100644 --- a/packages/SwingSet/src/controller/initializeKernel.js +++ b/packages/SwingSet/src/controller/initializeKernel.js @@ -100,7 +100,6 @@ export async function initializeKernel(config, kernelStorage, options = {}) { // the VatManager, since it isn't available until the bundle is evaluated assertKnownOptions(creationOptions, [ 'enablePipelining', - 'metered', 'managerType', 'enableDisavow', 'enableSetup', diff --git a/packages/SwingSet/src/controller/initializeSwingset.js b/packages/SwingSet/src/controller/initializeSwingset.js index 7ecb1f048c3..f04bb63f257 100644 --- a/packages/SwingSet/src/controller/initializeSwingset.js +++ b/packages/SwingSet/src/controller/initializeSwingset.js @@ -442,26 +442,24 @@ export async function initializeSwingset( * The host application gives us * config.[vats|devices].NAME.[bundle|bundleSpec|sourceSpec|bundleName] . * The 'bundleName' option points into - * config.bundles.BUNDLENAME.[bundle|bundleSpec|sourceSpec] , which can + * config.bundles.BUNDLENAME.[bundle|bundleSpec|sourceSpec], which can * also include arbitrary named bundles that will be made available to - * E(vatAdminService).getNamedBundleCap(bundleName) ,and temporarily as + * E(vatAdminService).getNamedBundleCap(bundleName), and temporarily as * E(vatAdminService).createVatByName(bundleName) * * The 'kconfig' we pass through to initializeKernel has * kconfig.[vats|devices].NAME.bundleID and - * kconfig.namedBundleIDs.BUNDLENAME=bundleID , which both point into + * kconfig.namedBundleIDs.BUNDLENAME=bundleID, which both point into * kconfig.idToBundle.BUNDLEID=bundle * - * @param {SwingSetConfigProperties | { bundleName: string }} desc + * @param {SwingSetConfigProperties} desc * @param {Record} [nameToBundle] */ async function getBundle(desc, nameToBundle) { trace( 'getBundle', Object.keys(desc), - // @ts-expect-error optional desc.moduleFormat, - // @ts-expect-error optional desc.endoZipBase64Sha512 || desc.sourceSpec, ); @@ -489,8 +487,9 @@ export async function initializeSwingset( throw Error(`unknown mode in desc`, desc); } - // fires with BundleWithID: { ...bundle, id } /** + * Returns a bundle record with an "id" property from an input that might be missing it. + * * @param {EndoZipBase64Bundle & {id?: string}} bundle * @returns {Promise} bundle */ @@ -508,11 +507,9 @@ export async function initializeSwingset( }; } - // fires with BundleWithID: { ...bundle, id } - /** * - * @param {(SwingSetConfigProperties | { bundleName: string }) & {bundleID?: string }} desc + * @param {SwingSetConfigProperties & {bundleID?: string}} desc * @param {Record} [nameToBundle] */ async function processDesc(desc, nameToBundle) { @@ -526,14 +523,14 @@ export async function initializeSwingset( modes.length === 1 || Fail`need =1 of bundle/bundleSpec/sourceSpec/bundleName, got ${modes}`; const mode = modes[0]; - return getBundle(desc, nameToBundle) - .then(addBundleID) - .then(bundleWithID => { - // replace original .sourceSpec/etc with a uniform .bundleID - delete desc[mode]; - desc.bundleID = bundleWithID.id; - return bundleWithID; - }); + + // Remove the original mode in favor of a uniform "bundleID" property. + const bundle = await getBundle(desc, nameToBundle); + const bundleWithID = await addBundleID(bundle); + delete desc[mode]; + desc.bundleID = bundleWithID.id; + + return bundleWithID; } /** diff --git a/packages/SwingSet/src/typeGuards.js b/packages/SwingSet/src/typeGuards.js index 44bbb67b365..a3a5a84adc7 100644 --- a/packages/SwingSet/src/typeGuards.js +++ b/packages/SwingSet/src/typeGuards.js @@ -10,38 +10,40 @@ export const ManagerType = M.or( const Bundle = M.splitRecord({ moduleType: M.string() }); -const SwingsetConfigOptions = { +const VatConfigOptions = harden({ creationOptions: M.splitRecord({}, { critical: M.boolean() }), parameters: M.recordOf(M.string(), M.any()), -}; -harden(SwingsetConfigOptions); +}); -const SwingSetConfigProperties = M.or( - M.splitRecord({ sourceSpec: M.string() }, SwingsetConfigOptions), - M.splitRecord({ bundleSpec: M.string() }, SwingsetConfigOptions), - M.splitRecord({ bundle: Bundle }, SwingsetConfigOptions), -); -const SwingSetConfigDescriptor = M.recordOf( - M.string(), - SwingSetConfigProperties, -); +const makeSwingSetConfigProperties = (required = {}, optional = {}, rest) => + M.or( + M.splitRecord({ sourceSpec: M.string(), ...required }, optional, rest), + M.splitRecord({ bundleSpec: M.string(), ...required }, optional, rest), + M.splitRecord({ bundle: Bundle, ...required }, optional, rest), + ); +const makeSwingSetConfigDescriptor = (required, optional, rest) => + M.recordOf( + M.string(), + makeSwingSetConfigProperties(required, optional, rest), + ); /** * NOTE: this pattern suffices for PSM bootstrap, * but does not cover the whole SwingSet config syntax. * * {@link ./docs/configuration.md} - * TODO: move this to swingset? * * @see SwingSetConfig * in ./types-external.js */ -export const SwingSetConfig = M.and( - M.splitRecord({}, { defaultManagerType: ManagerType }), - M.splitRecord({}, { includeDevDependencies: M.boolean() }), - M.splitRecord({}, { defaultReapInterval: M.number() }), // not in type decl - M.splitRecord({}, { snapshotInterval: M.number() }), - M.splitRecord({}, { vats: SwingSetConfigDescriptor }), - M.splitRecord({}, { bootstrap: M.string() }), - M.splitRecord({}, { bundles: SwingSetConfigDescriptor }), +export const SwingSetConfig = M.splitRecord( + { vats: makeSwingSetConfigDescriptor(undefined, VatConfigOptions) }, + { + defaultManagerType: ManagerType, + includeDevDependencies: M.boolean(), + defaultReapInterval: M.number(), + snapshotInterval: M.number(), + bootstrap: M.string(), + bundles: makeSwingSetConfigDescriptor(undefined, undefined, {}), + }, ); diff --git a/packages/SwingSet/src/types-external.js b/packages/SwingSet/src/types-external.js index 392324c1300..a49a18108bc 100644 --- a/packages/SwingSet/src/types-external.js +++ b/packages/SwingSet/src/types-external.js @@ -154,27 +154,25 @@ export {}; */ /** + * @typedef {{ bundle: Bundle }} BundleRef a bundle object + * @typedef {{ bundleName: string }} BundleName a name identifying a property in the `bundles` of a SwingSetOptions object + * @typedef {{ bundleSpec: string }} BundleSpec a path to a bundle file + * @typedef {{ sourceSpec: string }} SourceSpec a package specifier such as "@agoric/swingset-vat/tools/vat-puppet.js" + * * @typedef {{ - * sourceSpec: string // path to pre-bundled root - * }} SourceSpec - * @typedef {{ - * bundleSpec: string // path to bundled code - * }} BundleSpec - * @typedef {{ - * bundle: Bundle - * }} BundleRef - * @typedef {{ - * bundleName: string - * }} BundleName - * @typedef {(SourceSpec | BundleSpec | BundleRef | BundleName ) & { * bundleID?: BundleID, - * creationOptions?: Record, + * creationOptions?: StaticVatOptions, * parameters?: Record, - * }} SwingSetConfigProperties + * }} VatConfigOptions + */ +/** + * @template [Fields=object] + * @typedef {(SourceSpec | BundleSpec | BundleName | BundleRef) & Fields} SwingSetConfigProperties */ /** - * @typedef {Record} SwingSetConfigDescriptor + * @template [Fields=object] + * @typedef {Record>} SwingSetConfigDescriptor * Where the property name is the name of the vat. Note that * the `bootstrap` property names the vat that should be used as the bootstrap vat. Although a swingset * configuration can designate any vat as its bootstrap vat, `loadBasedir` will always look for a file named @@ -188,7 +186,7 @@ export {}; * `devDependencies` of the surrounding `package.json` should be accessible to * bundles. * @property {string} [bundleCachePath] if present, SwingSet will use a bundle cache at this path - * @property {SwingSetConfigDescriptor} vats + * @property {SwingSetConfigDescriptor} vats * @property {SwingSetConfigDescriptor} [bundles] * @property {BundleFormat} [bundleFormat] the bundle source / import bundle * format. @@ -206,7 +204,7 @@ export {}; */ /** - * @typedef {{ bundleName: string} | { bundle: Bundle } | { bundleID: BundleID } } SourceOfBundle + * @typedef {BundleName | BundleRef | {bundleID: BundleID}} SourceOfBundle */ /** * @typedef { import('@agoric/swing-store').KVStore } KVStore @@ -295,12 +293,8 @@ export {}; * Vat Creation and Management * * @typedef { string } BundleID - * @typedef {any} BundleCap + * @typedef { any } BundleCap * @typedef { { moduleFormat: 'endoZipBase64', endoZipBase64: string, endoZipBase64Sha512: string } } EndoZipBase64Bundle - * - * @typedef { unknown } Meter - * - * E(vatAdminService).createVat(bundle, options: DynamicVatOptions) */ /** @@ -326,8 +320,6 @@ export {}; * types are then defined as amendments to this base type. * * @typedef { object } BaseVatOptions - * @property { string } name - * @property { * } [vatParameters] * @property { boolean } [enableSetup] * If true, permits the vat to construct itself using the * `setup()` API, which bypasses the imposition of LiveSlots but @@ -346,6 +338,10 @@ export {}; * outbound syscalls so that the vat's internal state can be * reconstructed via replay. If false, no such record is kept. * Defaults to true. + * @property { ManagerType } [managerType] + * @property { boolean } [neverReap] + * If true, disables automatic bringOutYourDead deliveries to a vat. + * Defaults to false. * @property { number | 'never' } [reapInterval] * Trigger a bringOutYourDead after the vat has received * this many deliveries. If the value is 'never', @@ -360,7 +356,7 @@ export {}; */ /** - * @typedef { { meter?: Meter } } OptMeter + * @typedef { { meter?: unknown } } OptMeter * If a meter is provided, the new dynamic vat is limited to a fixed * amount of computation and allocation that can occur during any * given crank. Peak stack frames are limited as well. In addition, @@ -370,14 +366,13 @@ export {}; * terminated. If undefined, the vat is unmetered. Static vats * cannot be metered. * - * @typedef { { managerType?: ManagerType } } OptManagerType - * @typedef { BaseVatOptions & OptMeter & OptManagerType } DynamicVatOptions + * @typedef { BaseVatOptions & { name: string, vatParameters?: object } & OptMeter } DynamicVatOptions * * config.vats[name].creationOptions: StaticVatOptions * * @typedef { { enableDisavow?: boolean } } OptEnableDisavow * @typedef { { nodeOptions?: string[] } } OptNodeOptions - * @typedef { BaseVatOptions & OptManagerType & OptEnableDisavow & OptNodeOptions } StaticVatOptions + * @typedef { BaseVatOptions & OptEnableDisavow & OptNodeOptions } StaticVatOptions * * @typedef { { vatParameters?: object, upgradeMessage?: string } } VatUpgradeOptions * @typedef { { incarnationNumber: number } } VatUpgradeResults diff --git a/packages/SwingSet/src/types-internal.js b/packages/SwingSet/src/types-internal.js index 7a4b2ef1604..b8176f7b2f8 100644 --- a/packages/SwingSet/src/types-internal.js +++ b/packages/SwingSet/src/types-internal.js @@ -29,7 +29,6 @@ export {}; * @typedef { string } MeterID * @typedef { { meterID?: MeterID } } OptMeterID * @typedef { import('./types-external.js').BaseVatOptions } BaseVatOptions - * @typedef { import('./types-external.js').OptManagerType } OptManagerType * @typedef { import('@agoric/swingset-liveslots').VatDeliveryObject } VatDeliveryObject * @typedef { import('@agoric/swingset-liveslots').VatDeliveryResult } VatDeliveryResult * @typedef { import('@agoric/swingset-liveslots').VatSyscallObject } VatSyscallObject @@ -38,7 +37,7 @@ export {}; * * // used by vatKeeper.setSourceAndOptions(source, RecordedVatOptions) * - * @typedef { BaseVatOptions & OptMeterID & OptManagerType } InternalDynamicVatOptions + * @typedef { BaseVatOptions & OptMeterID } InternalDynamicVatOptions * * RecordedVatOptions is fully-specified, no optional fields * diff --git a/packages/SwingSet/test/gc/gc-vat.test.js b/packages/SwingSet/test/gc/gc-vat.test.js index 617e4b07a7e..c7aede4e52d 100644 --- a/packages/SwingSet/test/gc/gc-vat.test.js +++ b/packages/SwingSet/test/gc/gc-vat.test.js @@ -99,6 +99,7 @@ test.serial('drop presence (export retains)', t => dropPresence(t, false)); test.serial('drop presence (export drops)', t => dropPresence(t, true)); test('forward to fake zoe', async t => { + /** @type {SwingSetConfig} */ const config = { bootstrap: 'bootstrap', vats: { @@ -180,6 +181,7 @@ test('drop without retire', async t => { } // if (msgs.includes(o.type)) console.log(JSON.stringify(o)); } + /** @type {SwingSetConfig} */ const config = { bootstrap: 'bootstrap', // v6 vats: { diff --git a/packages/SwingSet/test/virtualObjects/weakcollections.test.js b/packages/SwingSet/test/virtualObjects/weakcollections.test.js index d6a401ff927..15cc04c8fd5 100644 --- a/packages/SwingSet/test/virtualObjects/weakcollections.test.js +++ b/packages/SwingSet/test/virtualObjects/weakcollections.test.js @@ -8,6 +8,7 @@ import { initializeSwingset, makeSwingsetController } from '../../src/index.js'; import makeNextLog from '../make-nextlog.js'; test('weakMap in vat', async t => { + /** @type {SwingSetConfig} */ const config = { includeDevDependencies: true, // for vat-data bootstrap: 'bootstrap', From 0ac88bd2623b66f3445943039ec8f57ee0f9127f Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Wed, 20 Nov 2024 14:18:12 -0500 Subject: [PATCH 2/2] refactor(SwingSet): Simplify some functions for comprehensibility and interactive debugging --- .../src/supervisors/supervisor-helper.js | 19 +++-- packages/SwingSet/tools/run-utils.js | 75 ++++++++----------- 2 files changed, 42 insertions(+), 52 deletions(-) diff --git a/packages/SwingSet/src/supervisors/supervisor-helper.js b/packages/SwingSet/src/supervisors/supervisor-helper.js index 434b7693d99..2050210b77d 100644 --- a/packages/SwingSet/src/supervisors/supervisor-helper.js +++ b/packages/SwingSet/src/supervisors/supervisor-helper.js @@ -32,16 +32,15 @@ function makeSupervisorDispatch(dispatch) { async function dispatchToVat(delivery) { // the (low-level) vat is responsible for giving up agency, but we still // protect against exceptions - return Promise.resolve(delivery) - .then(dispatch) - .then( - res => harden(['ok', res, null]), - err => { - // TODO react more thoughtfully, maybe terminate the vat - console.warn(`error during vat dispatch() of ${delivery}`, err); - return harden(['error', `${err}`, null]); - }, - ); + await null; + try { + const res = await dispatch(delivery); + return harden(['ok', res, null]); + } catch (err) { + // TODO react more thoughtfully, maybe terminate the vat + console.warn(`error during vat dispatch() of ${delivery}`, err); + return harden(['error', `${err}`, null]); + } } return harden(dispatchToVat); diff --git a/packages/SwingSet/tools/run-utils.js b/packages/SwingSet/tools/run-utils.js index bdb1a435390..a8d08ff1c00 100644 --- a/packages/SwingSet/tools/run-utils.js +++ b/packages/SwingSet/tools/run-utils.js @@ -53,10 +53,25 @@ export const makeRunUtils = (controller, harness) => { }; /** - * @typedef {import('@endo/eventual-send').EProxy & { - * sendOnly: (presence: unknown) => Record void>; - * vat: (name: string) => Record Promise>; - * }} EVProxy + * @typedef {import('@endo/eventual-send').EProxy | object} EVProxyMethods + * @property {(presence: unknown) => Record Promise>} sendOnly + * Returns a "methods proxy" for the presence that ignores the results of + * each method invocation. + * @property {(name: string) => Record Promise>} vat + * Returns a "methods proxy" for the root object of the specified vat. + * So e.g. `EV.vat('foo').m(0)` becomes + * `controller.queueToVatRoot('foo', 'm', [0])`. + * @property {(presence: unknown) => Record>} get + * Returns a "values proxy" for the presence for which each requested + * property manifests as a promise. + */ + /** + * @typedef {import('@endo/eventual-send').EProxy & EVProxyMethods} EVProxy + * Given a presence, return a "methods proxy" for which each requested + * property manifests as a method that forwards its invocation through the + * controller to the presence as an invocation of an identically-named method + * with identical arguments (modulo passable translation). + * So e.g. `EV(x).m(0)` becomes `controller.queueToVatObject(x, 'm', [0])`. */ // IMPORTANT WARNING TO USERS OF `EV` @@ -103,50 +118,26 @@ export const makeRunUtils = (controller, harness) => { // promise that can remain pending indefinitely, possibly to be settled by a // future message delivery. + const makeMethodsProxy = (invoker, target, voidResult = false) => + new Proxy(harden({}), { + get: (_t, method, _rx) => { + const boundMethod = (...args) => + queueAndRun(() => invoker(target, method, args), voidResult); + return harden(boundMethod); + }, + }); + /** @type {EVProxy} */ - // @ts-expect-error cast, approximate const EV = Object.assign( - presence => - new Proxy(harden({}), { - get: (_t, method, _rx) => { - const boundMethod = (...args) => - queueAndRun(() => - controller.queueToVatObject(presence, method, args), - ); - return harden(boundMethod); - }, - }), + presence => makeMethodsProxy(controller.queueToVatObject, presence), { - vat: vatName => - new Proxy(harden({}), { - get: (_t, method, _rx) => { - const boundMethod = (...args) => - queueAndRun(() => - controller.queueToVatRoot(vatName, method, args), - ); - return harden(boundMethod); - }, - }), + vat: vatName => makeMethodsProxy(controller.queueToVatRoot, vatName), sendOnly: presence => - new Proxy(harden({}), { - get: (_t, method, _rx) => { - const boundMethod = (...args) => - queueAndRun( - () => controller.queueToVatObject(presence, method, args), - true, - ); - return harden(boundMethod); - }, - }), + makeMethodsProxy(controller.queueToVatObject, presence, true), get: presence => new Proxy(harden({}), { - get: (_t, pathElement, _rx) => - queueAndRun(() => - controller.queueToVatRoot('bootstrap', 'awaitVatObject', [ - presence, - [pathElement], - ]), - ), + get: (_t, key, _rx) => + EV.vat('bootstrap').awaitVatObject(presence, [key]), }), }, );