From e0e2e1f8e77d334703342e5b4856fc7f3660a21a Mon Sep 17 00:00:00 2001 From: just-some-random-pal <95859708+just-some-random-pal@users.noreply.github.com> Date: Wed, 14 Oct 2026 01:37:54 +0300 Subject: [PATCH] fix(sqlite3): run BEGIN, COMMIT and ROLLBACK through exec so a failed COMMIT releases its lock On a local file database, a COMMIT that failed with SQLITE_BUSY left the database write-locked until the process exited: every later write from any connection failed with SQLITE_BUSY, even after the failed transaction was rolled back and the reader that caused the busy error was gone. Through @prisma/adapter-libsql this surfaces as every query timing out. Transaction control went through db.prepare(sql).run(). When a prepared COMMIT throws, libsql leaves the statement un-reset, and that statement keeps the connection's lock past a successful ROLLBACK and past close(), until garbage collection finalizes it. exec() finalizes its statement before returning, failed or not. Route every BEGIN, COMMIT and ROLLBACK, including the pool's rollback on release, through exec(). migrate() now always returns its connection to the pool, even when the rollback after a failed COMMIT itself fails. Fixes #361 --- .../src/__tests__/client.test.ts | 67 +++++++++++++++++++ packages/libsql-client/src/sqlite3.ts | 49 ++++++++++---- 2 files changed, 103 insertions(+), 13 deletions(-) diff --git a/packages/libsql-client/src/__tests__/client.test.ts b/packages/libsql-client/src/__tests__/client.test.ts index a0d7e2f..754a132 100644 --- a/packages/libsql-client/src/__tests__/client.test.ts +++ b/packages/libsql-client/src/__tests__/client.test.ts @@ -2048,6 +2048,73 @@ describe("timeout option (local files)", () => { withoutTimeout.close(); } }); + + // A COMMIT that fails with SQLITE_BUSY must not leave the database locked: + // once the failed transaction is rolled back and the reader is gone, other + // connections must be able to write again. + for (const [label, failCommit] of [ + [ + "transaction commit()", + async (c: libsql.Client) => { + const txn = await c.transaction("deferred"); + await txn.execute("INSERT INTO t VALUES (1)"); + await expect(txn.commit()).rejects.toBeLibsqlError( + "SQLITE_BUSY", + ); + }, + ], + [ + "batch()", + async (c: libsql.Client) => { + await expect( + c.batch(["INSERT INTO t VALUES (1)"], "deferred"), + ).rejects.toBeLibsqlError("SQLITE_BUSY"); + }, + ], + [ + "migrate()", + async (c: libsql.Client) => { + await expect( + c.migrate([{ sql: "INSERT INTO t VALUES (1)", args: [] }]), + ).rejects.toBeLibsqlError("SQLITE_BUSY"); + }, + ], + ] as const) { + test(`a COMMIT that fails with SQLITE_BUSY in ${label} releases its lock`, async () => { + const setup = createClient({ url: dbUrl }); + await setup.execute("CREATE TABLE t (a)"); + setup.close(); + + const writer = createClient({ url: dbUrl }); + const reader = createClient({ url: dbUrl }); + try { + // A read transaction holds a SHARED lock, so the writer's COMMIT + // cannot take the EXCLUSIVE lock it needs. + const read = await reader.transaction("read"); + await read.execute("SELECT count(*) FROM t"); + + await failCommit(writer); + await read.rollback(); + + // Neither the writer's own pool nor a new connection may be left + // blocked by the failed COMMIT. + await writer.execute("INSERT INTO t VALUES (2)"); + const other = createClient({ url: dbUrl }); + try { + await other.execute("INSERT INTO t VALUES (3)"); + const rs = await other.execute( + "SELECT count(*) AS n FROM t", + ); + expect(rs.rows[0]["n"]).toStrictEqual(2); + } finally { + other.close(); + } + } finally { + writer.close(); + reader.close(); + } + }); + } }); (isFile ? test : test.skip)("raw error codes", async () => { diff --git a/packages/libsql-client/src/sqlite3.ts b/packages/libsql-client/src/sqlite3.ts index 3044da3..9b9b7a7 100644 --- a/packages/libsql-client/src/sqlite3.ts +++ b/packages/libsql-client/src/sqlite3.ts @@ -218,7 +218,7 @@ export class ConnectionPool { // the next borrower would silently inherit it. if (db.open && db.inTransaction) { try { - db.prepare("ROLLBACK").run(); + executeControl(db, "ROLLBACK"); } catch { // the connection is unusable; drop it rather than reuse it closeQuietly(db); @@ -339,7 +339,7 @@ export class Sqlite3Client implements Client { const db = await this.#pool.acquire(); try { this.#checkUsable(db); - executeStmt(db, transactionModeToBegin(mode), this.#intMode); + executeControl(db, transactionModeToBegin(mode)); const resultSets = []; for (let i = 0; i < stmts.length; i++) { try { @@ -374,7 +374,7 @@ export class Sqlite3Client implements Client { throw e; } } - executeStmt(db, "COMMIT", this.#intMode); + executeControl(db, "COMMIT"); return resultSets; } finally { // `release` rolls back anything still open before reuse @@ -388,7 +388,7 @@ export class Sqlite3Client implements Client { try { this.#checkUsable(db); executeStmt(db, "PRAGMA foreign_keys=off", this.#intMode); - executeStmt(db, transactionModeToBegin("deferred"), this.#intMode); + executeControl(db, transactionModeToBegin("deferred")); const resultSets = []; for (let i = 0; i < stmts.length; i++) { try { @@ -417,14 +417,21 @@ export class Sqlite3Client implements Client { throw e; } } - executeStmt(db, "COMMIT", this.#intMode); + executeControl(db, "COMMIT"); return resultSets; } finally { - if (db.inTransaction) { - executeStmt(db, "ROLLBACK", this.#intMode); + try { + if (db.open && db.inTransaction) { + executeControl(db, "ROLLBACK"); + } + if (db.open) { + executeStmt(db, "PRAGMA foreign_keys=on", this.#intMode); + } + } finally { + // always give the connection back; `release` drops it if the + // rollback above failed and it is still inside a transaction + this.#pool.release(db); } - executeStmt(db, "PRAGMA foreign_keys=on", this.#intMode); - this.#pool.release(db); } } @@ -433,7 +440,7 @@ export class Sqlite3Client implements Client { const db = await this.#pool.acquire(true); try { this.#checkUsable(db); - executeStmt(db, transactionModeToBegin(mode), this.#intMode); + executeControl(db, transactionModeToBegin(mode)); } catch (e) { this.#pool.release(db); throw e; @@ -601,7 +608,7 @@ export class Sqlite3Transaction implements Transaction { } try { this.#checkNotClosed(); - executeStmt(db, "ROLLBACK", this.#intMode); + executeControl(db, "ROLLBACK"); } finally { this.#settle(); } @@ -609,7 +616,7 @@ export class Sqlite3Transaction implements Transaction { async commit(): Promise { try { - executeStmt(this.#getDatabase(), "COMMIT", this.#intMode); + executeControl(this.#getDatabase(), "COMMIT"); } finally { this.#settle(); } @@ -624,7 +631,7 @@ export class Sqlite3Transaction implements Transaction { // `client.close()` may have closed this connection already, and // reading `inTransaction` on a closed database aborts the process. if (db.open && db.inTransaction) { - executeStmt(db, "ROLLBACK", this.#intMode); + executeControl(db, "ROLLBACK"); } } finally { this.#settle(); @@ -816,6 +823,22 @@ function executeMultiple(db: Database.Database, sql: string): void { } } +// Runs a transaction-control statement (BEGIN, COMMIT, ROLLBACK). These go +// through `exec` rather than a prepared statement: when a prepared COMMIT +// fails, for example with SQLITE_BUSY because another connection holds a +// SHARED lock, libsql leaves the statement un-reset, and an un-reset statement +// keeps the connection's lock after a successful ROLLBACK and even after +// close(), until garbage collection finalizes it. Every later write from any +// connection then fails with SQLITE_BUSY. `exec` finalizes its statement +// before returning, whether it succeeded or not. +function executeControl(db: Database.Database, sql: string): void { + try { + db.exec(sql); + } catch (e) { + throw mapSqliteError(e); + } +} + function mapSqliteError(e: unknown): unknown { if (e instanceof Database.SqliteError) { const extendedCode = e.code;