Skip to content

Commit 4d0d5fd

Browse files
sirtimidsirtimid
authored
feat(kernel): Wait for crank to run kernel actions (#595)
Closes #582 Currently, kernel actions like `terminateVat`, `clearStorage`, and other control operations can be triggered while the Kernel is mid-crank, leading to race conditions and inconsistent kernel state with errors like "unknown kernel object". This PR implements a **two-layer solution** to prevent race conditions: 1. **Kernel Layer**: Added `waitForCrank()` calls to kernel methods, ensuring they wait for the current crank to complete before executing 2. **UI Layer**: Implemented a queuing mechanism to ensure kernel operations are processed sequentially and avoid race conditions. ## Implementation Note The `wakeUpPromise` is awaited after `endCrank()` to prevent the crank from hanging indefinitely. Without this order, the crank would remain active while waiting for new work, preventing `waitForCrank()` from ever completing. This ensures external operations can proceed once the current crank's work is done, while the run loop can still wait for new items.
1 parent 582861d commit 4d0d5fd

22 files changed

Lines changed: 468 additions & 264 deletions

‎packages/extension/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@
6565
"@ocap/cli": "workspace:^",
6666
"@ocap/test-utils": "workspace:^",
6767
"@ocap/vite-plugins": "workspace:^",
68-
"@playwright/test": "^1.51.1",
68+
"@playwright/test": "^1.54.2",
6969
"@testing-library/jest-dom": "^6.6.3",
7070
"@types/chrome": "^0.0.313",
7171
"@types/react": "^17.0.11",
@@ -85,7 +85,7 @@
8585
"eslint-plugin-prettier": "^5.2.6",
8686
"eslint-plugin-promise": "^7.2.1",
8787
"jsdom": "^26.0.0",
88-
"playwright": "^1.51.1",
88+
"playwright": "^1.54.2",
8989
"prettier": "^3.5.3",
9090
"rimraf": "^6.0.1",
9191
"typedoc": "^0.28.1",

‎packages/extension/test/e2e/control-panel.test.ts‎

Lines changed: 16 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@ test.describe('Control Panel', () => {
1414
const extension = await makeLoadExtension();
1515
extensionContext = extension.browserContext;
1616
popupPage = extension.popupPage;
17-
await expect(
18-
popupPage.locator('[data-testid="subcluster-accordion-s1"]'),
19-
).toBeVisible();
2017
});
2118

2219
test.afterEach(async () => {
@@ -56,26 +53,33 @@ test.describe('Control Panel', () => {
5653
).toBeVisible();
5754
});
5855

59-
test('should launch a new subcluster and vat within it', async () => {
60-
// Clear all state
61-
await popupPage.click('button:text("Clear All State")');
56+
test('should terminate a subcluster', async () => {
57+
// Open the subcluster accordion first
58+
await popupPage.locator('[data-testid="accordion-header"]').first().click();
6259
await expect(
63-
popupPage.locator('[data-testid="message-output"]'),
64-
).toContainText('State cleared');
60+
popupPage.locator('[data-testid="terminate-subcluster-button"]').first(),
61+
).toBeVisible();
62+
await popupPage
63+
.locator('[data-testid="terminate-subcluster-button"]')
64+
.first()
65+
.click();
6566
await expect(
66-
popupPage.locator('[data-testid="subcluster-accordion-s1"]'),
67-
).not.toBeVisible();
67+
popupPage.locator('[data-testid="message-output"]'),
68+
).toContainText('Terminated subcluster');
69+
});
70+
71+
test('should launch a new subcluster and vat within it', async () => {
6872
// Launch a new subcluster
6973
await launchSubcluster(minimalClusterConfig);
7074
const subcluster = popupPage.locator(
71-
'[data-testid="subcluster-accordion-s1"]',
75+
'[data-testid="subcluster-accordion-s2"]',
7276
);
7377
await expect(subcluster).toBeVisible({
7478
timeout: 2000,
7579
});
7680
await expect(popupPage.locator('text=1 Vat')).toBeVisible();
7781
// Open the subcluster accordion to view vats
78-
await popupPage.locator('[data-testid="accordion-header"]').click();
82+
await popupPage.locator('text=Subcluster s2 - 1 Vat').click();
7983
const vatTable = popupPage.locator('[data-testid="vat-table"]');
8084
await expect(vatTable).toBeVisible();
8185
await expect(vatTable.locator('tr')).toHaveCount(2);
@@ -126,21 +130,6 @@ test.describe('Control Panel', () => {
126130
).toContainText('pong');
127131
});
128132

129-
test('should terminate a subcluster', async () => {
130-
// Open the subcluster accordion first
131-
await popupPage.locator('[data-testid="accordion-header"]').first().click();
132-
await expect(
133-
popupPage.locator('[data-testid="terminate-subcluster-button"]').first(),
134-
).toBeVisible();
135-
await popupPage
136-
.locator('[data-testid="terminate-subcluster-button"]')
137-
.first()
138-
.click();
139-
await expect(
140-
popupPage.locator('[data-testid="message-output"]'),
141-
).toContainText('Terminated subcluster');
142-
});
143-
144133
test('should reload a subcluster', async () => {
145134
// Open the subcluster accordion first
146135
await popupPage.locator('[data-testid="accordion-header"]').first().click();
@@ -165,30 +154,6 @@ test.describe('Control Panel', () => {
165154
popupPage.locator('[data-testid="message-output"]'),
166155
).toContainText('All vats terminated');
167156
await expect(popupPage.locator('table')).not.toBeVisible();
168-
// ensure all references were garbage collected
169-
await popupPage.locator('[data-testid="clear-logs-button"]').click();
170-
await expect(
171-
popupPage.locator('[data-testid="message-output"]'),
172-
).toContainText('');
173-
await popupPage.click('button:text("Database Inspector")');
174-
const expectedValues = JSON.stringify([
175-
{ key: 'queue.run.head', value: '6' },
176-
{ key: 'queue.run.tail', value: '6' },
177-
{ key: 'gcActions', value: '[]' },
178-
{ key: 'reapQueue', value: '[]' },
179-
{ key: 'vats.terminated', value: '[]' },
180-
{ key: 'nextObjectId', value: '4' },
181-
{ key: 'nextPromiseId', value: '4' },
182-
{ key: 'nextVatId', value: '4' },
183-
{ key: 'nextRemoteId', value: '1' },
184-
{ key: 'subclusters', value: '[]' },
185-
{ key: 'nextSubclusterId', value: '2' },
186-
{ key: 'vatToSubclusterMap', value: '{}' },
187-
{ key: 'initialized', value: 'true' },
188-
]);
189-
await expect(
190-
popupPage.locator('[data-testid="message-output"]'),
191-
).toContainText(expectedValues);
192157
});
193158

194159
test('should clear kernel state', async () => {

‎packages/extension/test/e2e/database-inspector.test.ts‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,10 @@ test.describe('Database Inspector', () => {
1111
const extension = await makeLoadExtension();
1212
extensionContext = extension.browserContext;
1313
popupPage = extension.popupPage;
14+
await popupPage.click('button:text("Database Inspector")');
1415
await expect(
15-
popupPage.locator('[data-testid="subcluster-accordion-s1"]'),
16+
popupPage.locator('text=SELECT name FROM sqlite_master'),
1617
).toBeVisible();
17-
await popupPage.locator('[data-testid="accordion-header"]').first().click();
18-
await expect(popupPage.locator('table tr')).toHaveCount(4); // Header + 3 rows
19-
await popupPage.click('button:text("Database Inspector")');
20-
await expect(popupPage.locator('#root')).toContainText(
21-
'SELECT name FROM sqlite_master',
22-
);
2318
});
2419

2520
test.afterEach(async () => {
@@ -62,8 +57,8 @@ test.describe('Database Inspector', () => {
6257
'INVALID SQL QUERY',
6358
);
6459
await popupPage.click('[data-testid="execute-query-button"]');
65-
await expect(popupPage.locator('#root')).toContainText(
66-
'Failed to execute query',
67-
);
60+
await expect(
61+
popupPage.locator('text=Failed to execute query'),
62+
).toBeVisible();
6863
});
6964
});

‎packages/extension/test/e2e/object-registry.test.ts‎

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,6 @@
11
import { test, expect } from '@playwright/test';
2-
import type { Page, BrowserContext } from '@playwright/test';
2+
import type { Page, BrowserContext, Locator } from '@playwright/test';
33

4-
import {
5-
revokeObject,
6-
sendMessage,
7-
openObjectRegistryTab,
8-
} from './object-registry.ts';
94
import { makeLoadExtension } from '../helpers/extension.ts';
105

116
test.describe.configure({ mode: 'serial' });
@@ -14,11 +9,57 @@ test.describe('Object Registry', () => {
149
let extensionContext: BrowserContext;
1510
let popupPage: Page;
1611

12+
/**
13+
* Send a message to an object.
14+
*
15+
* @param page - The page to send the message on.
16+
* @param target - The target of the object.
17+
* @param method - The method to call.
18+
* @param params - The parameters to pass to the method.
19+
* @returns The message response locator.
20+
*/
21+
const sendMessage = async (
22+
page: Page,
23+
target: string,
24+
method: string,
25+
params: string,
26+
) => {
27+
await page
28+
.locator('select[data-testid="message-target"]')
29+
.selectOption(target);
30+
await page.locator('input[data-testid="message-method"]').fill(method);
31+
await page.locator('input[data-testid="message-params"]').fill(params);
32+
await page.locator('button[data-testid="message-send-button"]').click();
33+
return page.locator('[data-testid="message-response"]');
34+
};
35+
36+
/**
37+
* Revoke an object.
38+
*
39+
* @param page - The page to revoke the object on.
40+
* @param owner - The owner of the object.
41+
* @param target - The target of the object.
42+
* @returns The button and output locator.
43+
*/
44+
const revokeObject = async (
45+
page: Page,
46+
owner: string,
47+
target: string,
48+
): Promise<{ button: Locator; output: Locator }> => {
49+
await page
50+
.locator(`[data-testid="accordion-header"]:has(:text("${owner}"))`)
51+
.click();
52+
const button = page.locator(`[data-testid="revoke-button-${target}"]`);
53+
await button.click();
54+
return { button, output: page.locator('[data-testid="message-output"]') };
55+
};
56+
1757
test.beforeEach(async () => {
1858
const extension = await makeLoadExtension();
1959
extensionContext = extension.browserContext;
2060
popupPage = extension.popupPage;
21-
await openObjectRegistryTab(popupPage, expect);
61+
await popupPage.click('button:text("Object Registry")');
62+
await expect(popupPage.locator('text=Kernel Registry')).toBeVisible();
2263
});
2364

2465
test.afterEach(async () => {
@@ -31,9 +72,9 @@ test.describe('Object Registry', () => {
3172
);
3273
await clearLogsButton.click();
3374
await popupPage.click('button:text("Object Registry")');
34-
await expect(popupPage.locator('#root')).toContainText(
35-
'Alice (v1) - 3 objects, 3 promises',
36-
);
75+
await expect(
76+
popupPage.locator('text=Alice (v1) - 3 objects, 3 promises'),
77+
).toBeVisible();
3778
const targetSelect = popupPage.locator('[data-testid="message-target"]');
3879
await expect(targetSelect).toBeVisible();
3980
const options = targetSelect.locator('option:not([value=""])');
@@ -60,9 +101,9 @@ test.describe('Object Registry', () => {
60101
await popupPage.click('[data-testid="message-send-button"]');
61102
await expect(messageResponse).toContainText('"body":"#\\"vat Alice got');
62103
await expect(messageResponse).toContainText('"slots":[');
63-
await expect(popupPage.locator('#root')).toContainText(
64-
'Alice (v1) - 3 objects, 5 promises',
65-
);
104+
await expect(
105+
popupPage.locator('text=Alice (v1) - 3 objects, 5 promises'),
106+
).toBeVisible();
66107
});
67108

68109
test('should revoke an object', async () => {

‎packages/extension/test/e2e/object-registry.ts‎

Lines changed: 0 additions & 37 deletions
This file was deleted.

‎packages/extension/test/helpers/extension.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { chromium } from '@playwright/test';
1+
import { chromium, expect } from '@playwright/test';
22
import type { BrowserContext, Page } from '@playwright/test';
33
import { rm } from 'fs/promises';
44
import os from 'os';
@@ -62,6 +62,8 @@ export const makeLoadExtension = async (): Promise<{
6262

6363
const popupPage = await browserContext.newPage();
6464
await popupPage.goto(`chrome-extension://${extensionId}/popup.html`);
65+
// Wait for the default subcluster accordion to be visible
66+
await expect(popupPage.locator('text=Subcluster s1 - 3 Vats')).toBeVisible();
6567

6668
return { browserContext, extensionId, popupPage };
6769
};

‎packages/kernel-browser-runtime/src/rpc-handlers/get-status.test.ts‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,48 +8,48 @@ describe('getStatusHandler', () => {
88

99
beforeEach(() => {
1010
mockKernel = {
11-
getVats: vi.fn(),
12-
getSubclusters: vi.fn(),
11+
getStatus: vi.fn(),
1312
} as unknown as Kernel;
1413
});
1514

16-
it('should return vats and subclusters status', () => {
17-
const mockVats = [{ id: 'v1', config: { sourceSpec: 'test' } }];
15+
it('should return vats and subclusters status', async () => {
16+
const mockVats = [
17+
{ id: 'v1', config: { sourceSpec: 'test' }, subclusterId: 'sc1' },
18+
];
1819
const mockSubclusters = [
1920
{ id: 'sc1', config: { bootstrap: 'test', vats: {} }, vats: [] },
2021
];
2122

22-
vi.mocked(mockKernel.getVats).mockReturnValueOnce(mockVats);
23-
vi.mocked(mockKernel.getSubclusters).mockReturnValueOnce(mockSubclusters);
23+
vi.mocked(mockKernel.getStatus).mockResolvedValueOnce({
24+
vats: mockVats,
25+
subclusters: mockSubclusters,
26+
});
2427

25-
const result = getStatusHandler.implementation({ kernel: mockKernel }, []);
28+
const result = await getStatusHandler.implementation(
29+
{ kernel: mockKernel },
30+
[],
31+
);
2632

27-
expect(mockKernel.getVats).toHaveBeenCalledTimes(1);
28-
expect(mockKernel.getSubclusters).toHaveBeenCalledTimes(1);
33+
expect(mockKernel.getStatus).toHaveBeenCalledTimes(1);
2934
expect(result).toStrictEqual({
3035
vats: mockVats,
3136
subclusters: mockSubclusters,
3237
});
3338
});
3439

35-
it('should propagate errors from getVats', () => {
40+
it('should propagate errors from getVats', async () => {
3641
const error = new Error('Status check failed');
37-
vi.mocked(mockKernel.getVats).mockImplementationOnce(() => {
38-
throw error;
39-
});
40-
expect(() =>
42+
vi.mocked(mockKernel.getStatus).mockRejectedValueOnce(error);
43+
await expect(
4144
getStatusHandler.implementation({ kernel: mockKernel }, []),
42-
).toThrow(error);
45+
).rejects.toThrow(error);
4346
});
4447

45-
it('should propagate errors from getSubclusters', () => {
48+
it('should propagate errors from getSubclusters', async () => {
4649
const error = new Error('Subcluster status check failed');
47-
vi.mocked(mockKernel.getVats).mockReturnValueOnce([]);
48-
vi.mocked(mockKernel.getSubclusters).mockImplementationOnce(() => {
49-
throw error;
50-
});
51-
expect(() =>
50+
vi.mocked(mockKernel.getStatus).mockRejectedValueOnce(error);
51+
await expect(
5252
getStatusHandler.implementation({ kernel: mockKernel }, []),
53-
).toThrow(error);
53+
).rejects.toThrow(error);
5454
});
5555
});

0 commit comments

Comments
 (0)