diff --git a/packages/kernel/src/Kernel.test.ts b/packages/kernel/src/Kernel.test.ts index 21e7eaf99..6c797ae31 100644 --- a/packages/kernel/src/Kernel.test.ts +++ b/packages/kernel/src/Kernel.test.ts @@ -148,17 +148,56 @@ describe('Kernel', () => { }); describe('restartVat()', () => { - // Disabling this test for now, as vat restart is not currently a thing - it.todo('restarts a vat', async () => { + it('preserves vat state across multiple restarts', async () => { + const kernel = new Kernel(mockStream, mockWorkerService, mockKVStore); + await kernel.launchVat(mockVatConfig); + await kernel.restartVat('v1'); + expect(kernel.getVatIds()).toStrictEqual(['v1']); + await kernel.restartVat('v1'); + expect(kernel.getVatIds()).toStrictEqual(['v1']); + expect(terminateMock).toHaveBeenCalledTimes(2); + expect(launchWorkerMock).toHaveBeenCalledTimes(3); // initial + 2 restarts + expect(launchWorkerMock).toHaveBeenLastCalledWith('v1', mockVatConfig); + }); + + it('restarts a vat', async () => { const kernel = new Kernel(mockStream, mockWorkerService, mockKVStore); await kernel.launchVat(mockVatConfig); expect(kernel.getVatIds()).toStrictEqual(['v1']); await kernel.restartVat('v1'); expect(terminateMock).toHaveBeenCalledOnce(); expect(terminateWorkerMock).toHaveBeenCalledOnce(); + expect(launchWorkerMock).toHaveBeenCalledTimes(2); + expect(launchWorkerMock).toHaveBeenLastCalledWith('v1', mockVatConfig); expect(kernel.getVatIds()).toStrictEqual(['v1']); expect(initMock).toHaveBeenCalledTimes(2); }); + + it('throws error when restarting non-existent vat', async () => { + const kernel = new Kernel(mockStream, mockWorkerService, mockKVStore); + await expect(kernel.restartVat('v999')).rejects.toThrow(VatNotFoundError); + expect(terminateMock).not.toHaveBeenCalled(); + expect(launchWorkerMock).not.toHaveBeenCalled(); + }); + + it('handles restart failure during termination', async () => { + const kernel = new Kernel(mockStream, mockWorkerService, mockKVStore); + await kernel.launchVat(mockVatConfig); + terminateMock.mockRejectedValueOnce(new Error('Termination failed')); + await expect(kernel.restartVat('v1')).rejects.toThrow( + 'Termination failed', + ); + expect(launchWorkerMock).toHaveBeenCalledTimes(1); + }); + + it('handles restart failure during launch', async () => { + const kernel = new Kernel(mockStream, mockWorkerService, mockKVStore); + await kernel.launchVat(mockVatConfig); + launchWorkerMock.mockRejectedValueOnce(new Error('Launch failed')); + await expect(kernel.restartVat('v1')).rejects.toThrow('Launch failed'); + expect(terminateMock).toHaveBeenCalledOnce(); + expect(kernel.getVatIds()).toStrictEqual([]); + }); }); describe('sendMessage()', () => { diff --git a/packages/kernel/src/Kernel.ts b/packages/kernel/src/Kernel.ts index eb16c6e4a..4d11dfdc5 100644 --- a/packages/kernel/src/Kernel.ts +++ b/packages/kernel/src/Kernel.ts @@ -30,6 +30,7 @@ import type { ClusterConfig, VatConfig, } from './types.js'; +import { VatStateService } from './vat-state-service.js'; import { Vat } from './Vat.js'; export class Kernel { @@ -43,6 +44,8 @@ export class Kernel { readonly #logger: Logger; + readonly #vatStateService: VatStateService; + constructor( stream: DuplexStream, vatWorkerService: VatWorkerService, @@ -54,6 +57,7 @@ export class Kernel { this.#vatWorkerService = vatWorkerService; this.#storage = makeKernelStore(rawStorage); this.#logger = logger ?? makeLogger('[ocap kernel]'); + this.#vatStateService = new VatStateService(); } async init(): Promise { @@ -68,80 +72,6 @@ export class Kernel { }); } - async #receiveMessages(): Promise { - for await (const message of this.#stream) { - if (!isKernelCommand(message)) { - this.#logger.error('Received unexpected message', message); - continue; - } - - const { method, params } = message; - - let vat: Vat; - - switch (method) { - case KernelCommandMethod.ping: - await this.#reply({ method, params: 'pong' }); - break; - case KernelCommandMethod.evaluate: - if (!this.#vats.size) { - throw new Error('No vats available to call'); - } - vat = this.#vats.values().next().value as Vat; - await this.#reply({ - method, - params: await this.evaluate(vat.vatId, params), - }); - break; - case KernelCommandMethod.capTpCall: - if (!this.#vats.size) { - throw new Error('No vats available to call'); - } - vat = this.#vats.values().next().value as Vat; - await this.#reply({ - method, - params: stringify(await vat.callCapTp(params)), - }); - break; - case KernelCommandMethod.kvSet: - this.kvSet(params.key, params.value); - await this.#reply({ - method, - params: `~~~ set "${params.key}" to "${params.value}" ~~~`, - }); - break; - case KernelCommandMethod.kvGet: { - try { - const value = this.kvGet(params); - const result = - typeof value === 'string' ? `"${value}"` : `${value}`; - await this.#reply({ - method, - params: `~~~ got ${result} ~~~`, - }); - } catch (problem) { - // TODO: marshal - await this.#reply({ - method, - params: String(toError(problem)), - }); - } - break; - } - default: - console.error( - 'kernel worker received unexpected command', - // @ts-expect-error Runtime does not respect "never". - { method: method.valueOf(), params }, - ); - } - } - } - - async #reply(message: KernelCommandReply): Promise { - await this.#stream.write(message); - } - /** * Evaluate a string in the default iframe. * @@ -192,17 +122,7 @@ export class Kernel { if (this.#vats.has(vatId)) { throw new VatAlreadyExistsError(vatId); } - const multiplexer = await this.#vatWorkerService.launch(vatId, vatConfig); - multiplexer.start().catch((error) => this.#logger.error(error)); - const commandStream = multiplexer.createChannel< - VatCommandReply, - VatCommand - >('command', isVatCommandReply); - const capTpStream = multiplexer.createChannel('capTp'); - const vat = new Vat({ vatId, vatConfig, commandStream, capTpStream }); - this.#vats.set(vat.vatId, vat); - await vat.init(); - return vat; + return this.#initVat(vatId, vatConfig); } /** @@ -223,14 +143,18 @@ export class Kernel { /** * Restarts a vat. * - * @param id - The ID of the vat. + * @param vatId - The ID of the vat. + * @returns A promise that resolves the restarted vat. */ - async restartVat(id: VatId): Promise { - await this.terminateVat(id); - // XXX TODO the following line has been hacked up to enable a successful - // build, but is entirely wrong. Restart expressed this way loses the original vat - // ID and configuration. - await this.launchVat({ sourceSpec: 'not-really-there.js' }); + async restartVat(vatId: VatId): Promise { + const state = this.#vatStateService.get(vatId); + if (!state) { + throw new VatNotFoundError(vatId); + } + + await this.terminateVat(vatId); + const vat = await this.#initVat(vatId, state.config); + return vat; } /** @@ -274,6 +198,116 @@ export class Kernel { return vat.sendMessage(command); } + // -------------------------------------------------------------------------- + // Private methods + // -------------------------------------------------------------------------- + + /** + * Receives messages from the stream. + */ + async #receiveMessages(): Promise { + for await (const message of this.#stream) { + if (!isKernelCommand(message)) { + this.#logger.error('Received unexpected message', message); + continue; + } + + const { method, params } = message; + + let vat: Vat; + + switch (method) { + case KernelCommandMethod.ping: + await this.#reply({ method, params: 'pong' }); + break; + case KernelCommandMethod.evaluate: + if (!this.#vats.size) { + throw new Error('No vats available to call'); + } + vat = this.#vats.values().next().value as Vat; + await this.#reply({ + method, + params: await this.evaluate(vat.vatId, params), + }); + break; + case KernelCommandMethod.capTpCall: + if (!this.#vats.size) { + throw new Error('No vats available to call'); + } + vat = this.#vats.values().next().value as Vat; + await this.#reply({ + method, + params: stringify(await vat.callCapTp(params)), + }); + break; + case KernelCommandMethod.kvSet: + this.kvSet(params.key, params.value); + await this.#reply({ + method, + params: `~~~ set "${params.key}" to "${params.value}" ~~~`, + }); + break; + case KernelCommandMethod.kvGet: { + try { + const value = this.kvGet(params); + const result = + typeof value === 'string' ? `"${value}"` : `${value}`; + await this.#reply({ + method, + params: `~~~ got ${result} ~~~`, + }); + } catch (problem) { + // TODO: marshal + await this.#reply({ + method, + params: String(toError(problem)), + }); + } + break; + } + default: + console.error( + 'kernel worker received unexpected command', + // @ts-expect-error Runtime does not respect "never". + { method: method.valueOf(), params }, + ); + } + } + } + + /** + * Replies to a message. + * + * @param message - The message to reply to. + */ + async #reply(message: KernelCommandReply): Promise { + await this.#stream.write(message); + } + + /** + * Initializes a vat. + * + * @param vatId - The ID of the vat. + * @param vatConfig - The configuration of the vat. + * @returns A promise that resolves the vat. + */ + async #initVat(vatId: VatId, vatConfig: VatConfig): Promise { + const multiplexer = await this.#vatWorkerService.launch(vatId, vatConfig); + multiplexer.start().catch((error) => this.#logger.error(error)); + const commandStream = multiplexer.createChannel< + VatCommandReply, + VatCommand + >('command', isVatCommandReply); + const capTpStream = multiplexer.createChannel('capTp'); + const vat = new Vat({ vatId, vatConfig, commandStream, capTpStream }); + this.#vats.set(vat.vatId, vat); + this.#vatStateService.set(vatId, { + config: vatConfig, + }); + await vat.init(); + return vat; + } + /** * Gets a vat. * diff --git a/packages/kernel/src/types.ts b/packages/kernel/src/types.ts index 100303415..dadb0826f 100644 --- a/packages/kernel/src/types.ts +++ b/packages/kernel/src/types.ts @@ -47,7 +47,7 @@ type EndpointState = { kRefToERef: Map; }; -type VatState = { +type KernelVatState = { messagePort: typeof MessagePort; state: EndpointState; source: string; @@ -72,7 +72,7 @@ export type KernelPromise = { }; export type KernelState = { - vats: Map; + vats: Map; remotes: Map; kernelPromises: Map; }; @@ -176,9 +176,10 @@ export const VatConfigStruct = define('VatConfig', (value) => { return false; } - const { sourceSpec, bundleSpec, bundleName, creationOptions, parameters } = - value as Record; - const specOnly = { sourceSpec, bundleSpec, bundleName }; + const { creationOptions, parameters, ...specOnly } = value as Record< + string, + unknown + >; return ( is(specOnly, UserCodeSpecStruct) && diff --git a/packages/kernel/src/vat-state-service.test.ts b/packages/kernel/src/vat-state-service.test.ts new file mode 100644 index 000000000..4f92458bf --- /dev/null +++ b/packages/kernel/src/vat-state-service.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect, beforeEach } from 'vitest'; + +import type { VatId, VatConfig } from './types'; +import { VatStateService } from './vat-state-service'; + +describe('VatStateService', () => { + let vatStateService: VatStateService; + const mockVatId: VatId = 'v1'; + const mockVatConfig: VatConfig = { sourceSpec: 'test-vat.js' }; + const mockVatState = { config: mockVatConfig }; + + beforeEach(() => { + vatStateService = new VatStateService(); + }); + + describe('set', () => { + it('should store valid vat state', () => { + vatStateService.set(mockVatId, mockVatState); + expect(vatStateService.get(mockVatId)).toStrictEqual(mockVatState); + }); + + it('should overwrite existing state', () => { + const newState = { config: { sourceSpec: 'new.js' } }; + vatStateService.set(mockVatId, mockVatState); + vatStateService.set(mockVatId, newState); + expect(vatStateService.get(mockVatId)).toStrictEqual(newState); + }); + }); + + describe('get', () => { + it('should return undefined for non-existent vat', () => { + expect(vatStateService.get('v999')).toBeUndefined(); + }); + + it('should return correct state for existing vat', () => { + vatStateService.set(mockVatId, mockVatState); + expect(vatStateService.get(mockVatId)).toStrictEqual(mockVatState); + }); + }); + + describe('delete', () => { + it('should return true when deleting existing state', () => { + vatStateService.set(mockVatId, mockVatState); + expect(vatStateService.delete(mockVatId)).toBe(true); + expect(vatStateService.get(mockVatId)).toBeUndefined(); + }); + + it('should return false when deleting non-existent state', () => { + expect(vatStateService.delete('v999')).toBe(false); + }); + }); + + describe('has', () => { + it('should return true for existing vat', () => { + vatStateService.set(mockVatId, mockVatState); + expect(vatStateService.has(mockVatId)).toBe(true); + }); + + it('should return false for non-existent vat', () => { + expect(vatStateService.has('v999')).toBe(false); + }); + }); + + describe('vatIds', () => { + it('should return all vat IDs', () => { + vatStateService.set('v1', mockVatState); + vatStateService.set('v2', mockVatState); + expect(vatStateService.vatIds).toStrictEqual(['v1', 'v2']); + }); + + it('should return empty array when no states exist', () => { + expect(vatStateService.vatIds).toStrictEqual([]); + }); + }); + + describe('size', () => { + it('should return correct number of stored states', () => { + expect(vatStateService.size).toBe(0); + vatStateService.set('v1', mockVatState); + expect(vatStateService.size).toBe(1); + vatStateService.set('v2', mockVatState); + expect(vatStateService.size).toBe(2); + vatStateService.delete('v1'); + expect(vatStateService.size).toBe(1); + }); + }); +}); diff --git a/packages/kernel/src/vat-state-service.ts b/packages/kernel/src/vat-state-service.ts new file mode 100644 index 000000000..7d7a020c0 --- /dev/null +++ b/packages/kernel/src/vat-state-service.ts @@ -0,0 +1,72 @@ +import type { VatId, VatConfig } from './types.js'; + +export type VatState = { + config: VatConfig; +}; + +export class VatStateService { + readonly #states: Map; + + constructor() { + this.#states = new Map(); + } + + /** + * Set the state for a vat. + * + * @param vatId - The ID of the vat. + * @param state - The state to set. + * @throws {Error} If state is invalid. + */ + set(vatId: VatId, state: VatState): void { + this.#states.set(vatId, state); + } + + /** + * Get the state of a vat. + * + * @param vatId - The ID of the vat. + * @returns The vat state, or undefined if not found. + */ + get(vatId: VatId): VatState | undefined { + return this.#states.get(vatId); + } + + /** + * Delete the state of a vat. + * + * @param vatId - The ID of the vat. + * @returns true if state was deleted, false if it didn't exist. + */ + delete(vatId: VatId): boolean { + return this.#states.delete(vatId); + } + + /** + * Check if a vat has state stored. + * + * @param vatId - The ID of the vat. + * @returns true if state exists for the vat. + */ + has(vatId: VatId): boolean { + return this.#states.has(vatId); + } + + /** + * Get all vat IDs with stored state. + * + * @returns Array of vat IDs. + */ + get vatIds(): VatId[] { + return Array.from(this.#states.keys()); + } + + /** + * Get number of vats with stored state. + * + * @returns Number of vats. + */ + get size(): number { + return this.#states.size; + } +} diff --git a/vitest.config.ts b/vitest.config.ts index 5b0d65448..215fca277 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -47,10 +47,10 @@ export default defineConfig({ lines: 57.36, }, 'packages/kernel/**': { - statements: 77.4, - functions: 86.9, - branches: 64.22, - lines: 77.62, + statements: 78.74, + functions: 89.01, + branches: 64.86, + lines: 78.96, }, 'packages/shims/**': { statements: 0,