From 2f31b6811bb8c850a9b39ef3de4788058346109f Mon Sep 17 00:00:00 2001 From: Gustavo Bertoi Date: Mon, 29 Jun 2026 16:05:28 -0300 Subject: [PATCH] fix(state): serialize the WAL-mode switch under the flock (fixes flaky open) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `TestConcurrentOpenSerializes` intermittently failed under `-race` with `ping state.db: database is locked (5) SQLITE_BUSY`. Root cause: `Open` called `sqlDB.Ping()` BEFORE taking the flock. Ping forces the first real connection, which applies the DSN `_pragma`s — including `journal_mode=WAL`. The WAL switch writes the DB header under a write lock, and that journal-mode change is NOT covered by `busy_timeout`, so concurrent first-opens racing the conversion get an immediate SQLITE_BUSY. Fix: move `Ping` inside the existing `lock.WithLock` block, alongside `migrate` + `ensureContext`, so the WAL conversion (a genuine mutation) is serialized across processes by the machine-global advisory lock — exactly the contract spec 08 already states for the mutating section. No behavior change for the single-open path. Verified with `-race -count=12`. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/state/state.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/state/state.go b/internal/state/state.go index 3d21314..5ee88e8 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -66,14 +66,20 @@ func Open(ctx context.Context, dir, dockerContext string) (*DB, error) { // A single open connection keeps pragma state consistent and means the // backup checkpoint cannot race a second writer. sqlDB.SetMaxOpenConns(1) - if err := sqlDB.Ping(); err != nil { - _ = sqlDB.Close() - return nil, fmt.Errorf("ping state.db: %w", err) - } db := &DB{DB: sqlDB, Ctx: dockerContext, path: path} lockPath := filepath.Join(xdg.RuntimeDir(), "devstack.lock") if err := lock.WithLock(ctx, lockPath, func() error { + // Ping forces the first real connection, which applies the DSN pragmas — + // including the journal_mode=WAL switch that writes the DB header. That + // conversion takes a write lock, so it MUST run inside the flock: concurrent + // first-opens racing the WAL switch otherwise hit SQLITE_BUSY immediately + // (busy_timeout doesn't cover the journal-mode change). Keeping Ping + + // migrate + ensureContext together under the lock serializes the whole + // mutating section across processes (spec 08). + if err := sqlDB.Ping(); err != nil { + return fmt.Errorf("ping state.db: %w", err) + } if err := db.migrate(); err != nil { return err }