From 1b86c0b81ccd9d853a0faff73a5b16a1f6107c34 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sat, 31 Oct 2026 13:30:58 +0100 Subject: [PATCH 1/2] fix(dbkit/sqlite): end commit and row lock waits with their context --- dbkit/sqlite/CHANGELOG.md | 8 ++ dbkit/sqlite/busy_test.go | 169 ++++++++++++++++++++++++++++++-------- dbkit/sqlite/go.mod | 2 +- dbkit/sqlite/go.sum | 4 +- 4 files changed, 147 insertions(+), 36 deletions(-) diff --git a/dbkit/sqlite/CHANGELOG.md b/dbkit/sqlite/CHANGELOG.md index 328c586..0378c70 100644 --- a/dbkit/sqlite/CHANGELOG.md +++ b/dbkit/sqlite/CHANGELOG.md @@ -8,6 +8,14 @@ v0.x, minor releases may contain breaking changes. Releases of this module are tagged `dbkit/sqlite/vX.Y.Z`. +## [Unreleased] + +### Changed + +- **Breaking.** A lock wait while reading a later row of a query, closing its rows or committing a transaction now ends when the context of the query or of `BeginTx` ends. It then answers that context's error alone, not `dbkit.ErrBusy`, and so does an `InTx` error from its commit. +- **Breaking.** A share's `Tx.Commit` that waits for a lock now ends with the context of `Begin` or at the transaction timeout. Its error matches that context error, not `dbkit.ErrBusy`. +- Requires `modernc.org/sqlite` v1.61.1. + ## [0.3.0] - 2026-10-22 ### Changed diff --git a/dbkit/sqlite/busy_test.go b/dbkit/sqlite/busy_test.go index a7eff0f..3e34de5 100644 --- a/dbkit/sqlite/busy_test.go +++ b/dbkit/sqlite/busy_test.go @@ -19,10 +19,12 @@ import ( ) const ( - // busyDeadline is the deadline, and the cancel, of a call behind a held writer. + // busyDeadline is the deadline, and the cancel, of a call behind a held lock. busyDeadline = 100 * time.Millisecond - // busyBound is the longest a call behind a held writer may run, a tenth of the busy timeout. + // busyBound is the longest a call behind a held lock may run, a tenth of the busy timeout. busyBound = optionsBusyTimeout / 10 + // busyShareTimeout is the transaction timeout of a share whose commit waits for a lock. + busyShareTimeout = 300 * time.Millisecond // busyLateHold is how long a holder keeps the write lock after the deadline of the write it blocks. busyLateHold = 400 * time.Millisecond // busyHold is how long a holder keeps the write lock while a write with no deadline waits for it. @@ -47,6 +49,8 @@ const ( busyAuxRead = "SELECT count(*) FROM aux_rows" // busyAuxReturning writes one row to aux on its first step and commits it at the end of its rows. busyAuxReturning = "INSERT INTO aux.aux_rows VALUES (1) RETURNING a" + // busyAuxInsert writes one row to aux. + busyAuxInsert = "INSERT INTO aux.aux_rows VALUES (1)" // busyTypedSchema creates a table whose columns declare their types. busyTypedSchema = "CREATE TABLE typed_rows (amount INTEGER NOT NULL, label VARCHAR(20) NOT NULL)" // busyTypedInsert writes one row of synthetic values to the typed table. @@ -57,8 +61,6 @@ const ( busyNoTable = "VALUES (1), (2)" // busySetTimeout sets a connection's own busy timeout to its one argument in milliseconds. busySetTimeout = "PRAGMA busy_timeout=%d" - // busyShortTimeout is the busy timeout a later step of a query or the close of its rows waits out on purpose. - busyShortTimeout = 300 * time.Millisecond ) // busyShape is one call that waits for the write lock. @@ -122,7 +124,7 @@ func busyWarmUp(t *testing.T, db *sql.DB) { } } -// busyEnd is how a call behind a held writer ended. +// busyEnd is how a call behind a held lock ended. type busyEnd struct { // err is the error of the call. err error @@ -137,17 +139,17 @@ func busyRun(ctx context.Context, shape busyShape, db *sql.DB, prepared *sql.Stm return busyEnd{err: err, waited: time.Since(began)} } -// busyMustEnd fails the test unless the call of shape ended with want, matching no dbkit.ErrBusy, within busyBound. -func busyMustEnd(t *testing.T, shape busyShape, end busyEnd, want error) { +// busyMustEnd fails the test unless what ended with want, matching no dbkit.ErrBusy, within busyBound. +func busyMustEnd(t *testing.T, what string, end busyEnd, want error) { t.Helper() if !errors.Is(end.err, want) { - t.Errorf("%s error = %v, want one matching %v", shape.name, end.err, want) + t.Errorf("%s error = %v, want one matching %v", what, end.err, want) } if errors.Is(sqlite.Classify(end.err), dbkit.ErrBusy) { - t.Errorf("%s error %v matches dbkit.ErrBusy, want no busy match", shape.name, end.err) + t.Errorf("%s error %v matches dbkit.ErrBusy, want no busy match", what, end.err) } if end.waited > busyBound { - t.Errorf("%s answered after %v, want within a tenth of the busy timeout %v", shape.name, end.waited, + t.Errorf("%s answered after %v, want within a tenth of the busy timeout %v", what, end.waited, optionsBusyTimeout) } } @@ -164,7 +166,7 @@ func busyMustEndEvery(t *testing.T, end func(context.Context) (context.Context, got := busyRun(ctx, shape, db, prepared) - busyMustEnd(t, shape, got, want) + busyMustEnd(t, shape.name, got, want) }) } } @@ -444,8 +446,8 @@ func TestEveryConnectionEndsItsWaitAtItsDeadline(t *testing.T) { } } -// busyAttachHeld returns a connection of db with aux, a rollback journal file a plain handle locks by running hold. -func busyAttachHeld(t *testing.T, db *sql.DB, hold ...string) *sql.Conn { +// busyAuxFile returns the path of a new rollback journal file that holds aux_rows, and a plain connection to it. +func busyAuxFile(t *testing.T) (string, *sql.Conn) { t.Helper() path := filepath.Join(t.TempDir(), "aux.db") plain, err := sql.Open("sqlite", path) @@ -453,39 +455,66 @@ func busyAttachHeld(t *testing.T, db *sql.DB, hold ...string) *sql.Conn { t.Fatalf("sql.Open() error = %v, want nil", err) } t.Cleanup(func() { _ = plain.Close() }) - holder, pinned := busyPin(t, plain), busyPin(t, db) + holder := busyPin(t, plain) busyPinExec(t, holder, busyAuxSchema) - if _, err := pinned.ExecContext(t.Context(), busyAttach, path); err != nil { + return path, holder +} + +// busyAttachTo attaches the file at path to conn as aux. +func busyAttachTo(t *testing.T, conn *sql.Conn, path string) { + t.Helper() + if _, err := conn.ExecContext(t.Context(), busyAttach, path); err != nil { t.Fatalf("ExecContext(%q) error = %v, want nil", busyAttach, err) } +} + +// busyAttachHeld returns a connection of db with aux, a rollback journal file a plain handle locks by running hold. +func busyAttachHeld(t *testing.T, db *sql.DB, hold ...string) *sql.Conn { + t.Helper() + path, holder := busyAuxFile(t) + pinned := busyPin(t, db) + busyAttachTo(t, pinned, path) for _, query := range hold { busyPinExec(t, holder, query) } return pinned } -// busyShortOptions returns testOptions with busyShortTimeout as the busy timeout. -func busyShortOptions() sqlite.Options { - opts := testOptions() - opts.BusyTimeout = busyShortTimeout - return opts +// busyAttachEvery attaches aux, a rollback journal file a plain handle locks by running hold, to each connection of db. +func busyAttachEvery(t *testing.T, db *sql.DB, hold ...string) { + t.Helper() + path, holder := busyAuxFile(t) + for _, conn := range rulesConns(t, db) { + busyAttachTo(t, conn, path) + if err := conn.Close(); err != nil { + t.Fatalf("Close() error = %v, want nil", err) + } + } + for _, query := range hold { + busyPinExec(t, holder, query) + } +} + +// busyRowReader is a handle or a connection that reads one row. +type busyRowReader interface { + // QueryRowContext runs query with args under ctx and returns its first row. + QueryRowContext(ctx context.Context, query string, args ...any) *sql.Row } -// busyMustWaitOut fails the test unless what answered an error that Classify marks busy after busyShortTimeout. -func busyMustWaitOut(t *testing.T, what string, err error, waited time.Duration) { +// busyAuxRows returns how many rows aux holds as reader sees them. +func busyAuxRows(t *testing.T, reader busyRowReader) int64 { t.Helper() - if !errors.Is(sqlite.Classify(err), dbkit.ErrBusy) { - t.Errorf("%s error = %v, want busy once the busy timeout passed", what, err) - } - if waited < busyShortTimeout { - t.Errorf("%s answered after %v, want at least the busy timeout %v", what, waited, busyShortTimeout) + var n int64 + if err := reader.QueryRowContext(t.Context(), busyAuxRead).Scan(&n); err != nil { + t.Fatalf("%s error = %v, want nil", busyAuxRead, err) } + return n } -func TestALaterStepOfAQueryWaitsOutTheBusyTimeout(t *testing.T) { +func TestALaterStepOfAQueryEndsWithItsContext(t *testing.T) { t.Parallel() - pinned := busyAttachHeld(t, rulesOpen(t, busyShortOptions()), "BEGIN EXCLUSIVE") + pinned := busyAttachHeld(t, rulesOpen(t, testOptions()), "BEGIN EXCLUSIVE") ctx, cancel := context.WithCancel(t.Context()) defer cancel() rows, err := pinned.QueryContext(ctx, busyLaterStep) @@ -503,13 +532,13 @@ func TestALaterStepOfAQueryWaitsOutTheBusyTimeout(t *testing.T) { if !first || second { t.Errorf("Next() = %v, then %v, want the first row and then no second while aux is held", first, second) } - busyMustWaitOut(t, "the second step", rows.Err(), waited) + busyMustEnd(t, "the second step", busyEnd{err: rows.Err(), waited: waited}, context.Canceled) } -func TestClosingTheRowsOfAWriteWaitsOutTheBusyTimeout(t *testing.T) { +func TestClosingTheRowsOfAWriteEndsWithTheQuerysContext(t *testing.T) { t.Parallel() - pinned := busyAttachHeld(t, rulesOpen(t, busyShortOptions()), "BEGIN", busyAuxRead) + pinned := busyAttachHeld(t, rulesOpen(t, testOptions()), "BEGIN", busyAuxRead) ctx, cancel := context.WithCancel(t.Context()) defer cancel() rows, err := pinned.QueryContext(ctx, busyAuxReturning) @@ -524,7 +553,81 @@ func TestClosingTheRowsOfAWriteWaitsOutTheBusyTimeout(t *testing.T) { err = rows.Close() - busyMustWaitOut(t, "the commit at Close", err, time.Since(began)) + busyMustEnd(t, "the commit at Close", busyEnd{err: err, waited: time.Since(began)}, context.Canceled) + if n := busyAuxRows(t, pinned); n != 0 { + t.Errorf("rows in aux = %d after the commit at Close ended, want 0", n) + } +} + +func TestACommitEndsWithTheContextOfItsTransaction(t *testing.T) { + t.Parallel() + + pinned := busyAttachHeld(t, rulesOpen(t, testOptions()), "BEGIN", busyAuxRead) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + tx, err := pinned.BeginTx(ctx, nil) + if err != nil { + t.Fatalf("BeginTx() error = %v, want nil", err) + } + defer func() { _ = tx.Rollback() }() + if _, err := tx.ExecContext(ctx, busyAuxInsert); err != nil { + t.Fatalf("ExecContext(%q) error = %v, want nil", busyAuxInsert, err) + } + began := time.Now() + defer time.AfterFunc(busyDeadline, cancel).Stop() + + err = tx.Commit() + + busyMustEnd(t, "the commit", busyEnd{err: err, waited: time.Since(began)}, context.Canceled) + if n := busyAuxRows(t, pinned); n != 0 { + t.Errorf("rows in aux = %d after the commit ended, want 0 with its transaction rolled back", n) + } +} + +func TestAnInTxCommitEndsWithItsContext(t *testing.T) { + t.Parallel() + + db := rulesOpen(t, testOptions()) + busyAttachEvery(t, db, "BEGIN", busyAuxRead) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + var began time.Time + + err := dbkit.InTx(ctx, db, func(tx *sql.Tx) error { + if _, err := tx.ExecContext(ctx, busyAuxInsert); err != nil { + return err + } + began = time.Now() + time.AfterFunc(busyDeadline, cancel) + return nil + }) + + busyMustEnd(t, "InTx", busyEnd{err: err, waited: time.Since(began)}, context.Canceled) + if n := busyAuxRows(t, db); n != 0 { + t.Errorf("rows in aux = %d after InTx ended, want 0", n) + } +} + +func TestAShareCommitEndsAtItsTransactionTimeout(t *testing.T) { + t.Parallel() + + db := rulesOpen(t, testOptions()) + busyAttachEvery(t, db, "BEGIN", busyAuxRead) + opts := shareOptions("busy-share") + opts.TransactionTimeout = busyShareTimeout + tx, err := shareOn(t, db, opts).Begin(t.Context()) + if err != nil { + t.Fatalf("Begin() error = %v, want nil", err) + } + shareMustExec(t, tx, busyAuxInsert) + began := time.Now() + + err = tx.Commit() + + busyMustEnd(t, "the share commit", busyEnd{err: err, waited: time.Since(began)}, context.DeadlineExceeded) + if n := busyAuxRows(t, db); n != 0 { + t.Errorf("rows in aux = %d after the share commit ended, want 0", n) + } } // busyColumn is what database/sql reports of one column. diff --git a/dbkit/sqlite/go.mod b/dbkit/sqlite/go.mod index eb481c9..9325f50 100644 --- a/dbkit/sqlite/go.mod +++ b/dbkit/sqlite/go.mod @@ -6,7 +6,7 @@ require ( github.com/gopherium/framework/dbkit v0.3.0 github.com/pressly/goose/v3 v3.27.2 golang.org/x/sys v0.49.0 - modernc.org/sqlite v1.61.0 + modernc.org/sqlite v1.61.1 ) require ( diff --git a/dbkit/sqlite/go.sum b/dbkit/sqlite/go.sum index 389cce3..1567e5b 100644 --- a/dbkit/sqlite/go.sum +++ b/dbkit/sqlite/go.sum @@ -60,8 +60,8 @@ modernc.org/opt v0.2.0 h1:tGyef5ApycA7FSEOMraay9SaTk5zmbx7Tu+cJs4QKZg= modernc.org/opt v0.2.0/go.mod h1:03fq9lsNfvkYSfxrfUhZCWPk1lm4cq4N+Bh//bEtgns= modernc.org/sortutil v1.2.1 h1:+xyoGf15mM3NMlPDnFqrteY07klSFxLElE2PVuWIJ7w= modernc.org/sortutil v1.2.1/go.mod h1:7ZI3a3REbai7gzCLcotuw9AC4VZVpYMjDzETGsSMqJE= -modernc.org/sqlite v1.61.0 h1:sQi63oLQ9uquAUA4CzOisH35Q3pEszkPBuHM0tFL8CY= -modernc.org/sqlite v1.61.0/go.mod h1:f4gr5tbpx2umklaT/QVtbi/50hNdM+4ng+2zMo8KXJ0= +modernc.org/sqlite v1.61.1 h1:TBEE49wz7VlxS+qXRXMQiPUZYOXHJ4ku83daYv1fe04= +modernc.org/sqlite v1.61.1/go.mod h1:f4gr5tbpx2umklaT/QVtbi/50hNdM+4ng+2zMo8KXJ0= modernc.org/strutil v1.2.1 h1:UneZBkQA+DX2Rp35KcM69cSsNES9ly8mQWD71HKlOA0= modernc.org/strutil v1.2.1/go.mod h1:EHkiggD70koQxjVdSBM3JKM7k6L0FbGE5eymy9i3B9A= modernc.org/token v1.1.0 h1:Xl7Ap9dKaEs5kLoOQeQmPWevfnk/DM5qcLcYlA8ys6Y= From 943916d76d42135bb06d22fe7fea91c6bd6946ba Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sat, 31 Oct 2026 14:13:16 +0100 Subject: [PATCH 2/2] test(dbkit/sqlite): cover a share commit that ends with its Begin context --- dbkit/sqlite/busy_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/dbkit/sqlite/busy_test.go b/dbkit/sqlite/busy_test.go index 3e34de5..3e376ee 100644 --- a/dbkit/sqlite/busy_test.go +++ b/dbkit/sqlite/busy_test.go @@ -630,6 +630,29 @@ func TestAShareCommitEndsAtItsTransactionTimeout(t *testing.T) { } } +func TestAShareCommitEndsWithItsBeginContext(t *testing.T) { + t.Parallel() + + db := rulesOpen(t, testOptions()) + busyAttachEvery(t, db, "BEGIN", busyAuxRead) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + tx, err := shareOn(t, db, shareOptions("busy-share")).Begin(ctx) + if err != nil { + t.Fatalf("Begin() error = %v, want nil", err) + } + shareMustExec(t, tx, busyAuxInsert) + began := time.Now() + defer time.AfterFunc(busyDeadline, cancel).Stop() + + err = tx.Commit() + + busyMustEnd(t, "the share commit", busyEnd{err: err, waited: time.Since(began)}, context.Canceled) + if n := busyAuxRows(t, db); n != 0 { + t.Errorf("rows in aux = %d after the share commit ended, want 0", n) + } +} + // busyColumn is what database/sql reports of one column. type busyColumn struct { // name is the column's name.