From e54a39a5a6007b270d58a04e056d5f5a2a185a19 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:51:35 -0400 Subject: [PATCH 1/5] test(kernel-store): pin the crank commit point against a foreign savepoint `KernelQueue.#runLoop` calls releasing its `crank` savepoint "this crank's one commit point". `releaseAllSavepoints` releases `t0`, which is the outermost savepoint only when the crank opened the first one, and two production paths open savepoints through `KernelStore.createSavepoint` -- invisible to the ordinal numbering, uncoordinated with the crank, one of them held across an await. Real SQLite through the real driver, one test per interleaving. Co-Authored-By: Claude Opus 5 --- .../nodejs.savepoint-interleaving.test.ts | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 packages/kernel-store/src/sqlite/nodejs.savepoint-interleaving.test.ts diff --git a/packages/kernel-store/src/sqlite/nodejs.savepoint-interleaving.test.ts b/packages/kernel-store/src/sqlite/nodejs.savepoint-interleaving.test.ts new file mode 100644 index 000000000..e47da70ad --- /dev/null +++ b/packages/kernel-store/src/sqlite/nodejs.savepoint-interleaving.test.ts @@ -0,0 +1,91 @@ +import { describe, it, expect } from 'vitest'; + +import { makeSQLKernelDatabase } from './nodejs.ts'; + +/** + * `KernelQueue.#runLoop` calls releasing its `crank` savepoint "this crank's one + * commit point", on the grounds that only `delivery` is ever rolled back. + * + * `releaseAllSavepoints` releases `t0`, which is the outermost savepoint only if + * the crank opened the first one. Two production paths open savepoints through + * `KernelStore.createSavepoint`, which bypasses `ctx.savepoints` and so is + * invisible to the ordinal numbering: `RemoteHandle.handleRemoteMessage` (held + * across its `await this.#handleRedeemURLRequest(...)`) and + * `RemoteManager.#handlePeerIncarnation`. Neither waits for the crank, and the + * remote message handler is installed in `Kernel.#init` as a bare async callback, + * so all three orderings below are reachable while the run loop sits in + * `await deliver(queueItem)`. + * + * Real SQLite through the real driver: these are the savepoint semantics, not a + * mock's idea of them. + */ +describe('a savepoint the crank does not know about', () => { + it('outside the crank, leaves the crank release with nothing to commit', async () => { + const kdb = await makeSQLKernelDatabase({ dbFilename: ':memory:' }); + const kv = kdb.kernelKVStore; + + // RemoteHandle.handleRemoteMessage, parked on its await. + kdb.createSavepoint('receive_r1_7'); + + // The run loop wakes: startCrank, then the two crank savepoints. + kdb.createSavepoint('t0'); + kdb.createSavepoint('t1'); + kv.set('crankWrite', 'durable'); + + // endCrank -> releaseAllSavepoints -> releaseSavepoint('t0'). `t0` is not the + // outermost savepoint, so this releases into the remote's, not to a commit. + kdb.releaseSavepoint('t0'); + + // The remote message then fails, so RemoteHandle rolls its savepoint back. + kdb.rollbackSavepoint('receive_r1_7'); + + // If releasing `crank` were a commit point, the crank would have survived. + expect(kv.get('crankWrite')).toBe('durable'); + kdb.close(); + }); + + it('inside the crank, is destroyed by the delivery rollback behind its owner', async () => { + const kdb = await makeSQLKernelDatabase({ dbFilename: ':memory:' }); + const kv = kdb.kernelKVStore; + + // startCrank + kdb.createSavepoint('t0'); + kdb.createSavepoint('t1'); + + // A remote message arrives during `await deliver(queueItem)`. + kdb.createSavepoint('receive_r1_7'); + kv.set('remoteSeq.r1.highestReceivedSeq', '7'); + + // The delivery aborts: rollbackCrank('delivery') issues ROLLBACK TO t1, and + // SQLite cancels every savepoint started after t1 -- including the remote's. + kdb.rollbackSavepoint('t1'); + + // The remote handler, still inside its own try, reaches its release. + expect(() => kdb.releaseSavepoint('receive_r1_7')).not.toThrow(); + kdb.close(); + }); + + it('inside a crank that succeeds, is committed under its owner', async () => { + const kdb = await makeSQLKernelDatabase({ dbFilename: ':memory:' }); + const kv = kdb.kernelKVStore; + + // startCrank + kdb.createSavepoint('t0'); + kdb.createSavepoint('t1'); + + // A remote message arrives and gets as far as its await, so the seq row it + // writes "at the end, within the transaction" is not written yet. + kdb.createSavepoint('receive_r1_7'); + kv.set('remoteHalfDone', 'yes'); + + // endCrank releases t0, and with it everything stacked above. + kdb.releaseSavepoint('t0'); + + // The remote handler resumes and tries to commit its own savepoint. Whatever + // it decides, its half-finished work is already durable -- and it will report + // failure, leaving the peer to retry an effect that has landed. + expect(() => kdb.releaseSavepoint('receive_r1_7')).not.toThrow(); + expect(kv.get('remoteHalfDone')).toBe('yes'); + kdb.close(); + }); +}); From b072c2645cb9851c7599382ab44f039f1565aab1 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:51:38 -0400 Subject: [PATCH 2/5] test(kernel-store): pin the transaction that survives a failed rollback A failed `ROLLBACK TO` is taken to discard the whole transaction, which is what makes truncating the savepoint list to zero match the database. That holds only when the compensating abort succeeds, and both drivers catch and log one that does not. This driver reads `db.inTransaction` from SQLite, so it cannot wedge a flag -- and also cannot end an ownerless transaction. Co-Authored-By: Claude Opus 5 --- .../nodejs.transaction-survival.test.ts | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 packages/kernel-store/src/sqlite/nodejs.transaction-survival.test.ts diff --git a/packages/kernel-store/src/sqlite/nodejs.transaction-survival.test.ts b/packages/kernel-store/src/sqlite/nodejs.transaction-survival.test.ts new file mode 100644 index 000000000..75efe368e --- /dev/null +++ b/packages/kernel-store/src/sqlite/nodejs.transaction-survival.test.ts @@ -0,0 +1,112 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +import { makeSQLKernelDatabase } from './nodejs.ts'; + +/** + * Two invariants the crank layer relies on: + * + * - a failed `ROLLBACK TO` discards the whole transaction, so truncating + * `ctx.savepoints` to zero still matches the database; + * - `commitIfNeeded` leaves no transaction behind. + * + * Both drivers catch and log an abort that fails while discarding a transaction, + * so "the whole transaction is discarded" holds only when that abort succeeds. + * This driver has no `_inTx` flag, reading `db.inTransaction` from SQLite + * instead, which prevents a wedged flag but does not end an ownerless + * transaction. + */ + +/** Every statement and exec call, in order. */ +let issued: string[] = []; +/** SQL that throws when next run. */ +let failOnce: Set = new Set(); + +const makeStatement = (text: string): Record => ({ + run: () => { + issued.push(text); + if (failOnce.delete(text)) { + throw new Error(`SQLITE_IOERR: ${text}`); + } + return undefined; + }, + get: () => undefined, + all: () => [], + pluck: () => undefined, + iterate: () => [], +}); + +const mockDb = { + prepare: vi.fn((text: string) => makeStatement(text)), + transaction: vi.fn((fn: () => void) => fn), + exec: vi.fn((text: string) => { + issued.push(text); + if (failOnce.delete(text)) { + throw new Error(`SQLITE_IOERR: ${text}`); + } + }), + inTransaction: false, + _spStack: [] as string[], + close: vi.fn(), +}; + +vi.mock('better-sqlite3', () => ({ + default: vi.fn(function () { + return mockDb; + }), +})); +vi.mock('node:fs/promises', () => ({ mkdir: vi.fn() })); +vi.mock('node:os', () => ({ tmpdir: vi.fn(() => '/mock-tmpdir') })); + +describe('the nodejs driver after a failure it tolerates', () => { + beforeEach(() => { + issued = []; + failOnce = new Set(); + mockDb.inTransaction = false; + mockDb._spStack = []; + }); + + it('discards the transaction when the rollback fails and the abort fails too', async () => { + const kdb = await makeSQLKernelDatabase({}); + // A crank in progress: SAVEPOINT t0, SAVEPOINT t1. + mockDb.inTransaction = true; + mockDb._spStack = ['t0', 't1']; + issued = []; + + // The disk fills. `ROLLBACK TO SAVEPOINT t1` fails, and so does the + // `ROLLBACK TRANSACTION` meant to discard the transaction instead. The driver + // logs that second failure and carries on, so SQLite is still in a + // transaction with t0 and t1 on its stack. + failOnce.add('ROLLBACK TO SAVEPOINT t1'); + failOnce.add('ROLLBACK TRANSACTION'); + expect(() => kdb.rollbackSavepoint('t1')).toThrow('SQLITE_IOERR'); + expect(mockDb._spStack).toStrictEqual([]); + + // `_spStack` now says "no savepoints, nothing to commit or abort" while + // SQLite says otherwise. Teardown still runs after the run loop dies -- + // `reset`, a peer incarnation change, a remote message -- and takes a + // savepoint. + issued = []; + kdb.createSavepoint('teardown'); + kdb.releaseSavepoint('teardown'); + + // `beginIfNeeded` saw `inTransaction` and skipped BEGIN, so `teardown` was + // created inside the transaction that was supposed to be gone, and releasing + // it committed that transaction whole -- the abandoned crank included. + expect(issued).not.toContain('COMMIT TRANSACTION'); + }); + + it('discards the transaction when the commit fails', async () => { + const kdb = await makeSQLKernelDatabase({}); + mockDb.inTransaction = true; + mockDb._spStack = ['t0']; + issued = []; + + // endCrank: RELEASE SAVEPOINT t0 succeeds, the COMMIT it triggers does not. + failOnce.add('COMMIT TRANSACTION'); + expect(() => kdb.releaseSavepoint('t0')).toThrow('SQLITE_IOERR'); + + // A failed COMMIT can leave the transaction open, and `_spStack` was already + // spliced empty. Nothing here ends it. + expect(issued).toContain('ROLLBACK TRANSACTION'); + }); +}); From 489f2998c85970b9125400a69ee09ef99d905da3 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:51:41 -0400 Subject: [PATCH 3/5] test(kernel-store): pin the transaction a failed COMMIT leaves open Clearing `_inTx` before stepping the COMMIT stops a throwing COMMIT wedging the flag true, and leaves nothing able to end the transaction it left open: `rollbackIfNeeded` reads the false flag and returns, and `releaseSavepoint` reaches `commitIfNeeded` with nothing wrapping it. Co-Authored-By: Claude Opus 5 --- .../sqlite/wasm.transaction-survival.test.ts | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 packages/kernel-store/src/sqlite/wasm.transaction-survival.test.ts diff --git a/packages/kernel-store/src/sqlite/wasm.transaction-survival.test.ts b/packages/kernel-store/src/sqlite/wasm.transaction-survival.test.ts new file mode 100644 index 000000000..edfdcad86 --- /dev/null +++ b/packages/kernel-store/src/sqlite/wasm.transaction-survival.test.ts @@ -0,0 +1,96 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +import { makeSQLKernelDatabase } from './wasm.ts'; + +/** + * `commitIfNeeded` clears `_inTx` before stepping the COMMIT, so a COMMIT that + * throws cannot wedge the flag true. That closes one door and opens another: + * SQLite can fail a COMMIT with the transaction still open, and with `_inTx` + * already false `rollbackIfNeeded` is a no-op, `commitIfNeeded` will not try + * again, and `releaseSavepoint` reaches `commitIfNeeded` with nothing wrapping + * it. The transaction is left with no owner -- the same hazard + * `rollbackSavepoint` and `releaseSavepoint` discard the transaction to avoid, + * reached by a third door. + * + * The nodejs driver reads `db.inTransaction` from SQLite rather than caching it, + * so it cannot wedge the flag; see `nodejs.transaction-survival.test.ts` for why + * that is not the same as having no gap. + */ + +/** Every exec call and statement step, in order. */ +let issued: string[] = []; +/** SQL that throws when next stepped. */ +let failOnce: Set = new Set(); + +const makeStatement = (text: string): Record => ({ + bind: () => undefined, + step: () => { + issued.push(text); + if (failOnce.delete(text)) { + throw new Error(`SQLITE_IOERR: ${text}`); + } + return false; + }, + getString: () => undefined, + reset: () => undefined, + get: () => undefined, + getColumnName: () => undefined, + columnCount: 2, +}); + +const mockDb = { + exec: vi.fn((text: string) => { + issued.push(text); + if (failOnce.delete(text)) { + throw new Error(`SQLITE_IOERR: ${text}`); + } + }), + prepare: vi.fn((text: string) => makeStatement(text)), + _inTx: false, + _spStack: [] as string[], + close: vi.fn(), +}; + +vi.mock('@sqlite.org/sqlite-wasm', () => ({ + default: vi.fn(async () => ({ + oo1: { + OpfsDb: vi.fn(function () { + return mockDb; + }), + DB: vi.fn(function () { + return mockDb; + }), + }, + })), +})); + +vi.mock('./env.ts', () => ({ getDBFolder: vi.fn(() => 'test-folder') })); + +describe('the wasm driver when a COMMIT fails', () => { + beforeEach(() => { + issued = []; + failOnce = new Set(); + mockDb._inTx = false; + mockDb._spStack = []; + }); + + it('discards the transaction the failed COMMIT leaves open', async () => { + const kdb = await makeSQLKernelDatabase({}); + // endCrank, with the crank's outermost savepoint still listed. + mockDb._inTx = true; + mockDb._spStack = ['t0']; + issued = []; + + // RELEASE SAVEPOINT t0 succeeds, so `commitIfNeeded` runs. The COMMIT fails. + failOnce.add('COMMIT TRANSACTION'); + expect(() => kdb.releaseSavepoint('t0')).toThrow('SQLITE_IOERR'); + + // The driver now believes there is no transaction, so nothing it does later + // will end one: `rollbackIfNeeded` reads the false flag and returns, and + // `commitIfNeeded` needs a savepoint release to be called again. Every plain + // `kvSet` between here and then joins the surviving transaction and reports + // success. + expect(mockDb._inTx).toBe(false); + expect(issued).toContain('ROLLBACK TRANSACTION'); + }); +}); From d2edb2fe019018db621303fccfa821fc8ce802c7 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:51:44 -0400 Subject: [PATCH 4/5] test(ocap-kernel): pin the audit that runs after the crank answers its callers The flush is last of the crank's own work so that no external caller is answered before the fallible work is done, and the reference count audit runs after the flush so that buffered items are not read as leaks. The audit is itself fallible, so the two orderings contradict each other. Co-Authored-By: Claude Opus 5 --- .../src/KernelQueue.audit-ordering.test.ts | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 packages/ocap-kernel/src/KernelQueue.audit-ordering.test.ts diff --git a/packages/ocap-kernel/src/KernelQueue.audit-ordering.test.ts b/packages/ocap-kernel/src/KernelQueue.audit-ordering.test.ts new file mode 100644 index 000000000..496f98246 --- /dev/null +++ b/packages/ocap-kernel/src/KernelQueue.audit-ordering.test.ts @@ -0,0 +1,93 @@ +import type { CapData } from '@endo/marshal'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +import { KernelQueue } from './KernelQueue.ts'; +import type { KernelStore } from './store/index.ts'; +import type { CrankResult, KRef, RunQueueItem } from './types.ts'; + +vi.mock('./garbage-collection/garbage-collection.ts', () => ({ + processGCActionSet: vi.fn().mockReturnValue(null), +})); + +/** + * `#processCrankResult` flushes the crank buffer last of the crank's own work, + * "after the fallible work above, not before it", because the flush settles the + * promise `enqueueMessage` gave an external caller and a later rollback would + * discard the state that answer was computed from. + * + * `assertRefCountsIfAuditing` then runs after the flush, because a buffered + * item's references were counted at enqueue time and so read as a leak if the + * audit runs mid-flush. Both orderings are individually justified and they + * contradict each other: the audit is fallible, it runs after the answers have + * gone out, and a crank that throws there is rolled back by the run loop's catch. + */ +describe('a reference count audit that fails after the flush', () => { + let kernelStore: KernelStore; + let kernelQueue: KernelQueue; + let resolveSubscription: (value: CapData) => void; + + const AUDIT_FAILED = 'reference count invariant violated'; + + beforeEach(() => { + resolveSubscription = vi.fn(); + + kernelStore = { + startCrank: vi.fn(), + endCrank: vi.fn(), + createCrankSavepoint: vi.fn(), + rollbackCrank: vi.fn(), + nextTerminatedVatCleanup: vi.fn(), + nextReapAction: vi.fn().mockReturnValue(null), + runQueueLength: vi.fn().mockReturnValue(0), + dequeueRun: vi.fn(), + enqueueRun: vi.fn(), + incrementRefCount: vi.fn(), + collectGarbage: vi.fn(), + // The buffered notify a successful crank flushes. + flushCrankBuffer: vi + .fn() + .mockReturnValue([{ type: 'notify', endpointId: 'v1', kpid: 'kp1' }]), + getKernelPromise: vi.fn().mockReturnValue({ + state: 'fulfilled', + value: { body: '{}', slots: [] }, + }), + // The audit the `auditRefCounts` option turns on, finding drift. + assertRefCountsIfAuditing: vi.fn(() => { + throw new Error(AUDIT_FAILED); + }), + } as unknown as KernelStore; + + kernelQueue = new KernelQueue( + kernelStore, + vi.fn().mockResolvedValue(undefined), + ); + + // An external caller waiting on `kp1`, as `enqueueMessage` leaves one. + kernelQueue.subscriptions.set('kp1' as KRef, { + resolve: resolveSubscription, + reject: vi.fn(), + }); + }); + + it('does not answer the external caller it is about to roll back', async () => { + const item: RunQueueItem = { + type: 'notify', + endpointId: 'v1', + kpid: 'kp1' as KRef, + } as RunQueueItem; + vi.mocked(kernelStore.runQueueLength).mockReturnValueOnce(1); + vi.mocked(kernelStore.dequeueRun).mockReturnValueOnce(item); + + const deliver = vi + .fn<(queueItem: RunQueueItem) => Promise>() + .mockResolvedValue(undefined); + + await expect(kernelQueue.run(deliver)).rejects.toThrow(AUDIT_FAILED); + + // The flush invoked the subscription, so the caller has its answer... + expect(resolveSubscription).toHaveBeenCalled(); + // ...and then the audit threw and the run loop rolled the delivery back + // underneath it. One of these two has to go. + expect(kernelStore.rollbackCrank).not.toHaveBeenCalledWith('delivery'); + }); +}); From 17ed8d910dd4048a7164318ce4a5173507de52d1 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:51:48 -0400 Subject: [PATCH 5/5] test(ocap-kernel): pin the GC candidate a rollback must not discard `maybeFreeKrefs` is not per-crank: only `collectGarbage` empties it, so a candidate added while no crank was open must survive an unrelated crank's rollback. `RemoteManager.#handlePeerIncarnation` is such a producer, and the objects it abandons are invisible to the reference count audit once lost. Co-Authored-By: Claude Opus 5 --- .../methods/crank.cross-crank-gc.test.ts | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 packages/ocap-kernel/src/store/methods/crank.cross-crank-gc.test.ts diff --git a/packages/ocap-kernel/src/store/methods/crank.cross-crank-gc.test.ts b/packages/ocap-kernel/src/store/methods/crank.cross-crank-gc.test.ts new file mode 100644 index 000000000..e503d7981 --- /dev/null +++ b/packages/ocap-kernel/src/store/methods/crank.cross-crank-gc.test.ts @@ -0,0 +1,85 @@ +import { describe, it, expect, beforeEach } from 'vitest'; + +import { makeMapKernelDatabase } from '../../../test/storage.ts'; +import { makeKernelStore } from '../index.ts'; + +/** + * `revertStateBeneathRollback` reverts `maybeFreeKrefs`, which nothing else + * rolls back. The set is not per-crank: only `collectGarbage` empties it, so a + * candidate added while no crank was open is still owed a collection and has to + * survive an unrelated crank's rollback. + * + * `RemoteManager.#handlePeerIncarnation` is one such producer. It runs from a + * network callback with no crank open, under its own `peerIncarnation_` + * savepoint, and `persistPeerRestart` -> `forgetEndpointImports` adds every + * export the restarting peer abandoned. It calls no `collectGarbage` of its own, + * so those krefs wait for the next crank's harvest -- and their kv state is + * already committed by the time it comes. A rollback that discarded them would + * leave the objects orphaned, undeleted, and invisible even to the reference + * count audit, which sees an orphan with no holders and a count of zero as + * consistent. + */ +describe('a GC candidate produced outside a crank', () => { + let kernelStore: ReturnType; + + /** + * Abandon a remote's export the way a peer restart does. + * + * @returns The kref of the now-ownerless object. + */ + function orphanARemoteExport(): string { + const kref = kernelStore.initKernelObject('r1'); + kernelStore.addCListEntry('r1', kref, 'o+1'); + // RemoteManager.#handlePeerIncarnation, inside its own savepoint, no crank. + kernelStore.forgetEndpointImports('r1'); + return kref; + } + + /** + * Run one crank, optionally rolling its delivery back. + * + * @param options - How the crank ends. + * @param options.rollback - Whether the delivery aborts. + */ + function runCrank({ rollback = false }: { rollback?: boolean } = {}): void { + kernelStore.startCrank(); + kernelStore.createCrankSavepoint('crank'); + kernelStore.createCrankSavepoint('delivery'); + if (rollback) { + kernelStore.rollbackCrank('delivery'); + } + kernelStore.collectGarbage(); + kernelStore.endCrank(); + } + + beforeEach(() => { + kernelStore = makeKernelStore(makeMapKernelDatabase()); + }); + + it('is collected by the next crank that succeeds', () => { + const kref = orphanARemoteExport(); + + runCrank(); + + expect(kernelStore.kernelRefExists(kref)).toBe(false); + }); + + it('is collected by the next crank that rolls back', () => { + const kref = orphanARemoteExport(); + + runCrank({ rollback: true }); + + expect(kernelStore.kernelRefExists(kref)).toBe(false); + }); + + it('survives a rollback of a crank that never touched it', () => { + const kref = orphanARemoteExport(); + + runCrank({ rollback: true }); + for (let crank = 0; crank < 5; crank += 1) { + runCrank(); + } + + expect(kernelStore.kernelRefExists(kref)).toBe(false); + }); +});