fix(db): keep the lock-free path for the transaction handle only (#408) #433

Merged
maximus merged 3 commits from issue-408-db-strict-lock into main 2026-10-14 00:28:08 +00:00
6 changed files with 556 additions and 113 deletions

View file

@ -142,6 +142,17 @@ Index Bilan (9) — 7 ajoutés en v9 :
- FK `balance_account_transfers.account_id` → `balance_accounts(id)` `ON DELETE CASCADE`
- FK `balance_account_transfers.transaction_id` → `transactions(id)` `ON DELETE RESTRICT` — décision structurante pour la reproductibilité Modified Dietz, voir [ADR 0010](adr/0010-fk-restrict-balance-transfers.md)
### Accès DB et transactions
`src/services/db.ts` fait passer **toute** opération sur la base par un verrou FIFO unique (`runExclusive`). tauri-plugin-sql charge SQLite dans un pool sqlx multi-connexions et n'expose aucune transaction côté JS : un `BEGIN` et son `COMMIT` envoyés par deux `execute` séparés pouvaient partir sur deux connexions et laisser une transaction d'écriture ouverte (le « database is locked » de la 0.10.0). Une lecture ou une écriture isolée prend le verrou le temps d'une instruction ; `withTransaction` le tient pour tout son rappel.
- **Verrou strict, non réentrant (#408).** Le rappel de `withTransaction` reçoit un **handle de transaction** (typé `Database`) qui exécute directement sur la connexion. Toute autre requête attend derrière la transaction, y compris un `getDb()` appelé depuis le rappel ou depuis une fonction qu'il appelle : celui-ci attend la transaction qui l'attend, un interblocage. Règle : dans un rappel, n'utiliser **que** le handle reçu et le passer aux fonctions appelées (par exemple `exec`, obligatoire sur `findOrCreateSecurity`). Avant #408, un drapeau global laissait passer hors verrou toute requête lancée pendant un rappel, d'où qu'elle vienne.
- **Handle révoqué.** À la fin du rappel (succès ou erreur), le handle est révoqué : un appel tardif depuis une promesse orpheline lève `TransactionHandleRevokedError` au lieu de passer hors verrou. `close()` sur le handle lève toujours `TransactionHandleCloseError`.
- **Changement de profil dans une seule section.** `connectToProfile`, `initializeNewProfileDb` et `closeDb` sont chacun **un seul** `runExclusive` : fermeture de l'ancien pool, puis `repair_migrations` et `prepare_profile_db` (pour `connectToProfile`, appelé seulement après la fin du `close()`), puis `Database.load` et la bascule. Ils attendent donc les opérations déjà en file et une transaction en cours, et rien ne s'exécute entre la fermeture et l'ouverture. Les helpers internes (`closeUnlocked`, `loadUnlocked`) ne prennent jamais le verrou. Pendant un rattrapage long, toute la file attend : rien d'utile ne peut tourner pendant la bascule.
- **Génération de connexion.** Chaque ouverture et chaque fermeture incrémentent un numéro de génération. Le proxy de `getDb()` et le handle de transaction le vérifient au moment où l'opération sort de la file : une opération mise en file pour une connexion fermée entre-temps lève `DbConnectionClosedError` (« connection closed by profile switch »), au lieu de tourner sur la base du profil suivant ou sur un pool fermé. Un handle obtenu par `getDb()` avant un changement de profil ne sert donc plus après : le reprendre par `getDb()`.
- **Alerte de blocage.** Une opération qui attend le verrou depuis plus de `BLOCKED_ALERT_MS` (15 s) écrit **un** `logWarn` au Journal, qui nomme ce qui tient le verrou (`transaction`, `query`, `profile opening`, `profile initialization`, `connection close`) et depuis quand. Cause typique : un `getDb()` dans un rappel. Un rattrapage de migrations de plus de 15 s produit une alerte « profile opening », attendue. `logService` n'écrit qu'en mémoire et en `sessionStorage`, jamais en base : l'appel est sûr pendant que le verrou est tenu.
- **Tests.** Seul `src/services/db.test.ts` voit le verrou (un `Database` factice qui journalise le début et la fin de chaque opération). Les tests de services mockent `db.ts` (`withTransaction: fn => fn(await getDb())`) et ne le voient pas.
## Système de migrations
Les migrations sont définies inline dans `src-tauri/src/lib.rs` via `tauri_plugin_sql::Migration` :
@ -183,7 +194,7 @@ Le dossier des bases est celui du plugin : `app_config_dir()`, résolu par `prof
| Service | Responsabilité |
|---------|---------------|
| `db.ts` | Wrapper de connexion (tauri-plugin-sql). `connectToProfile` appelle `repair_migrations` puis `prepare_profile_db` avant `Database.load`, et écrit au Journal ce que la préparation a changé (`describePreparation`, noms de fichiers seulement, #390) ; un échec est écrit au Journal et le profil s'ouvre quand même |
| `db.ts` | Wrapper de connexion (tauri-plugin-sql), verrou FIFO de tous les accès et `withTransaction` (voir « Accès DB et transactions »). `connectToProfile` appelle `repair_migrations` puis `prepare_profile_db` avant `Database.load`, et écrit au Journal ce que la préparation a changé (`describePreparation`, noms de fichiers seulement, #390) ; un échec est écrit au Journal et le profil s'ouvre quand même |
| `profileService.ts` | Gestion des profils |
| `categoryService.ts` | CRUD catégories hiérarchiques |
| `transactionService.ts` | CRUD et filtrage des transactions ; détection d'erreurs FK RESTRICT pour transactions liées à un compte de bilan (typed `TransactionLinkedToBalanceError`) ; `getTransactionCount` (compte brut de la table, sans filtre — signal des états vides de Transactions et Budget, #351/#353) |

View file

@ -2367,7 +2367,7 @@ describe("roundToCent", () => {
describe("findOrCreateSecurity", () => {
it("rejects an empty/whitespace symbol", async () => {
await expect(
findOrCreateSecurity({ symbol: " ", asset_type: "stock" })
findOrCreateSecurity({ symbol: " ", asset_type: "stock" }, mockDb)
).rejects.toMatchObject({ code: "security_symbol_required" });
expect(mockExecute).not.toHaveBeenCalled();
});
@ -2375,7 +2375,7 @@ describe("findOrCreateSecurity", () => {
it("rejects an invalid asset_type", async () => {
await expect(
// @ts-expect-error runtime guard
findOrCreateSecurity({ symbol: "AAPL", asset_type: "bond" })
findOrCreateSecurity({ symbol: "AAPL", asset_type: "bond" }, mockDb)
).rejects.toMatchObject({ code: "security_asset_type_invalid" });
expect(mockExecute).not.toHaveBeenCalled();
});
@ -2393,10 +2393,10 @@ describe("findOrCreateSecurity", () => {
updated_at: "",
},
]);
const sec = await findOrCreateSecurity({
symbol: " aapl ",
asset_type: "stock",
});
const sec = await findOrCreateSecurity(
{ symbol: " aapl ", asset_type: "stock" },
mockDb
);
expect(sec.id).toBe(3);
// The INSERT and the lookup both use the normalized symbol.
expect(mockExecute.mock.calls[0][1]?.[0]).toBe("AAPL");
@ -2410,26 +2410,29 @@ describe("findOrCreateSecurity", () => {
mockSelect.mockResolvedValueOnce([
{ id: 9, symbol: "BTC", name: null, currency: "CAD", asset_type: "crypto", created_at: "", updated_at: "" },
]);
const a = await findOrCreateSecurity({ symbol: "btc", asset_type: "crypto" });
const a = await findOrCreateSecurity({ symbol: "btc", asset_type: "crypto" }, mockDb);
// Second call, different casing — the normalized conflict target returns id 9.
mockExecute.mockResolvedValueOnce({ rowsAffected: 1 });
mockSelect.mockResolvedValueOnce([
{ id: 9, symbol: "BTC", name: null, currency: "CAD", asset_type: "crypto", created_at: "", updated_at: "" },
]);
const b = await findOrCreateSecurity({ symbol: " BtC ", asset_type: "crypto" });
const b = await findOrCreateSecurity({ symbol: " BtC ", asset_type: "crypto" }, mockDb);
expect(a.id).toBe(b.id);
expect(b.id).toBe(9);
});
it("threads an executor when given (in-txn create)", async () => {
it("runs on the executor it is given and never reaches for getDb() (#408)", async () => {
vi.mocked(getDb).mockClear();
const execSelect = vi.fn().mockResolvedValueOnce([
{ id: 1, symbol: "VEQT.TO", name: null, currency: "CAD", asset_type: "stock", created_at: "", updated_at: "" },
]);
const execExecute = vi.fn().mockResolvedValueOnce({ rowsAffected: 1 });
const exec = { select: execSelect, execute: execExecute };
await findOrCreateSecurity({ symbol: "veqt.to", asset_type: "stock" }, exec);
// The provided executor was used, NOT the top-level db handle.
// The provided executor was used, NOT the top-level db handle: a getDb()
// call from a transaction callback would deadlock behind the DB lock.
expect(execExecute).toHaveBeenCalledTimes(1);
expect(getDb).not.toHaveBeenCalled();
expect(mockExecute).not.toHaveBeenCalled();
expect(mockSelect).not.toHaveBeenCalled();
});

View file

@ -977,16 +977,18 @@ const ASSET_TYPES: readonly BalanceAssetType[] = ["stock", "crypto"];
* UPSERT a security by NORMALIZED symbol, returning its row. Idempotent: a
* second call with the same symbol (any casing) returns the existing row,
* updating `name`/`asset_type`/`currency` only when the caller passes new
* values. Callable inside an open transaction by threading `exec` (so a
* brand-new symbol can create its security then its holding atomically inside
* `saveSnapshotAtomic`); omit `exec` to use the top-level db handle.
* values. `exec` is mandatory: inside a transaction, the handle the
* `withTransaction` callback received (so a brand-new symbol can create its
* security then its holding atomically inside `saveSnapshotAtomic`); outside,
* `await getDb()`. No `getDb()` fallback here: called from a transaction
* callback, it would deadlock behind the DB lock (#408).
*
* @throws `security_symbol_required` on empty symbol,
* `security_asset_type_invalid` on a non-enum asset_type.
*/
export async function findOrCreateSecurity(
input: FindOrCreateSecurityInput,
exec?: SqlExecutor
exec: SqlExecutor
): Promise<BalanceSecurity> {
const symbol = normalizeSecuritySymbol(input.symbol ?? "");
if (symbol.length === 0) {
@ -1003,7 +1005,7 @@ export async function findOrCreateSecurity(
}
const currency = input.currency ?? BALANCE_CURRENCY_CAD;
const name = input.name ? input.name.trim() || null : null;
const db: SqlExecutor = exec ?? (await getDb());
const db = exec;
// ON CONFLICT(symbol) DO UPDATE keeps the row's metadata fresh (e.g. a name
// arriving on a later save) while preserving its id. COALESCE keeps an
// existing name when the new payload omits one. The column is UNIQUE COLLATE

View file

@ -1,4 +1,4 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
// Regression coverage for the "database is locked" fix (#228 follow-up).
//
@ -30,22 +30,28 @@ vi.mock("@tauri-apps/plugin-sql", () => ({
vi.mock("@tauri-apps/api/core", () => ({
invoke: vi.fn(async (command: string) => (command === "prepare_profile_db" ? h.prepare() : false)),
}));
vi.mock("./logService", () => ({ logInfo: vi.fn(), logError: vi.fn() }));
vi.mock("./logService", () => ({ logInfo: vi.fn(), logWarn: vi.fn(), logError: vi.fn() }));
import Database from "@tauri-apps/plugin-sql";
import { invoke } from "@tauri-apps/api/core";
import { logError, logInfo } from "./logService";
import { logError, logInfo, logWarn } from "./logService";
import {
BLOCKED_ALERT_MS,
closeDb,
connectToProfile,
DbConnectionClosedError,
describePreparation,
getDb,
initializeNewProfileDb,
TransactionHandleCloseError,
TransactionHandleRevokedError,
withTransaction,
type ProfilePreparation,
} from "./db";
// Each op records start, yields a real macrotask (a window for interleaving if
// the lock were broken), then records end. The lock must keep them ordered.
function makeFakeDb(): FakeDb {
function makeFakeDb(name = "db"): FakeDb {
return {
execute: vi.fn(async (q: string) => {
events.push(`start:${q}`);
@ -59,15 +65,19 @@ function makeFakeDb(): FakeDb {
events.push(`end:${q}`);
return [];
}),
close: vi.fn(async () => {}),
close: vi.fn(async () => {
events.push(`close:${name}`);
}),
};
}
beforeEach(async () => {
events.length = 0;
h.fakeDb = makeFakeDb();
h.prepare = async () => null;
await connectToProfile("test.db");
// Cleared after connecting: the connection opens by closing the previous
// test's fake, whose close is logged too.
events.length = 0;
});
describe("db serialization (database-locked fix)", () => {
@ -103,17 +113,97 @@ describe("db serialization (database-locked fix)", () => {
]);
});
it("lets a nested getDb() call run directly inside a transaction — no deadlock", async () => {
// A service called inside withTransaction might reach for getDb() (the
// serialized proxy) instead of the threaded handle. The reentrancy guard
// must run it directly rather than queue it behind the held lock (deadlock).
const result = await withTransaction(async () => {
const inner = await getDb();
await inner.execute("NESTED");
return "ok";
it("makes a getDb() call from inside a transaction wait behind it — strict lock (#408)", async () => {
// The lock is not reentrant: only the callback's handle runs directly. A
// nested service reaching for getDb() queues behind the transaction (this
// test releases the callback by hand; a callback awaiting it deadlocks,
// which the blocking alert reports).
let release: () => void = () => {};
const gate = new Promise<void>((resolve) => {
release = resolve;
});
expect(result).toBe("ok");
expect(events).toEqual(["start:NESTED", "end:NESTED"]);
let nested: Promise<unknown> = Promise.resolve();
const txn = withTransaction(async (tx) => {
await tx.execute("BEGIN");
nested = (await getDb()).execute("NESTED");
await new Promise((r) => setTimeout(r, 10));
await tx.execute("COMMIT");
await gate;
});
try {
await vi.waitFor(() => expect(events).toContain("end:COMMIT"));
expect(events).not.toContain("start:NESTED");
} finally {
// Released even on failure, so a red assertion leaves no held lock.
release();
await txn;
await nested;
}
expect(events).toEqual(["start:BEGIN", "end:BEGIN", "start:COMMIT", "end:COMMIT", "start:NESTED", "end:NESTED"]);
});
it("runs a read launched from elsewhere during the callback after the COMMIT (#408)", async () => {
// The pre-#408 global flag let ANY query bypass the lock while a callback
// ran: a read started after BEGIN ran in parallel with the transaction.
const db = await getDb();
let outside: Promise<unknown> = Promise.resolve();
const txn = withTransaction(async (tx) => {
await tx.execute("BEGIN");
outside = db.select("READ");
await tx.execute("WRITE");
await tx.execute("COMMIT");
});
await txn;
await outside;
expect(events).toEqual([
"start:BEGIN",
"end:BEGIN",
"start:WRITE",
"end:WRITE",
"start:COMMIT",
"end:COMMIT",
"start:READ",
"end:READ",
]);
});
it("lets the callback's own handle run its statements without waiting", async () => {
const rows = await withTransaction(async (tx) => {
await tx.execute("BEGIN");
const result = await tx.select<unknown[]>("SELECT");
await tx.execute("COMMIT");
return result;
});
expect(rows).toEqual([]);
expect(events).toEqual(["start:BEGIN", "end:BEGIN", "start:SELECT", "end:SELECT", "start:COMMIT", "end:COMMIT"]);
});
it("revokes the transaction handle once the callback settled", async () => {
let kept: Awaited<ReturnType<typeof getDb>> | null = null;
await withTransaction(async (tx) => {
kept = tx;
});
await expect(kept!.execute("LATE")).rejects.toBeInstanceOf(TransactionHandleRevokedError);
await expect(kept!.select("LATE")).rejects.toBeInstanceOf(TransactionHandleRevokedError);
expect(events).not.toContain("start:LATE");
});
it("revokes the handle of a failed transaction too", async () => {
let kept: Awaited<ReturnType<typeof getDb>> | null = null;
await expect(
withTransaction(async (tx) => {
kept = tx;
throw new Error("boom");
})
).rejects.toThrow("boom");
await expect(kept!.execute("LATE")).rejects.toBeInstanceOf(TransactionHandleRevokedError);
});
it("refuses close() on the transaction handle", async () => {
await withTransaction(async (tx) => {
await expect(tx.close()).rejects.toBeInstanceOf(TransactionHandleCloseError);
});
expect(h.fakeDb.close).not.toHaveBeenCalled();
});
it("releases the lock after a failed transaction so later ops still run", async () => {
@ -139,6 +229,192 @@ describe("db serialization (database-locked fix)", () => {
});
});
// ---------------------------------------------------------------------------
// Profile switch through the lock, connection generation, blocking alert (#408)
// ---------------------------------------------------------------------------
describe("profile switch goes through the lock (#408)", () => {
afterEach(() => {
vi.mocked(Database.load).mockImplementation(async () => h.fakeDb as never);
});
it("closes the connection only after a running transaction's COMMIT", async () => {
const old = h.fakeDb;
let switching: Promise<void> = Promise.resolve();
const txn = withTransaction(async (tx) => {
await tx.execute("BEGIN");
h.fakeDb = makeFakeDb("next");
switching = connectToProfile("next.db");
await new Promise((r) => setTimeout(r, 10));
await tx.execute("COMMIT");
});
await txn;
await switching;
expect(events).toEqual(["start:BEGIN", "end:BEGIN", "start:COMMIT", "end:COMMIT", "close:db"]);
expect(old.close).toHaveBeenCalledTimes(1);
});
it("two concurrent switches leave one pool open, on the last file asked for", async () => {
const pools = new Map<string, FakeDb>();
vi.mocked(Database.load).mockImplementation(async (url: string) => {
const fake = makeFakeDb(url);
pools.set(url, fake);
return fake as never;
});
await Promise.all([connectToProfile("a.db"), connectToProfile("b.db")]);
expect([...pools.keys()]).toEqual(["sqlite:a.db", "sqlite:b.db"]);
expect(pools.get("sqlite:a.db")!.close).toHaveBeenCalledTimes(1);
expect(pools.get("sqlite:b.db")!.close).not.toHaveBeenCalled();
await (await getDb()).execute("Q");
expect(pools.get("sqlite:b.db")!.execute).toHaveBeenCalledWith("Q", undefined);
});
it("invokes prepare_profile_db only once the old connection's close() resolved", async () => {
const old = h.fakeDb;
vi.mocked(old.close).mockImplementation(async () => {
events.push("close-start");
await new Promise((r) => setTimeout(r, 10));
events.push("close-end");
});
h.prepare = async () => {
events.push("prepare");
return null;
};
h.fakeDb = makeFakeDb("next");
await connectToProfile("next.db");
expect(events).toEqual(["close-start", "close-end", "prepare"]);
});
it("fails a transaction queued behind a switch instead of running it on the new database", async () => {
const old = h.fakeDb;
const next = makeFakeDb("next");
h.fakeDb = next;
const switching = connectToProfile("next.db");
const callback = vi.fn(async () => "ran");
const txn = withTransaction(callback);
await switching;
await expect(txn).rejects.toBeInstanceOf(DbConnectionClosedError);
await expect(txn).rejects.toThrow("connection closed by profile switch");
expect(callback).not.toHaveBeenCalled();
expect(next.execute).not.toHaveBeenCalled();
expect(old.execute).not.toHaveBeenCalled();
});
it("fails a query queued on the old handle behind a switch", async () => {
const before = await getDb();
const next = makeFakeDb("next");
h.fakeDb = next;
const switching = connectToProfile("next.db");
const late = before.select("OLD");
await switching;
await expect(late).rejects.toBeInstanceOf(DbConnectionClosedError);
expect(next.select).not.toHaveBeenCalled();
});
it("closeDb waits for the queue, then the old handle refuses to run", async () => {
const db = await getDb();
const first = db.execute("FIRST");
const closing = closeDb();
await first;
await closing;
expect(events).toEqual(["start:FIRST", "end:FIRST", "close:db"]);
await expect(db.execute("AFTER")).rejects.toBeInstanceOf(DbConnectionClosedError);
});
it("initializeNewProfileDb runs its SQL inside the section, before a queued query", async () => {
const next = makeFakeDb("new");
h.fakeDb = next;
const init = initializeNewProfileDb("new.db", ["CREATE A", "CREATE B"]);
const after = (async () => {
await init;
return (await getDb()).select("READ");
})();
await after;
expect(events).toEqual([
"close:db",
"start:CREATE A",
"end:CREATE A",
"start:CREATE B",
"end:CREATE B",
"start:READ",
"end:READ",
]);
});
});
describe("blocking alert (#408)", () => {
beforeEach(() => {
vi.mocked(logWarn).mockClear();
vi.useFakeTimers();
});
afterEach(() => {
vi.useRealTimers();
});
it("warns once, naming the transaction that holds the lock, when getDb() is called from a callback", async () => {
let release: () => void = () => {};
const gate = new Promise<void>((resolve) => {
release = resolve;
});
let nested: Promise<unknown> = Promise.resolve();
const startedAt = new Date(Date.now()).toISOString();
const txn = withTransaction(async () => {
nested = (await getDb()).execute("NESTED");
await gate;
});
try {
await vi.advanceTimersByTimeAsync(BLOCKED_ALERT_MS - 1);
expect(logWarn).not.toHaveBeenCalled();
await vi.advanceTimersByTimeAsync(1);
expect(logWarn).toHaveBeenCalledTimes(1);
const message = String(vi.mocked(logWarn).mock.calls[0][0]);
expect(message).toContain("a query has waited 15 s");
expect(message).toContain(`held by a transaction since ${startedAt}`);
// Once per operation: still one warning much later.
await vi.advanceTimersByTimeAsync(BLOCKED_ALERT_MS * 3);
expect(logWarn).toHaveBeenCalledTimes(1);
} finally {
release();
await vi.advanceTimersByTimeAsync(10);
await txn;
await nested;
}
});
it("says nothing for an operation that got the lock in time", async () => {
const db = await getDb();
const run = db.execute("QUICK");
await vi.advanceTimersByTimeAsync(10);
await run;
await vi.advanceTimersByTimeAsync(BLOCKED_ALERT_MS * 2);
expect(logWarn).not.toHaveBeenCalled();
});
it("names a profile opening that holds the lock (long catch-up)", async () => {
let finish: () => void = () => {};
h.prepare = () =>
new Promise((resolve) => {
finish = () => resolve(null);
});
h.fakeDb = makeFakeDb("next");
const switching = connectToProfile("next.db");
// Settled outcome caught at once: the rejection lands while timers advance.
const waiting = withTransaction(async () => "late").catch((e: unknown) => e);
try {
await vi.advanceTimersByTimeAsync(BLOCKED_ALERT_MS);
expect(logWarn).toHaveBeenCalledTimes(1);
expect(String(vi.mocked(logWarn).mock.calls[0][0])).toContain(
"a transaction has waited 15 s, the lock is held by a profile opening"
);
} finally {
finish();
await vi.advanceTimersByTimeAsync(10);
await switching;
}
expect(await waiting).toBeInstanceOf(DbConnectionClosedError);
});
});
// ---------------------------------------------------------------------------
// Profile preparation (#390)
// ---------------------------------------------------------------------------

View file

@ -1,12 +1,12 @@
import Database from "@tauri-apps/plugin-sql";
import { invoke } from "@tauri-apps/api/core";
import { logError, logInfo } from "./logService";
import { logError, logInfo, logWarn } from "./logService";
let rawInstance: Database | null = null;
let serializedInstance: Database | null = null;
// ---------------------------------------------------------------------------
// DB access serialization (#228 follow-up — fix "database is locked").
// DB access serialization (#228 follow-up — fix "database is locked"; #408).
//
// tauri-plugin-sql exposes NO transaction primitive to the JS side and loads
// SQLite through a default multi-connection sqlx pool (`Pool::connect` =>
@ -23,20 +23,101 @@ let serializedInstance: Database | null = null;
// statements all run on the same connection and can never strand. Serializing
// all DB access is a non-issue for a local single-user app where queries are
// tiny and infrequent — and it makes the "atomic" saves genuinely atomic.
//
// Rules (#408):
// - The lock is strict and NOT reentrant. Inside a `withTransaction` callback,
// use ONLY the handle the callback receives: it runs directly on the
// connection the transaction holds. Calling `getDb()` from a callback (or
// from anything it calls) queues behind the transaction that waits for it:
// a deadlock, reported in the Journal after BLOCKED_ALERT_MS (see below).
// - The transaction handle is revoked when the callback settles: a call on it
// afterwards (an orphan promise) throws `TransactionHandleRevokedError`
// instead of running outside the lock. `close()` on it always throws.
// - Opening, closing and switching a profile each run as ONE exclusive section
// (`connectToProfile`, `initializeNewProfileDb`, `closeDb`), so they wait for
// the operations already queued and for a running transaction, and nothing
// runs between closing the old pool and opening the new one.
// - Every open or close bumps a connection generation. A handle checks its
// generation when its operation leaves the queue: an operation queued for a
// connection that was closed meanwhile throws `DbConnectionClosedError`
// rather than running on the next profile's database or on a closed pool.
// ---------------------------------------------------------------------------
let lockTail: Promise<unknown> = Promise.resolve();
/** An operation queued for a connection closed before it could run. */
export class DbConnectionClosedError extends Error {
constructor() {
super("connection closed by profile switch");
this.name = "DbConnectionClosedError";
}
}
// True only while a `withTransaction` callback is executing. The lock is held
// for that whole callback, so anything it triggers (the threaded handle, or an
// accidental `getDb()` call from a nested service) must run DIRECTLY rather
// than re-entering the non-reentrant lock — otherwise it would deadlock behind
// the transaction that is waiting for it.
let inTransaction = false;
/** A transaction handle used after its `withTransaction` callback settled. */
export class TransactionHandleRevokedError extends Error {
constructor() {
super("transaction handle used after its withTransaction callback ended");
this.name = "TransactionHandleRevokedError";
}
}
/** `close()` called on a transaction handle: only the profile functions close. */
export class TransactionHandleCloseError extends Error {
constructor() {
super("close() is not allowed on a transaction handle; use closeDb()");
this.name = "TransactionHandleCloseError";
}
}
/** What holds the lock, named in the blocking alert. */
type LockKind =
| "query"
| "transaction"
| "profile opening"
| "profile initialization"
| "connection close";
/**
* An operation still waiting for the lock after this delay writes ONE warning
* to the Journal, naming what holds the lock and since when. The typical cause
* is a `getDb()` call from inside a `withTransaction` callback (deadlock). A
* profile catch-up (`prepare_profile_db`) that takes longer also triggers it,
* naming a "profile opening": expected, nothing can run during a switch.
* `logWarn` only writes to memory and sessionStorage, never to the DB, so it
* is safe to call while the lock is held.
*/
export const BLOCKED_ALERT_MS = 15_000;
let lockTail: Promise<unknown> = Promise.resolve();
let lockHolder: { kind: LockKind; since: number } | null = null;
let generation = 0;
/** Run `fn` after every previously-queued DB operation has settled (FIFO). */
function runExclusive<T>(fn: () => Promise<T>): Promise<T> {
const result = lockTail.then(fn, fn);
function runExclusive<T>(kind: LockKind, fn: () => Promise<T>): Promise<T> {
const queuedAt = Date.now();
let started = false;
const alert = setTimeout(() => {
if (started) return;
const holder = lockHolder;
const waited = Math.round((Date.now() - queuedAt) / 1000);
logWarn(
holder
? `DB lock: a ${kind} has waited ${waited} s, the lock is held by a ${holder.kind} since ${new Date(holder.since).toISOString()}` +
(holder.kind === "transaction"
? " (a getDb() call inside a withTransaction callback deadlocks: use the callback's handle)"
: "")
: `DB lock: a ${kind} has waited ${waited} s behind the queue`
);
}, BLOCKED_ALERT_MS);
const run = async (): Promise<T> => {
started = true;
clearTimeout(alert);
lockHolder = { kind, since: Date.now() };
try {
return await fn();
} finally {
lockHolder = null;
}
};
const result = lockTail.then(run, run);
// Keep the chain alive regardless of fn's outcome; only the tail swallows.
lockTail = result.then(
() => undefined,
@ -45,26 +126,38 @@ function runExclusive<T>(fn: () => Promise<T>): Promise<T> {
return result;
}
/** Throw when the connection a handle was made for is gone. */
function assertGeneration(handleGeneration: number): void {
if (handleGeneration !== generation) {
throw new DbConnectionClosedError();
}
}
/**
* Wrap the raw Database so `execute`/`select` are serialized through the lock.
* While a transaction holds the lock (`inTransaction`), calls run directly so a
* nested call never deadlocks. The proxy keeps the `Database` type, so call
* sites (including `db.select<T>(...)`) are unchanged.
* Wrap the raw Database so `execute`/`select` are serialized through the lock
* and checked against the connection generation once they leave the queue.
* The proxy keeps the `Database` type, so call sites (including
* `db.select<T>(...)`) are unchanged.
*/
function serialize(db: Database): Database {
function serialize(db: Database, handleGeneration: number): Database {
return new Proxy(db, {
get(target, prop, receiver) {
if (prop === "execute") {
return (query: string, bindValues?: unknown[]) =>
inTransaction
? target.execute(query, bindValues)
: runExclusive(() => target.execute(query, bindValues));
runExclusive("query", async () => {
assertGeneration(handleGeneration);
return target.execute(query, bindValues);
});
}
if (prop === "select") {
return (query: string, bindValues?: unknown[]) =>
inTransaction
? target.select(query, bindValues)
: runExclusive(() => target.select(query, bindValues));
runExclusive("query", async () => {
assertGeneration(handleGeneration);
return target.select(query, bindValues);
});
}
if (prop === "close") {
return () => closeDb();
}
const value = Reflect.get(target, prop, receiver);
return typeof value === "function" ? value.bind(target) : value;
@ -72,6 +165,51 @@ function serialize(db: Database): Database {
});
}
/**
* The handle a `withTransaction` callback receives: runs directly on the
* connection (the transaction already holds the lock), refuses `close()`, and
* throws once `revoke()` ran at the end of the callback.
*/
function transactionHandle(
db: Database,
handleGeneration: number
): { handle: Database; revoke: () => void } {
let active = true;
const check = () => {
if (!active) throw new TransactionHandleRevokedError();
assertGeneration(handleGeneration);
};
const handle = new Proxy(db, {
get(target, prop, receiver) {
if (prop === "execute") {
return async (query: string, bindValues?: unknown[]) => {
check();
return target.execute(query, bindValues);
};
}
if (prop === "select") {
return async (query: string, bindValues?: unknown[]) => {
check();
return target.select(query, bindValues);
};
}
if (prop === "close") {
return async () => {
throw new TransactionHandleCloseError();
};
}
const value = Reflect.get(target, prop, receiver);
return typeof value === "function" ? value.bind(target) : value;
},
});
return {
handle,
revoke: () => {
active = false;
},
};
}
export async function getDb(): Promise<Database> {
if (!serializedInstance) {
throw new Error("No database connection. Call connectToProfile() first.");
@ -82,28 +220,51 @@ export async function getDb(): Promise<Database> {
/**
* Run `fn` as one serialized transaction. The global DB lock is held for the
* ENTIRE callback so no other db.execute/select can interleave and strand the
* transaction on a different pooled connection. `fn` receives the db handle and
* is responsible for issuing BEGIN/COMMIT (and ROLLBACK on error), exactly as
* the inline pattern did before — `withTransaction` only guarantees exclusive,
* non-interleaved access for the whole BEGIN..COMMIT.
* transaction on a different pooled connection. `fn` receives the transaction
* handle and is responsible for issuing BEGIN/COMMIT (and ROLLBACK on error)
* on it — `withTransaction` only guarantees exclusive, non-interleaved access
* for the whole BEGIN..COMMIT. Never call `getDb()` from `fn` (deadlock).
*/
export async function withTransaction<T>(
fn: (db: Database) => Promise<T>
): Promise<T> {
if (!serializedInstance) {
if (!rawInstance) {
throw new Error("No database connection. Call connectToProfile() first.");
}
const db = serializedInstance;
return runExclusive(async () => {
inTransaction = true;
const db = rawInstance;
const handleGeneration = generation;
return runExclusive("transaction", async () => {
assertGeneration(handleGeneration);
const { handle, revoke } = transactionHandle(db, handleGeneration);
try {
return await fn(db);
return await fn(handle);
} finally {
inTransaction = false;
revoke();
}
});
}
// Internal helpers for the profile sections below. They NEVER take the lock:
// only call them from inside a `runExclusive` section.
async function closeUnlocked(): Promise<void> {
if (rawInstance) {
const closing = rawInstance;
rawInstance = null;
serializedInstance = null;
generation += 1;
await closing.close();
}
}
async function loadUnlocked(dbFilename: string): Promise<Database> {
const db = await Database.load(`sqlite:${dbFilename}`);
generation += 1;
rawInstance = db;
serializedInstance = serialize(db, generation);
return db;
}
/** What `prepare_profile_db` did to one profile database (#390). */
export interface ProfilePreparation {
catch_up: {
@ -159,12 +320,10 @@ export function describePreparation(
return { info, errors };
}
export async function connectToProfile(dbFilename: string): Promise<void> {
if (rawInstance) {
await rawInstance.close();
rawInstance = null;
serializedInstance = null;
}
export function connectToProfile(dbFilename: string): Promise<void> {
return runExclusive("profile opening", async () => {
// The old pool is fully closed before the preparation touches the file.
await closeUnlocked();
// Repair migration checksums before loading (fixes "migration was modified" error)
try {
const repaired = await invoke<boolean>("repair_migrations", { dbFilename });
@ -187,27 +346,20 @@ export async function connectToProfile(dbFilename: string): Promise<void> {
`Profile ${dbFilename}: database preparation failed: ${e instanceof Error ? e.message : String(e)}`
);
}
rawInstance = await Database.load(`sqlite:${dbFilename}`);
serializedInstance = serialize(rawInstance);
await loadUnlocked(dbFilename);
});
}
export async function initializeNewProfileDb(dbFilename: string, sqlStatements: string[]): Promise<void> {
if (rawInstance) {
await rawInstance.close();
rawInstance = null;
serializedInstance = null;
}
rawInstance = await Database.load(`sqlite:${dbFilename}`);
serializedInstance = serialize(rawInstance);
export function initializeNewProfileDb(dbFilename: string, sqlStatements: string[]): Promise<void> {
return runExclusive("profile initialization", async () => {
await closeUnlocked();
const db = await loadUnlocked(dbFilename);
for (const sql of sqlStatements) {
await rawInstance.execute(sql);
await db.execute(sql);
}
});
}
export async function closeDb(): Promise<void> {
if (rawInstance) {
await rawInstance.close();
rawInstance = null;
serializedInstance = null;
}
export function closeDb(): Promise<void> {
return runExclusive("connection close", () => closeUnlocked());
}

View file

@ -126,9 +126,8 @@ export async function getOnboardingSnapshot(): Promise<OnboardingSnapshot> {
* it.
*
* The flag is read first, outside any transaction: once D9 ran, every later
* opening costs that one queued read and opens no transaction (while a
* `withTransaction` callback runs, `db.ts` lets every other query bypass its
* lock). Without the flag, D9 runs as one transaction under the DB lock, flag
* opening costs that one queued read and opens no transaction. Without the
* flag, D9 runs as one transaction under the DB lock, flag
* re-read inside it, so two concurrent first loads cannot apply it twice
* (CWE-362): the hidden preference is written first, then the flag, then
* COMMIT; any failure rolls both back and leaves the flag unset, so the next