From a7c62f737490e1128de36eb5beaabd7cd2945803 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 15:36:34 +0000 Subject: [PATCH 1/2] fix(driver-sql): close the SQLite connection a refused connect() opened Claude-Session: https://claude.ai/code/session_01WQM1Hwm3eDA8xPhFuZGmZ6 Co-authored-by: Claude --- ...084-sqlite-refused-connect-release.test.ts | 212 ++++++++++++++++++ packages/drivers/driver-sql/src/sql-driver.ts | 70 ++++-- 2 files changed, 268 insertions(+), 14 deletions(-) create mode 100644 packages/drivers/driver-sql/src/sql-driver-25084-sqlite-refused-connect-release.test.ts diff --git a/packages/drivers/driver-sql/src/sql-driver-25084-sqlite-refused-connect-release.test.ts b/packages/drivers/driver-sql/src/sql-driver-25084-sqlite-refused-connect-release.test.ts new file mode 100644 index 00000000000..ac43b77e862 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-25084-sqlite-refused-connect-release.test.ts @@ -0,0 +1,212 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A SQLite `connect()` that refuses its file closes what it opened before it + * rejects (#25084). + * + * The refusal ({@link SqliteDatabaseFileError}: `SQLITE_CANTOPEN`, + * `SQLITE_NOTADB`, `SQLITE_CORRUPT`) is the answer of the connection's first + * statement, so the file was opened to give it. The pool used to keep that + * better-sqlite3 connection: for a damaged file whose header says WAL, SQLite + * kept its `-shm` and an empty `-wal` beside it until something called + * `disconnect()`, and a caller that never does — the datasource connection + * service's degraded path — kept them for the life of the process. + * + * Pinned with no `disconnect()` before any reading: what is left behind is + * read straight after the rejection. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { + existsSync, mkdtempSync, readdirSync, readFileSync, readlinkSync, renameSync, rmSync, symlinkSync, truncateSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { SqlDriver } from './sql-driver.js'; +import { SqliteDatabaseFileError } from './sqlite-database-file.js'; + +const drivers: SqlDriver[] = []; +function make(filename: string): SqlDriver { + const d = new SqlDriver({ client: 'better-sqlite3', connection: { filename }, useNullAsDefault: true }); + drivers.push(d); + return d; +} + +/** `connect()`'s rejection, or a failure saying it resolved. */ +async function refusalOf(driver: SqlDriver): Promise { + try { + await driver.connect(); + } catch (error) { + return error; + } + throw new Error('connect() resolved for a file the datasource cannot use'); +} + +function expectFileRefusal(error: any, filename: string, code: string): void { + expect(error).toBeInstanceOf(SqliteDatabaseFileError); + expect(error.code).toBe(code); + expect(error.filename).toBe(filename); + expect(error.message).toContain(filename); +} + +/** A real SQLite database in WAL (this driver's journal mode), holding one row in `t`. */ +async function seedDatabase(filename: string): Promise { + const seed = new SqlDriver({ client: 'better-sqlite3', connection: { filename }, useNullAsDefault: true }); + await seed.connect(); + await seed.execute('CREATE TABLE t (a INTEGER)'); + await seed.execute('INSERT INTO t (a) VALUES (7)'); + await seed.disconnect(); +} + +/** A seeded database cut short of the pages its schema names: its header still says WAL. */ +async function damagedWalDatabase(filename: string): Promise { + await seedDatabase(filename); + truncateSync(filename, 4096); + const before = readFileSync(filename); + // Byte 18 is the header's write version: 2 is WAL. + expect(before[18]).toBe(2); + return before; +} + +/** + * This process's open file descriptors on `filename` (or on any of its SQLite + * siblings), read from `/proc/self/fd` — the operating system's own count of + * open handles, so no driver internals are trusted to report themselves. + */ +function openHandlesOn(filename: string): string[] { + const fds = '/proc/self/fd'; + const open: string[] = []; + for (const fd of readdirSync(fds)) { + let target: string; + try { + target = readlinkSync(join(fds, fd)); + } catch { + continue; // the descriptor readdir itself used, already closed + } + if (target.startsWith(filename)) open.push(target); + } + return open; +} + +let dir: string; +beforeEach(() => { dir = mkdtempSync(join(tmpdir(), 'os-25084-')); }); +afterEach(async () => { + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + await Promise.all(drivers.splice(0).map((d) => d.disconnect().catch(() => {}))); + rmSync(dir, { recursive: true, force: true }); +}); + +describe('SqlDriver.connect — a refused SQLite file leaves no open handle behind (#25084)', () => { + it('a damaged file whose header says WAL: same SQLITE_CORRUPT refusal, and no -shm / -wal sibling once connect() rejects (NODE_ENV=production)', async () => { + vi.stubEnv('NODE_ENV', 'production'); + const filename = join(dir, 'archive.db'); + const before = await damagedWalDatabase(filename); + + const error = await refusalOf(make(filename)); + + expectFileRefusal(error, filename, 'SQLITE_CORRUPT'); + expect((error.cause as { code?: string }).code).toBe('SQLITE_CORRUPT'); + expect(error.message).toContain((error.cause as Error).message); + // Read before any disconnect(): the refused connection is already closed. + expect(readdirSync(dir)).toEqual(['archive.db']); + expect(readFileSync(filename).equals(before)).toBe(true); + }); + + describe.runIf(existsSync('/proc/self/fd'))('the handle itself, counted by the operating system', () => { + it('SQLITE_NOTADB: the same refusal, and no descriptor left open on the file', async () => { + const filename = join(dir, 'archive.db'); + writeFileSync(filename, 'archive notes — plain text, not a SQLite database\n'); + + const error = await refusalOf(make(filename)); + + expectFileRefusal(error, filename, 'SQLITE_NOTADB'); + expect((error.cause as { code?: string }).code).toBe('SQLITE_NOTADB'); + expect(openHandlesOn(filename)).toEqual([]); + expect(readdirSync(dir)).toEqual(['archive.db']); + }); + + it('SQLITE_CORRUPT: the same refusal, and no descriptor left open on the file or its siblings', async () => { + const filename = join(dir, 'archive.db'); + await damagedWalDatabase(filename); + + expectFileRefusal(await refusalOf(make(filename)), filename, 'SQLITE_CORRUPT'); + expect(openHandlesOn(filename)).toEqual([]); + }); + + it('SQLITE_CANTOPEN: the same refusal, and no descriptor left open', async () => { + // A link to a file in a directory that does not exist: SQLite cannot open it. + const filename = join(dir, 'archive.db'); + symlinkSync(join(dir, 'missing', 'target.db'), filename); + + const error = await refusalOf(make(filename)); + + expectFileRefusal(error, filename, 'SQLITE_CANTOPEN'); + expect(openHandlesOn(filename)).toEqual([]); + expect(openHandlesOn(join(dir, 'missing'))).toEqual([]); + }); + + it('CONTROL: a healthy file connects and keeps its handle until disconnect()', async () => { + const filename = join(dir, 'archive.db'); + await seedDatabase(filename); + + const driver = make(filename); + await driver.connect(); + expect(await driver.execute('SELECT a FROM t')).toEqual([{ a: 7 }]); + expect(openHandlesOn(filename)).not.toEqual([]); + + await driver.disconnect(); + expect(openHandlesOn(filename)).toEqual([]); + }); + }); + + it('the refused instance connects again: the same refusal while the file is bad, and a working connection once it is repaired', async () => { + // A host-adopted driver is connected again on the SAME object + // (`createPrebuiltDriverFactory`), so releasing must leave it usable. + const filename = join(dir, 'archive.db'); + await damagedWalDatabase(filename); + const driver = make(filename); + + expectFileRefusal(await refusalOf(driver), filename, 'SQLITE_CORRUPT'); + expectFileRefusal(await refusalOf(driver), filename, 'SQLITE_CORRUPT'); + expect(readdirSync(dir)).toEqual(['archive.db']); + + // The operator restores the file from a good copy. + const good = join(dir, 'restored.db'); + await seedDatabase(good); + rmSync(filename); + renameSync(good, filename); + + await driver.connect(); + expect(await driver.execute('SELECT a FROM t')).toEqual([{ a: 7 }]); + }); + + it('a release that itself fails is logged at warn and the original refusal goes on unchanged', async () => { + const filename = join(dir, 'archive.db'); + await damagedWalDatabase(filename); + const driver = make(filename); + const knex = (driver as any).knex; + const realDestroy = knex.destroy.bind(knex); + const releaseFailure = new Error('pool would not close'); + vi.spyOn(knex, 'destroy').mockRejectedValueOnce(releaseFailure).mockImplementation(realDestroy); + const warn = vi.spyOn((driver as any).logger, 'warn'); + + const error = await refusalOf(driver); + + expectFileRefusal(error, filename, 'SQLITE_CORRUPT'); + expect(error).not.toBe(releaseFailure); + expect(warn).toHaveBeenCalledWith(expect.any(String), releaseFailure); + }); + + it('CONTROL: a healthy WAL file connects and keeps working, its -shm / -wal held open beside it', async () => { + const filename = join(dir, 'archive.db'); + await seedDatabase(filename); + + const driver = make(filename); + await driver.connect(); + await driver.execute('INSERT INTO t (a) VALUES (8)'); + expect(await driver.execute('SELECT a FROM t ORDER BY a')).toEqual([{ a: 7 }, { a: 8 }]); + expect(readdirSync(dir).sort()).toEqual(['archive.db', 'archive.db-shm', 'archive.db-wal']); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 778b08f36ba..b4bd7774924 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -7004,22 +7004,64 @@ export class SqlDriver implements IDataDriver { // =================================== async connect(): Promise { - // Ensure the database directory exists before any query can trigger - // better-sqlite3 to open the file (e.g. loadMetaFromDb on startup). - await this.ensureDatabaseExists(); + try { + // Ensure the database directory exists before any query can trigger + // better-sqlite3 to open the file (e.g. loadMetaFromDb on startup). + await this.ensureDatabaseExists(); - if (this.isSqlite) { - // Both pragmas below are persistent properties of the FILE, so one - // connection setting them is enough for every process that follows. - // Each is READ first and set only when the file answers differently, so - // a connect to a file an earlier boot already configured writes nothing - // to it (#21734) — one rule for every connect, read-only boots included. - // A `false` means the very first statement on this connection failed — - // in practice a native addon that cannot load — so asking it for another - // pragma would only repeat the same warning. - if (await this.applyAutoVacuumIncremental()) { - await this.applySqliteJournalMode(); + if (this.isSqlite) { + // Both pragmas below are persistent properties of the FILE, so one + // connection setting them is enough for every process that follows. + // Each is READ first and set only when the file answers differently, so + // a connect to a file an earlier boot already configured writes nothing + // to it (#21734) — one rule for every connect, read-only boots included. + // A `false` means the very first statement on this connection failed — + // in practice a native addon that cannot load — so asking it for another + // pragma would only repeat the same warning. + if (await this.applyAutoVacuumIncremental()) { + await this.applySqliteJournalMode(); + } } + } catch (error) { + if (this.isSqlite) await this.releaseRefusedSqliteConnection(); + throw error; + } + } + + /** + * Close what a refused SQLite `connect()` opened, before the refusal leaves + * it (#25084). + * + * A file refused by its first statement ({@link SQLITE_FILE_REFUSALS}) was + * opened to answer it: the pool keeps that better-sqlite3 connection, and for + * a file whose header says WAL, SQLite keeps its `-shm` and an empty `-wal` + * beside it for as long as the connection is open. No caller is owed that + * handle — the connect failed — and a caller that does not `disconnect()` a + * handle whose `connect()` rejected (the datasource connection service's + * degraded path) used to keep it for the life of the process. + * + * The pool is destroyed and a fresh, empty one put in its place, so the + * instance is left as it was constructed: nothing open, and a later + * `connect()` opens the file again and gives its own answer. Destroying + * alone would not: every later statement would answer "Unable to acquire a + * connection", which carries no code, so a second `connect()` would log it + * and resolve over a datasource that cannot run one query. A host-adopted + * instance is connected again on the same object (`createPrebuiltDriverFactory` + * in `@objectstack/service-datasource`), so that second `connect()` is real. + * + * SQLite only: it is the one dialect whose `connect()` can reject holding a + * connection. A server dialect rejects there only when the connection itself + * could not be made, so its pool holds nothing to release. + * + * Never throws: the refusal the caller is owed is the error `connect()` + * raised, so a release that fails is logged and that error goes on unchanged. + */ + private async releaseRefusedSqliteConnection(): Promise { + try { + await this.knex.destroy(); + this.knex.initialize(); + } catch (e) { + this.logger.warn('Failed to close the SQLite connection a refused connect() opened', e); } } From 808e62f0df782808520bfa76b5e02213c9ff0d1d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 15:40:17 +0000 Subject: [PATCH 2/2] chore(driver-sql): changeset, and the #25019 pin's note on the refused connection's siblings Claude-Session: https://claude.ai/code/session_01WQM1Hwm3eDA8xPhFuZGmZ6 Co-authored-by: Claude --- .changeset/25084-sqlite-refused-connect-release.md | 13 +++++++++++++ ...19-sqlite-damaged-file-and-dangling-link.test.ts | 8 +++++--- 2 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 .changeset/25084-sqlite-refused-connect-release.md diff --git a/.changeset/25084-sqlite-refused-connect-release.md b/.changeset/25084-sqlite-refused-connect-release.md new file mode 100644 index 00000000000..059e3795534 --- /dev/null +++ b/.changeset/25084-sqlite-refused-connect-release.md @@ -0,0 +1,13 @@ +--- +'@objectstack/driver-sql': patch +--- + +fix(driver-sql): a SQLite `connect()` that refuses its file closes the connection it opened before it rejects + +Clause-②: no (resource release only: no response, status, error, outcome or export moves) +Metadata-protocol: none + +- **What it was.** `SqlDriver.connect()` refuses a declared SQLite file its first statement cannot use (`SqliteDatabaseFileError`: `SQLITE_CANTOPEN`, `SQLITE_NOTADB`, `SQLITE_CORRUPT`). The file was opened to give that answer, and the pool kept the better-sqlite3 connection after the rejection. For a damaged file whose header says WAL, SQLite kept its `-shm` and an empty `-wal` beside the file until something called `disconnect()`. A datasource left unconnected on the degraded path is never disconnected, so it kept them for the life of the process. +- **What changed.** When a SQLite `connect()` rejects, it destroys its pool and puts a fresh, empty one in its place, then rethrows the same error: same class, code, message and `cause`. Nothing is left open, and the `-shm` / `-wal` siblings are gone when the rejection arrives. A later `connect()` on the same instance opens the file again: it gives the same refusal while the file is bad, and connects once the file is repaired. A release that itself fails is logged at `warn`, and the original refusal is still the error thrown. +- **Unchanged.** Which files are refused, the codes and the messages. Server dialects (`pg`, `mysql`) are untouched: their `connect()` rejects only when no connection could be made, so there is nothing to release. `SqliteWasmDriver` and the local mode of `TursoDriver` inherit the release through `super.connect()`. +- **What to do.** Nothing. diff --git a/packages/drivers/driver-sql/src/sql-driver-25019-sqlite-damaged-file-and-dangling-link.test.ts b/packages/drivers/driver-sql/src/sql-driver-25019-sqlite-damaged-file-and-dangling-link.test.ts index b08fcd4ad6f..c0f5c681ab6 100644 --- a/packages/drivers/driver-sql/src/sql-driver-25019-sqlite-damaged-file-and-dangling-link.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-25019-sqlite-damaged-file-and-dangling-link.test.ts @@ -89,9 +89,11 @@ async function seedDatabase(filename: string): Promise { /** * The refused file holds the bytes it was found with, and nothing is left - * beside it. The header still says WAL (this driver's journal mode), so SQLite - * keeps its `-shm` and an empty `-wal` while the refused connection is open: - * no `.corrupt-*` sibling then, and nothing at all once it is closed. + * beside it: no `.corrupt-*` sibling, and nothing at all after `disconnect()`. + * The header still says WAL (this driver's journal mode); the `-shm` and empty + * `-wal` SQLite keeps while a connection is open are gone already when + * `connect()` rejects, because the refusal closes its connection (#25084, + * pinned in `sql-driver-25084-sqlite-refused-connect-release.test.ts`). */ async function expectLeftAsItWas(driver: SqlDriver, filename: string, before: Buffer): Promise { expect(readdirSync(dir).filter((name) => name.includes('.corrupt-'))).toEqual([]);