diff --git a/src/sqlite.ts b/src/sqlite.ts index 9576bc78..42b5f4a1 100644 --- a/src/sqlite.ts +++ b/src/sqlite.ts @@ -142,6 +142,16 @@ export function isSqliteReadonlyError(err: unknown): boolean { ) } +/// A read-only parent reports SQLITE_READONLY_DIRECTORY when it must create the +/// sidecars from scratch, but SQLITE_CANTOPEN when a `-wal` is present and the +/// `-shm` it needs to index it is not. openReadonlyCache re-throws the original +/// error when the database itself is missing, which is the other CANTOPEN. +function isSqliteSidecarError(err: unknown): boolean { + if (isSqliteReadonlyError(err)) return true + const errcode = (err as { errcode?: unknown } | null)?.errcode + return typeof errcode === 'number' && (errcode & 0xff) === 14 +} + type DatabaseFingerprint = { dev: number ino: number @@ -268,13 +278,11 @@ function readOnlyCachePath(sourcePath: string, fingerprint: DatabaseFingerprint) const tempBase = `${cachePath}.tmp-${process.pid}-${randomBytes(8).toString('hex')}` const tempWal = tempBase + '-wal' - const tempShm = tempBase + '-shm' const tempMetadata = `${metadataPath}.tmp-${process.pid}-${randomBytes(8).toString('hex')}` try { copyFileSync(sourcePath, tempBase) const copiedWal = copyOptionalFile(sourcePath + '-wal', tempWal) - const copiedShm = copyOptionalFile(sourcePath + '-shm', tempShm) // Do not publish a cache made from a moving database. A live WAL writer will // normally make the direct open succeed once its sidecars exist; this check @@ -288,7 +296,6 @@ function readOnlyCachePath(sourcePath: string, fingerprint: DatabaseFingerprint) unlinkIfPresent(cachePath + '-shm') renameSync(tempBase, cachePath) if (copiedWal) renameSync(tempWal, cachePath + '-wal') - if (copiedShm) renameSync(tempShm, cachePath + '-shm') const metadata: { version: number; sourcePath: string; fingerprint: DatabaseFingerprint } = { version: SQLITE_CACHE_VERSION, @@ -301,7 +308,6 @@ function readOnlyCachePath(sourcePath: string, fingerprint: DatabaseFingerprint) } finally { unlinkIfPresent(tempBase) unlinkIfPresent(tempWal) - unlinkIfPresent(tempShm) unlinkIfPresent(tempMetadata) } } @@ -331,10 +337,10 @@ export function openDatabase(path: string): SqliteDatabase { try { db = new DatabaseSync(path, { readOnly: true }) } catch (err) { - if (!isSqliteReadonlyError(err)) throw err + if (!isSqliteSidecarError(err)) throw err fallbackUsed = true - warnSqliteReadonlyOnce(path) db = openReadonlyCache(path, err) + warnSqliteReadonlyOnce(path) } try { db.exec?.('PRAGMA busy_timeout = 1000') @@ -347,16 +353,16 @@ export function openDatabase(path: string): SqliteDatabase { try { return db.prepare(sql).all(...params) as T[] } catch (err) { - if (!isSqliteReadonlyError(err)) throw err + if (!isSqliteSidecarError(err)) throw err if (fallbackUsed) throw err fallbackUsed = true - warnSqliteReadonlyOnce(path) try { db.close() } catch { // The failed connection may already have been closed by node:sqlite. } db = openReadonlyCache(path, err) + warnSqliteReadonlyOnce(path) try { db.exec?.('PRAGMA busy_timeout = 1000') } catch { diff --git a/tests/sqlite-readonly-parent.test.ts b/tests/sqlite-readonly-parent.test.ts index 13a71af3..fc1882d5 100644 --- a/tests/sqlite-readonly-parent.test.ts +++ b/tests/sqlite-readonly-parent.test.ts @@ -1,4 +1,4 @@ -import { chmodSync, existsSync, readdirSync, statSync } from 'node:fs' +import { chmodSync, copyFileSync, existsSync, mkdirSync, readdirSync, statSync } from 'node:fs' import { mkdtemp, rm } from 'node:fs/promises' import { createRequire } from 'node:module' import { join } from 'node:path' @@ -155,6 +155,38 @@ describe('SQLite read-only parent fallback', () => { expect(cachedDatabaseFiles()).toEqual([]) }) + it('reads un-checkpointed WAL rows when the parent is read-only and the -shm is absent', ({ skip }) => { + const originDir = join(sourceRoot, 'origin') + mkdirSync(originDir) + const originPath = join(originDir, 'state.vscdb') + const writer = new NativeDatabase(originPath) + writer.exec('PRAGMA journal_mode=WAL') + writer.exec('CREATE TABLE values_table (c INTEGER)') + writer.prepare('INSERT INTO values_table (c) VALUES (?)').run(1) + writer.exec('PRAGMA wal_checkpoint(TRUNCATE)') + writer.exec('PRAGMA wal_autocheckpoint=0') + writer.prepare('INSERT INTO values_table (c) VALUES (?)').run(2) + + // A database copied off a live source (snapshot, rsync, unclean unmount) keeps + // its -wal but not its -shm. SQLite reports that as SQLITE_CANTOPEN, not + // SQLITE_READONLY, and the un-checkpointed row lives only in the -wal. + const dbPath = join(sourceRoot, 'state.vscdb') + copyFileSync(originPath, dbPath) + copyFileSync(originPath + '-wal', dbPath + '-wal') + writer.close() + expect(existsSync(dbPath + '-shm')).toBe(false) + if (!makeSourceParentReadOnly(skip)) return + + const db = openDatabase(dbPath) + try { + expect(db.query<{ c: number }>('SELECT c FROM values_table ORDER BY c')).toEqual([{ c: 1 }, { c: 2 }]) + } finally { + db.close() + } + expect(existsSync(dbPath + '-shm')).toBe(false) + expect(cachedDatabaseFiles()).toHaveLength(1) + }) + it('reuses an unchanged fallback copy instead of copying the database again', ({ skip }) => { const dbPath = join(sourceRoot, 'state.vscdb') createClosedWalDatabase(dbPath)