From f2b07202260b3a4b484af2d5b7b1cd1828c906f7 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Mon, 6 Jan 2025 22:19:35 -0500 Subject: [PATCH 1/2] refactor(SwingSet): Cleanup vatManagerFactory * Warn on any use of `setup` or `enableSetup` that overrides `type`. * Improve error message detail. * Collapse together all typed vat factory invocations. --- .../src/kernel/vat-loader/manager-factory.js | 67 +++++++------------ 1 file changed, 24 insertions(+), 43 deletions(-) diff --git a/packages/SwingSet/src/kernel/vat-loader/manager-factory.js b/packages/SwingSet/src/kernel/vat-loader/manager-factory.js index cb1b28c68c9..2f4a35e5727 100644 --- a/packages/SwingSet/src/kernel/vat-loader/manager-factory.js +++ b/packages/SwingSet/src/kernel/vat-loader/manager-factory.js @@ -18,14 +18,12 @@ export function makeVatManagerFactory({ vatEndowments, gcTools, }); - const nodeSubprocessFactory = makeNodeSubprocessFactory({ startSubprocessWorker: startSubprocessWorkerNode, kernelKeeper, kernelSlog, testLog: allVatPowers.testLog, }); - const xsWorkerFactory = makeXsSubprocessFactory({ startXSnap, kernelKeeper, @@ -69,57 +67,40 @@ export function makeVatManagerFactory({ * @param {import('@agoric/swingset-liveslots').LiveSlotsOptions} options.liveSlotsOptions * @returns { Promise } */ - async function vatManagerFactory( - vatID, - { managerOptions, liveSlotsOptions }, - ) { + async function vatManagerFactory(vatID, options) { + const { managerOptions, liveSlotsOptions } = options; validateManagerOptions(managerOptions); const { workerOptions, enableSetup } = managerOptions; const { type } = workerOptions; - if (type !== 'local' && 'setup' in managerOptions) { - console.warn(`TODO: stop using setup() with ${type}`); - } - if (enableSetup) { - if (managerOptions.setup) { - return localFactory.createFromSetup(vatID, managerOptions); - } else { - return localFactory.createFromBundle( - vatID, - managerOptions.bundle, - managerOptions, - liveSlotsOptions, - ); - } - } else if (type === 'local') { - return localFactory.createFromBundle( - vatID, - managerOptions.bundle, - managerOptions, - liveSlotsOptions, - ); + if (type !== 'local' && (enableSetup || 'setup' in managerOptions)) { + console.warn(`TODO: stop using enableSetup and setup() with ${type}`); } - if (type === 'node-subprocess') { - return nodeSubprocessFactory.createFromBundle( - vatID, - managerOptions.bundle, - managerOptions, - liveSlotsOptions, - ); + if (enableSetup && managerOptions.setup) { + return localFactory.createFromSetup(vatID, managerOptions); } - if (type === 'xsnap') { - assert(managerOptions.bundle, 'xsnap requires Bundle'); - return xsWorkerFactory.createFromBundle( - vatID, - managerOptions.bundle, - managerOptions, - liveSlotsOptions, - ); + /** @type {Pick} */ + let factory; + if (enableSetup || type === 'local') { + factory = localFactory; + } else if (type === 'node-subprocess') { + factory = nodeSubprocessFactory; + } else if (type === 'xsnap') { + assert(managerOptions.bundle, 'worker type xsnap requires a bundle'); + factory = xsWorkerFactory; + } else { + throw Error(`unknown vat worker type ${type}`); } - throw Error(`unknown type ${type}, not 'local' or 'xsnap'`); + return factory.createFromBundle( + vatID, + // @ts-expect-error managerOptions.bundle might be undefined + managerOptions.bundle, + managerOptions, + liveSlotsOptions, + ); } return harden(vatManagerFactory); From 570fa57a874ba08f158948f6e8c64e2fa9510d07 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Mon, 6 Jan 2025 22:26:31 -0500 Subject: [PATCH 2/2] refactor(SwingSet): Rename "vatManagerFactory" to the more precise "makeVatManager" --- packages/SwingSet/src/kernel/kernel.js | 6 +++--- packages/SwingSet/src/kernel/vat-loader/manager-factory.js | 6 +++--- packages/SwingSet/src/kernel/vat-loader/vat-loader.js | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js index cd20086dde7..726043e5d7a 100644 --- a/packages/SwingSet/src/kernel/kernel.js +++ b/packages/SwingSet/src/kernel/kernel.js @@ -9,7 +9,7 @@ import { makeUpgradeDisconnection } from '@agoric/internal/src/upgrade-api.js'; import { kser, kslot, makeError } from '@agoric/kmarshal'; import { assertKnownOptions } from '../lib/assertOptions.js'; import { foreverPolicy } from '../lib/runPolicies.js'; -import { makeVatManagerFactory } from './vat-loader/manager-factory.js'; +import { makeVatManagerMaker } from './vat-loader/manager-factory.js'; import { makeVatWarehouse } from './vat-warehouse.js'; import makeDeviceManager from './deviceManager.js'; import makeKernelKeeper, { @@ -1557,7 +1557,7 @@ export default function buildKernel( gcAndFinalize, meterControl: makeDummyMeterControl(), }); - const vatManagerFactory = makeVatManagerFactory({ + const makeVatManager = makeVatManagerMaker({ allVatPowers, kernelKeeper, vatEndowments, @@ -1652,7 +1652,7 @@ export default function buildKernel( } const vatLoader = makeVatLoader({ - vatManagerFactory, + makeVatManager, kernelSlog, makeSourcedConsole, kernelKeeper, diff --git a/packages/SwingSet/src/kernel/vat-loader/manager-factory.js b/packages/SwingSet/src/kernel/vat-loader/manager-factory.js index 2f4a35e5727..e0cd554cb91 100644 --- a/packages/SwingSet/src/kernel/vat-loader/manager-factory.js +++ b/packages/SwingSet/src/kernel/vat-loader/manager-factory.js @@ -4,7 +4,7 @@ import { makeLocalVatManagerFactory } from './manager-local.js'; import { makeNodeSubprocessFactory } from './manager-subprocess-node.js'; import { makeXsSubprocessFactory } from './manager-subprocess-xsnap.js'; -export function makeVatManagerFactory({ +export function makeVatManagerMaker({ allVatPowers, kernelKeeper, vatEndowments, @@ -67,7 +67,7 @@ export function makeVatManagerFactory({ * @param {import('@agoric/swingset-liveslots').LiveSlotsOptions} options.liveSlotsOptions * @returns { Promise } */ - async function vatManagerFactory(vatID, options) { + async function makeVatManager(vatID, options) { const { managerOptions, liveSlotsOptions } = options; validateManagerOptions(managerOptions); const { workerOptions, enableSetup } = managerOptions; @@ -103,5 +103,5 @@ export function makeVatManagerFactory({ ); } - return harden(vatManagerFactory); + return harden(makeVatManager); } diff --git a/packages/SwingSet/src/kernel/vat-loader/vat-loader.js b/packages/SwingSet/src/kernel/vat-loader/vat-loader.js index c67fb97fa9c..c862bfe0c9a 100644 --- a/packages/SwingSet/src/kernel/vat-loader/vat-loader.js +++ b/packages/SwingSet/src/kernel/vat-loader/vat-loader.js @@ -9,7 +9,7 @@ export function makeVatRootObjectSlot() { export function makeVatLoader(stuff) { const { overrideVatManagerOptions = {}, - vatManagerFactory, + makeVatManager, kernelSlog, makeSourcedConsole, kernelKeeper, @@ -127,7 +127,7 @@ export function makeVatLoader(stuff) { }; const finish = starting && kernelSlog.startup(vatID); - const manager = await vatManagerFactory(vatID, { + const manager = await makeVatManager(vatID, { managerOptions, liveSlotsOptions, }); @@ -150,7 +150,7 @@ export function makeVatLoader(stuff) { ...overrideVatManagerOptions, }; const liveSlotsOptions = {}; - const manager = await vatManagerFactory(vatID, { + const manager = await makeVatManager(vatID, { managerOptions, liveSlotsOptions, });