Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion packages/extension/test/e2e/vat-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,47 @@ test.describe('Vat Manager', () => {
await popupPage.click('button:text("Terminate All Vats")');
await expect(messageOutput).toContainText('All vats terminated');
await expect(popupPage.locator('table')).not.toBeVisible();
// ensure all references were garbage collected
await popupPage.locator('[data-testid="clear-logs-button"]').click();
await expect(messageOutput).toContainText('');
await popupPage.click('button:text("Database Inspector")');
const expectedValues = JSON.stringify([
{ key: 'queue.run.head', value: '6' },
{ key: 'queue.run.tail', value: '6' },
{ key: 'gcActions', value: '[]' },
{ key: 'reapQueue', value: '[]' },
{ key: 'vats.terminated', value: '[]' },
{ key: 'nextObjectId', value: '4' },
{ key: 'nextPromiseId', value: '4' },
{ key: 'nextVatId', value: '4' },
{ key: 'nextRemoteId', value: '1' },
{ key: 'initialized', value: 'true' },
]);
await expect(messageOutput).toContainText(expectedValues);
});

test('should clear kernel state', async () => {
await popupPage.click('button:text("Clear All State")');
await expect(messageOutput).toContainText('State cleared');
await expect(popupPage.locator('table')).not.toBeVisible();
// ensure kernel state was cleared
await popupPage.locator('[data-testid="clear-logs-button"]').click();
await expect(messageOutput).toContainText('');
await popupPage.click('button:text("Database Inspector")');
const expectedValues = JSON.stringify([
{ key: 'queue.run.head', value: '1' },
{ key: 'queue.run.tail', value: '1' },
{ key: 'gcActions', value: '[]' },
{ key: 'reapQueue', value: '[]' },
{ key: 'vats.terminated', value: '[]' },
{ key: 'nextObjectId', value: '1' },
{ key: 'nextPromiseId', value: '1' },
{ key: 'nextVatId', value: '1' },
{ key: 'nextRemoteId', value: '1' },
]);
await expect(messageOutput).toContainText(expectedValues);
await expect(messageOutput).not.toContainText('"initialized":true');
await popupPage.click('button:text("Vat Manager")');
await launchVat('test-vat-new');
await expect(popupPage.locator('table tr')).toHaveCount(2);
});
Expand Down Expand Up @@ -277,11 +312,11 @@ test.describe('Vat Manager', () => {
'{"key":"v3.c.o+0","value":"ko3"}',
'{"key":"v3.c.kp3","value":"R p-1"}',
'{"key":"v3.c.p-1","value":"kp3"}',
'{"key":"ko3.refCount","value":"1,1"}',
];
const v1ko3Values = [
'{"key":"v1.c.ko3","value":"R o-2"}',
'{"key":"v1.c.o-2","value":"ko3"}',
'{"key":"ko3.refCount","value":"2,2"}',
'{"key":"kp3.state","value":"fulfilled"}',
'{"key":"kp3.value","value"',
];
Expand Down
2 changes: 2 additions & 0 deletions packages/extension/vite-plugins/extension-dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export function extensionDev({
// Launch Chrome with the extension loaded
const browserContext = await chromium.launchPersistentContext('', {
headless: false,
viewport: null, // Let the OS window control the size
ignoreDefaultArgs: ['--enable-automation'],
args: [
`--disable-extensions-except=${extensionPath}`,
Expand Down Expand Up @@ -87,6 +88,7 @@ export function extensionDev({

// Open the extensions page for our extension
const extensionsPage = await browserContext.newPage();
await extensionsPage.setViewportSize({ width: 1300, height: 700 });
await extensionsPage.goto(`chrome://extensions/?id=${extensionId}`);

// Update state after all checks
Expand Down
2 changes: 1 addition & 1 deletion packages/kernel-test/src/garbage-collection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('Garbage Collection', () => {
// Check that the object is reachable as a promise from the importer vat
const importerKref = kernelStore.erefToKref(importerVatId, 'p-1') as KRef;
expect(kernelStore.hasCListEntry(importerVatId, importerKref)).toBe(true);
expect(kernelStore.getRefCount(importerKref)).toBe(2);
expect(kernelStore.getRefCount(importerKref)).toBe(1);
// Use the object
const useResult = await kernel.queueMessageFromKernel(
importerKRef,
Expand Down
82 changes: 41 additions & 41 deletions packages/kernel/src/Kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ export class Kernel {
await this.#runVat(vatId, vatConfig);
this.#kernelStore.initEndpoint(vatId);
const rootRef = this.#kernelStore.exportFromVat(vatId, ROOT_OBJECT_VREF);
this.#kernelStore.incrementRefCount(rootRef, 'root');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to pin every root ref (in fact, we absolutely shouldn't) but something like this has to happen somewhere (perhaps launchSubcluster?) to ensure that references (e.g., to bootstrap vats) that are only held externally (e.g., by the control panel or test scripts) but never explicitly exported via a message parameter are counted, else one's whole subcluster could simply disappear after a GC. (Speaking cosmologically, the universe as a whole is unreferenced.). This can sneak up on you (ask me how I know :-)

Copy link
Contributor Author

@sirtimid sirtimid Apr 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pinky swear that a whole subcluster can't disappear except if one deletes the bootstrap vat after they have deleted all the other vats and have garbage collected them. Shouldn't we support this process? One can't just delete all vats (with the bootstrap) and garbage collect them, the references brake. In other words: the requirement that you retire the secondary vats before you retire the bootstrap is what guarantees safety imo.

Anyway I suggest we do this on another "add removable bootstrap keep‑alive pin" task and perhaps follow agorics convention of pin/unpin. wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that that sounds like the right plan to me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, subtle point:
It's not that the vat itself gets GC'd, it's that the vat's root object gets GC'd, so unless the vat has in the meantime wired itself up with other vats you lose the ability to talk to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And.. done #494

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, as a general rule, GC'ing a vat's root object while the rest of the vat continues to run is not a problem. At Agoric, it was not uncommon to create a vat with an ephemeral root object whose only job was to initialize things and then go away.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha! I didn't expect that, but, yes of course, why not!

this.#kernelStore.setVatConfig(vatId, vatConfig);
return rootRef;
}
Expand Down Expand Up @@ -313,30 +312,6 @@ export class Kernel {
this.#kernelStore.markVatAsTerminated(vatId);
}

/**
* Terminate all vats.
*/
async terminateAllVats(): Promise<void> {
await Promise.all(
this.getVatIds().map(async (id) => {
await this.terminateVat(id);
}),
);
}

/**
* Terminate all running vats and reload the default subcluster.
* This is for debugging purposes only.
*/
async reload(): Promise<void> {
if (!this.#mostRecentSubcluster) {
throw Error('no subcluster to reload');
}
await this.terminateAllVats();
this.collectGarbage();
await this.launchSubcluster(this.#mostRecentSubcluster);
}

/**
* Clear the database.
*/
Expand Down Expand Up @@ -367,22 +342,6 @@ export class Kernel {
return vat.sendVatCommand({ method, params });
}

/**
* Stop all running vats and reset the kernel state.
*/
async reset(): Promise<void> {
await this.terminateAllVats();
this.#resetKernelState();
}

/**
* Reset the kernel state.
*/
#resetKernelState(): void {
this.#kernelStore.clear();
this.#kernelStore.reset();
}

/**
* Get a vat.
*
Expand Down Expand Up @@ -455,6 +414,47 @@ export class Kernel {
}
}

/**
* Reset the kernel state.
* This is for debugging purposes only.
*/
#resetKernelState(): void {
this.#kernelStore.clear();
this.#kernelStore.reset();
}

/**
* Stop all running vats and reset the kernel state.
* This is for debugging purposes only.
*/
async reset(): Promise<void> {
await this.terminateAllVats();
this.#resetKernelState();
}

/**
* Terminate all vats and collect garbage.
* This is for debugging purposes only.
*/
async terminateAllVats(): Promise<void> {
for (const id of this.getVatIds().reverse()) {
await this.terminateVat(id);
this.collectGarbage();
}
}

/**
* Terminate all running vats and reload the default subcluster.
* This is for debugging purposes only.
*/
async reload(): Promise<void> {
if (!this.#mostRecentSubcluster) {
throw Error('no subcluster to reload');
}
await this.terminateAllVats();
await this.launchSubcluster(this.#mostRecentSubcluster);
}

/**
* Collect garbage.
* This is for debugging purposes only.
Expand Down
55 changes: 45 additions & 10 deletions packages/kernel/src/KernelQueue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import type {
Message,
RunQueueItem,
RunQueueItemNotify,
RunQueueItemSend,
} from './types.ts';

vi.mock('./services/garbage-collection.ts', () => ({
Expand Down Expand Up @@ -118,18 +117,54 @@ describe('KernelQueue', () => {
});
});

describe('enqueueRun', () => {
it('adds an item to the run queue', () => {
const item: RunQueueItemSend = {
describe('enqueueSend', () => {
it('enqueues a send message and increments reference counts', () => {
const target = 'ko123';
const message: Message = {
methargs: { body: 'method args', slots: ['slot1', 'slot2'] },
result: 'kp456',
};
kernelQueue.enqueueSend(target, message);
expect(kernelStore.incrementRefCount).toHaveBeenCalledWith(
target,
'queue|target',
);
expect(kernelStore.incrementRefCount).toHaveBeenCalledWith(
message.result,
'queue|result',
);
expect(kernelStore.incrementRefCount).toHaveBeenCalledWith(
'slot1',
'queue|slot',
);
expect(kernelStore.incrementRefCount).toHaveBeenCalledWith(
'slot2',
'queue|slot',
);
expect(kernelStore.enqueueRun).toHaveBeenCalledWith({
type: 'send',
target: 'ko123',
message: {} as Message,
target,
message,
});
});

it('handles messages without result or slots', () => {
const target = 'ko123';
const message: Message = {
methargs: { body: 'method args', slots: [] },
result: null,
};
(kernelStore.runQueueLength as unknown as MockInstance).mockReturnValue(
0,
kernelQueue.enqueueSend(target, message);
expect(kernelStore.incrementRefCount).toHaveBeenCalledTimes(1);
expect(kernelStore.incrementRefCount).toHaveBeenCalledWith(
target,
'queue|target',
);
kernelQueue.enqueueRun(item);
expect(kernelStore.enqueueRun).toHaveBeenCalledWith(item);
expect(kernelStore.enqueueRun).toHaveBeenCalledWith({
type: 'send',
target,
message,
});
});
});

Expand Down
55 changes: 32 additions & 23 deletions packages/kernel/src/KernelQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,20 @@ export class KernelQueue {
}
}

/**
* Add an item to the tail of the kernel's run queue.
*
* @param item - The item to add.
*/
#enqueueRun(item: RunQueueItem): void {
this.#kernelStore.enqueueRun(item);
if (this.#kernelStore.runQueueLength() === 1 && this.#wakeUpTheRunQueue) {
const wakeUpTheRunQueue = this.#wakeUpTheRunQueue;
this.#wakeUpTheRunQueue = null;
wakeUpTheRunQueue();
}
}

/**
* Queue a message to be delivered from the kernel to an object in a vat.
*
Expand All @@ -105,40 +119,35 @@ export class KernelQueue {
args: unknown[],
): Promise<CapData<KRef>> {
const result = this.#kernelStore.initKernelPromise()[0];
const message: Message = {
const { promise, resolve } = makePromiseKit<CapData<KRef>>();
this.subscriptions.set(result, resolve);
this.enqueueSend(target, {
methargs: kser([method, args]),
result,
};
});
return promise;
}

/**
* Enqueue a send message to be delivered to a vat.
*
* @param target - The object to which the message is directed.
* @param message - The message to be delivered.
*/
enqueueSend(target: KRef, message: Message): void {
this.#kernelStore.incrementRefCount(target, 'queue|target');
this.#kernelStore.incrementRefCount(result, 'queue|result');
if (message.result) {
this.#kernelStore.incrementRefCount(message.result, 'queue|result');
}
for (const slot of message.methargs.slots || []) {
this.#kernelStore.incrementRefCount(slot, 'queue|slot');
}

const queueItem: RunQueueItemSend = {
type: 'send',
target,
message,
};
const { promise, resolve } = makePromiseKit<CapData<KRef>>();
this.subscriptions.set(result, resolve);
this.enqueueRun(queueItem);
return promise;
}

/**
* Add an item to the tail of the kernel's run queue.
*
* @param item - The item to add.
*/
enqueueRun(item: RunQueueItem): void {
this.#kernelStore.enqueueRun(item);
if (this.#kernelStore.runQueueLength() === 1 && this.#wakeUpTheRunQueue) {
const wakeUpTheRunQueue = this.#wakeUpTheRunQueue;
this.#wakeUpTheRunQueue = null;
wakeUpTheRunQueue();
}
this.#enqueueRun(queueItem);
}

/**
Expand All @@ -150,7 +159,7 @@ export class KernelQueue {
*/
enqueueNotify(vatId: VatId, kpid: KRef): void {
const notifyItem: RunQueueItemNotify = { type: 'notify', vatId, kpid };
this.enqueueRun(notifyItem);
this.#enqueueRun(notifyItem);
// Increment reference count for the promise being notified about
this.#kernelStore.incrementRefCount(kpid, 'notify');
}
Expand Down
6 changes: 1 addition & 5 deletions packages/kernel/src/KernelRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,7 @@ export class KernelRouter {
message,
);
await vat.deliverMessage(vatTarget, vatMessage);
// decrement refcount for processed 'send' items except for the root object
const vatKref = this.#kernelStore.erefToKref(vatId, 'o+0');
if (vatKref !== target) {
this.#kernelStore.decrementRefCount(target, 'deliver|send|target');
}
this.#kernelStore.decrementRefCount(target, 'deliver|send|target');
for (const slot of message.methargs.slots) {
this.#kernelStore.decrementRefCount(slot, 'deliver|send|slot');
}
Expand Down
8 changes: 2 additions & 6 deletions packages/kernel/src/VatSyscall.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('VatSyscall', () => {

beforeEach(() => {
kernelQueue = {
enqueueRun: vi.fn(),
enqueueSend: vi.fn(),
resolvePromises: vi.fn(),
enqueueNotify: vi.fn(),
} as unknown as KernelQueue;
Expand All @@ -40,11 +40,7 @@ describe('VatSyscall', () => {
const message = {} as unknown as Message;
const vso = ['send', target, message] as unknown as VatSyscallObject;
await vatSys.handleSyscall(vso);
expect(kernelQueue.enqueueRun).toHaveBeenCalledWith({
type: 'send',
target,
message,
});
expect(kernelQueue.enqueueSend).toHaveBeenCalledWith(target, message);
});

it('calls resolvePromises for resolve syscall', async () => {
Expand Down
Loading
Loading