Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
76dec71
test: pin the transaction invariants #1005 left broken
grypez Aug 6, 2026
aeec7b3
fix: keep a crank's store work inside one transaction
sirtimid Aug 6, 2026
848f334
test(kernel-test): reap until the vat's GC is visible, not three times
sirtimid Aug 6, 2026
a4bb536
test: fix rollback crank test
sirtimid Aug 6, 2026
fa4285a
fix(ocap-kernel): forget every savepoint when a crank rollback fails
sirtimid Aug 6, 2026
8df9fde
fix(kernel-store): log an abort that fails while discarding a transac…
sirtimid Aug 6, 2026
249b802
test(ocap-kernel): pin the flush's ordering against a failing enqueue
sirtimid Aug 6, 2026
7f5abd3
docs: correct the transaction claims review found wrong
sirtimid Aug 6, 2026
f7ab72e
docs: cut the padding from this branch's comments and changelogs
sirtimid Aug 6, 2026
e5b2e27
fix(ocap-kernel): revert cached values and GC candidates on crank rol…
sirtimid Aug 10, 2026
64a33e1
test(kernel-store): pin the failed COMMIT that wedges `_inTx`
grypez Aug 10, 2026
74c169b
test(ocap-kernel): pin the endCrank failure that buries the real error
grypez Aug 10, 2026
bb7ea13
test(kernel-node-runtime): pin the kernel store's missing logger
grypez Aug 10, 2026
5fc571f
test(ocap-kernel): pin the release failure lost at the remote savepoint
grypez Aug 10, 2026
99f118a
test: tighten the four repros after review
grypez Aug 10, 2026
f3b9431
fix(kernel-store): clear `_inTx` before the COMMIT, not after
sirtimid Aug 13, 2026
7bd8e21
fix(ocap-kernel): stop `endCrank` burying the error that killed the r…
sirtimid Aug 13, 2026
29274c4
fix(ocap-kernel): report the remote release failure, not a missing sa…
sirtimid Aug 13, 2026
6c57325
test(ocap-kernel): pin the in-memory revert against a failed crank ro…
sirtimid Aug 13, 2026
2ebcbcd
fix(runtimes): give the kernel store a logger
sirtimid Aug 13, 2026
65bd467
chore: cite this PR in the changelogs
sirtimid Aug 13, 2026
13b7616
test(kernel-store): pin the transaction a failed COMMIT abandons
sirtimid Aug 20, 2026
913524a
fix(kernel-store): discard the transaction a failed COMMIT leaves open
sirtimid Aug 20, 2026
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
1 change: 1 addition & 0 deletions packages/kernel-browser-runtime/CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- The kernel worker gives the kernel store a logger, so the wasm SQLite driver's diagnostics — including the transaction aborts it reports on the kernel's dying path — reach the log rather than nowhere ([#1021](https://github.com/MetaMask/ocap-kernel/pull/1021))
- Process platform-services RPC request handlers in the background so a request handler that fires a reentrant outbound RPC (e.g. transport handshake calling back into the kernel) cannot deadlock waiting for its response ([#948](https://github.com/MetaMask/ocap-kernel/pull/948))

## [0.6.0]
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,7 +44,10 @@ async function main(): Promise<void> {
isJsonRpcMessage,
),
PlatformServicesClient.make(globalThis as PostMessageTarget),
makeSQLKernelDatabase({ dbFilename: DB_FILENAME }),
makeSQLKernelDatabase({
dbFilename: DB_FILENAME,
logger: logger.subLogger({ tags: ['kernel-store'] }),
}),
]);

setupConsoleForwarding({
Expand Down
1 change: 1 addition & 0 deletions packages/kernel-node-runtime/CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- `makeKernel` gives the kernel store a logger, so the SQLite driver's diagnostics — including the transaction aborts it reports on the kernel's dying path — reach the log rather than nowhere ([#1021](https://github.com/MetaMask/ocap-kernel/pull/1021))
- The RPC socket server refuses to bind a Unix socket that has a live listener, rather than unlinking it and orphaning the previous owner; stale socket files with no listener are still cleaned up automatically ([#952](https://github.com/MetaMask/ocap-kernel/pull/952))

## [0.1.0]
Expand Down
21 changes: 20 additions & 1 deletion packages/kernel-node-runtime/src/kernel/make-kernel.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
import { makeSQLKernelDatabase } from '@metamask/kernel-store/sqlite/nodejs';
import { Logger } from '@metamask/logger';
import { Kernel } from '@metamask/ocap-kernel';
import { describe, expect, it, vi } from 'vitest';

Expand All@@ -8,7 +10,9 @@ vi.mock('@metamask/kernel-store/sqlite/nodejs', async () => {
'../../../ocap-kernel/test/storage.ts'
);
return {
makeSQLKernelDatabase: makeMapKernelDatabase,
// Wrapped so that a test can see what the database was constructed with,
// while still getting a real store back.
makeSQLKernelDatabase: vi.fn(makeMapKernelDatabase),
};
});

Expand All@@ -18,4 +22,19 @@ describe('makeKernel', () => {

expect(kernel).toBeInstanceOf(Kernel);
});

// FAILING REPRO.
//
// The kernel store is the only collaborator `makeKernel` builds without
// handing it a logger, so every `logger?.` call inside the SQLite driver is
// dead code in production — including the four abort failures #1012 added
// logging for. `kernel-worker.ts` omits it too, which keeps the wasm driver's
// pair dead even once this passes.
it('gives the kernel store a logger', async () => {
await makeKernel({});

expect(vi.mocked(makeSQLKernelDatabase)).toHaveBeenCalledWith(
expect.objectContaining({ logger: expect.any(Logger) }),
);
});
});
5 changes: 4 additions & 1 deletion packages/kernel-node-runtime/src/kernel/make-kernel.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -60,7 +60,10 @@ export async function makeKernel({
});

// Initialize kernel store.
const kernelDatabase = await makeSQLKernelDatabase({ dbFilename });
const kernelDatabase = await makeSQLKernelDatabase({
dbFilename,
logger: rootLogger.subLogger({ tags: ['kernel-store'] }),
});

// Create and start kernel.
const kernel = await Kernel.make(platformServicesClient, kernelDatabase, {
Expand Down
5 changes: 5 additions & 0 deletions packages/kernel-store/CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,6 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `rollbackSavepoint` discards the enclosing transaction when `ROLLBACK TO` itself fails, instead of leaving the savepoint on its stack and the transaction open ([#1005](https://github.com/MetaMask/ocap-kernel/pull/1005))
- Nothing would ever commit or abort that transaction, so every later write on the connection silently joined it, reported success, and vanished on close. Discarding it is no wider than the caller asked for: the transaction begins with the outermost savepoint, so it holds only the work the rollback was abandoning
- The rollback failure is still what gets thrown, even if aborting the transaction fails too
- `releaseSavepoint` discards the enclosing transaction when `RELEASE` fails, as `rollbackSavepoint` already did for `ROLLBACK TO` ([#1021](https://github.com/MetaMask/ocap-kernel/pull/1021))
- Otherwise the savepoint stayed on the stack and the transaction open with nothing left to commit or abort it. The release failure is still what gets thrown
- The wasm driver clears `_inTx` when aborting or committing a transaction throws, and discards the transaction that a failed COMMIT can leave open ([#1021](https://github.com/MetaMask/ocap-kernel/pull/1021))
- Left true, `beginIfNeeded` became a permanent no-op and later writes autocommitted — including the next savepoint, which was then created bare, where its `RELEASE` commits and no rollback could undo the delivery. The nodejs driver reads `db.inTransaction` and was never affected
- An abort that fails while recovering from a failed savepoint operation is now logged in both drivers ([#1021](https://github.com/MetaMask/ocap-kernel/pull/1021))

## [0.6.0]

Expand Down
39 changes: 39 additions & 0 deletions packages/kernel-store/src/sqlite/nodejs.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -360,6 +360,45 @@ describe('makeSQLKernelDatabase', () => {
expect(mockDb._spStack).toStrictEqual([]);
});

// The hazard `rollbackSavepoint` guards against, by the other door.
it('releaseSavepoint discards the transaction when the release fails', async () => {
const db = await makeSQLKernelDatabase({});
mockDb.inTransaction = true;
mockDb._spStack = ['point1'];
mockStatement.run.mockClear();
mockDb.exec.mockImplementationOnce(() => {
throw new Error('disk I/O error');
});

expect(() => db.releaseSavepoint('point1')).toThrowError(
'disk I/O error',
);

expect(mockDb._spStack).toStrictEqual([]);
// The abort is the only prepared statement this path runs.
expect(mockStatement.run).toHaveBeenCalledOnce();
mockDb.inTransaction = false;
});

it('releaseSavepoint reports the release failure even if the abort fails too', async () => {
const db = await makeSQLKernelDatabase({});
mockDb.inTransaction = true;
mockDb._spStack = ['point1'];
mockDb.exec.mockImplementationOnce(() => {
throw new Error('disk I/O error');
});
mockStatement.run.mockImplementationOnce(() => {
throw new Error('cannot rollback');
});

expect(() => db.releaseSavepoint('point1')).toThrowError(
'disk I/O error',
);

expect(mockDb._spStack).toStrictEqual([]);
mockDb.inTransaction = false;
});

it('supports nested savepoints', async () => {
const db = await makeSQLKernelDatabase({});
db.createSavepoint('outer');
Expand Down
33 changes: 30 additions & 3 deletions packages/kernel-store/src/sqlite/nodejs.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -298,8 +298,14 @@ export async function makeSQLKernelDatabase({
db._spStack.length = 0;
try {
rollbackIfNeeded();
} catch {
// The rollback failure below is the one worth reporting.
} catch (abortError) {
// The rollback failure below is the one worth reporting, but a failed
// abort leaves SQLite holding a transaction the next crank would
// silently write into. Nothing here can repair that.
logger?.error(
'failed to discard transaction after rollback',
abortError,
);
}
throw error;
}
Expand All@@ -321,7 +327,28 @@ export async function makeSQLKernelDatabase({
throw new Error(`No such savepoint: ${name}`);
}
const query = SQL_QUERIES.RELEASE_SAVEPOINT.replace('%NAME%', name);
db.exec(query);
try {
db.exec(query);
} catch (error) {
// The hazard `rollbackSavepoint` guards against, by the other door: left as
// it was, the savepoint stays on the stack and the transaction open with
// nothing to ever commit or abort it, so every later write on this
// connection joins it, reports success, and vanishes on close. There is no
// committing this transaction now, so discard it.
db._spStack.length = 0;
try {
rollbackIfNeeded();
} catch (abortError) {
// The release failure below is the one worth reporting, but a failed
// abort leaves SQLite holding a transaction the next crank would
// silently write into. Nothing here can repair that.
logger?.error(
'failed to discard transaction after release',
abortError,
);
}
throw error;
}
db._spStack.splice(idx);
if (db._spStack.length === 0) {
commitIfNeeded();
Expand Down
161 changes: 160 additions & 1 deletion packages/kernel-store/src/sqlite/wasm.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -488,7 +488,6 @@ describe('makeSQLKernelDatabase', () => {
);

expect(mockDb._spStack).toStrictEqual([]);
mockDb._inTx = false;
});

it('releaseSavepoint validates savepoint exists', async () => {
Expand DownExpand Up@@ -518,6 +517,166 @@ describe('makeSQLKernelDatabase', () => {
expect(mockDb._inTx).toBe(false);
});

// The hazard `rollbackSavepoint` guards against, by the other door.
it('releaseSavepoint discards the transaction when the release fails', async () => {
const db = await makeSQLKernelDatabase({});
mockDb._inTx = true;
mockDb._spStack = ['point1'];
mockDb.exec.mockImplementationOnce(() => {
throw new Error('disk I/O error');
});

expect(() => db.releaseSavepoint('point1')).toThrowError(
'disk I/O error',
);

expect(mockDb._spStack).toStrictEqual([]);
expect(mockDb._inTx).toBe(false);
});

it('releaseSavepoint reports the release failure even if the abort fails too', async () => {
const db = await makeSQLKernelDatabase({});
mockDb._inTx = true;
mockDb._spStack = ['point1'];
mockDb.exec.mockImplementationOnce(() => {
throw new Error('disk I/O error');
});
mockStatement.step.mockImplementationOnce(() => {
throw new Error('cannot rollback');
});

expect(() => db.releaseSavepoint('point1')).toThrowError(
'disk I/O error',
);

expect(mockDb._inTx).toBe(false);
});

// A failed abort is the one case that can leave `_inTx` disagreeing with the
// database. Left true, `beginIfNeeded` is a no-op forever after.
it('stops believing it is in a transaction when the abort fails too', async () => {
const db = await makeSQLKernelDatabase({});
mockDb._inTx = true;
mockDb._spStack = ['point1'];
mockDb.exec.mockImplementationOnce(() => {
throw new Error('disk I/O error');
});
mockStatement.step.mockImplementationOnce(() => {
throw new Error('cannot rollback');
});

expect(() => db.rollbackSavepoint('point1')).toThrowError(
'disk I/O error',
);

expect(mockDb._inTx).toBe(false);
});

// Why that matters: a savepoint created outside a transaction commits when
// released (Agoric/agoric-sdk#8423), so an aborted crank would keep its
// writes.
it('begins a transaction for the next savepoint after a failed abort', async () => {
const db = await makeSQLKernelDatabase({});
mockDb._inTx = true;
mockDb._spStack = ['point1'];
mockDb.exec.mockImplementationOnce(() => {
throw new Error('disk I/O error');
});
mockStatement.step.mockImplementationOnce(() => {
throw new Error('cannot rollback');
});
expect(() => db.rollbackSavepoint('point1')).toThrowError(
'disk I/O error',
);

mockDb.exec.mockClear();
mockStatement.step.mockClear();
db.createSavepoint('next');

// BEGIN is the only prepared statement `createSavepoint` runs; the
// SAVEPOINT itself goes through `exec`.
expect(mockStatement.step).toHaveBeenCalledOnce();
expect(mockDb.exec).toHaveBeenCalledWith('SAVEPOINT next');
});

// Why the ordering matters: see `commitIfNeeded`.
it('stops believing it is in a transaction when the commit fails', async () => {
const db = await makeSQLKernelDatabase({});
mockDb._inTx = true;
mockDb._spStack = ['point1'];
// The RELEASE goes through `exec` and succeeds; COMMIT is the first
// prepared statement this path steps, and it is what fails.
mockStatement.step.mockImplementationOnce(() => {
throw new Error('disk I/O error');
});

expect(() => db.releaseSavepoint('point1')).toThrowError(
'disk I/O error',
);

expect(mockDb._inTx).toBe(false);

// And so the next savepoint gets a transaction of its own rather than
// being created bare.
mockDb.exec.mockClear();
mockStatement.step.mockClear();
db.createSavepoint('next');
expect(mockStatement.step).toHaveBeenCalledOnce();
expect(mockDb.exec).toHaveBeenCalledWith('SAVEPOINT next');
});

// Why the COMMIT path discards too: see `commitIfNeeded`.
it('releaseSavepoint discards the transaction when the commit fails', async () => {
const db = await makeSQLKernelDatabase({});
mockStatement.step.mockClear();
mockDb._inTx = true;
mockDb._spStack = ['point1'];
// The RELEASE goes through `exec` and succeeds; COMMIT is the first
// prepared statement this path steps, and it is what fails.
mockStatement.step.mockImplementationOnce(() => {
throw new Error('database is locked');
});

expect(() => db.releaseSavepoint('point1')).toThrowError(
'database is locked',
);

// Every statement shares one mock, so the ABORT is only visible as a
// second step.
expect(mockStatement.step).toHaveBeenCalledTimes(2);
expect(mockDb._inTx).toBe(false);
});

it('releaseSavepoint reports the commit failure even if the abort fails too', async () => {
const logger = {
debug: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
subLogger: vi.fn(() => logger),
} as unknown as Logger;
const db = await makeSQLKernelDatabase({ logger });
mockStatement.step.mockClear();
mockDb._inTx = true;
mockDb._spStack = ['point1'];
mockStatement.step
.mockImplementationOnce(() => {
throw new Error('database is locked');
})
.mockImplementationOnce(() => {
throw new Error('cannot rollback');
});

expect(() => db.releaseSavepoint('point1')).toThrowError(
'database is locked',
);

expect(logger.error).toHaveBeenCalledWith(
'failed to discard transaction after commit',
expect.objectContaining({ message: 'cannot rollback' }),
);
expect(mockDb._inTx).toBe(false);
});

it('supports nested savepoints', async () => {
const db = await makeSQLKernelDatabase({});
db.createSavepoint('outer');
Expand Down
Loading
Loading