From c22b2c7eb55274a43616a62df7e66af35170b540 Mon Sep 17 00:00:00 2001 From: bfs2021 <101915083+bfs2021@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:20:52 +0800 Subject: [PATCH 1/2] fix(dbkit/sqlite): end a commit's lock wait with BeginTx's context database/sql's Tx.Commit takes no context, and BeginTx returned the driver's transaction unchanged, so a commit that waited for a lock ran with no span and waited up to BusyTimeout. A context that ended at 100ms answered 20s later, once the reader let go. Wrap the transaction so Commit runs under the context BeginTx was given. Rollback stays the driver's: database/sql rolls back on an ended context, and a rollback that stopped early would return a connection still inside a transaction to the pool. Fixes #104 --- dbkit/sqlite/busy.go | 24 +++++++++++++++++++++++- dbkit/sqlite/busy_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/dbkit/sqlite/busy.go b/dbkit/sqlite/busy.go index 47b53bc..2cc7ed0 100644 --- a/dbkit/sqlite/busy.go +++ b/dbkit/sqlite/busy.go @@ -169,7 +169,29 @@ func (c *busyConn) BeginTx(ctx context.Context, opts driver.TxOptions) (driver.T if err != nil { return nil, ended(ctx, err) } - return tx, nil + return &busyTx{Tx: tx, wait: c.wait, ctx: ctx}, nil +} + +// busyTx is a transaction whose commit waits under the context BeginTx was given. +// +// database/sql's Tx.Commit takes no context, so without this wrapper a commit +// that waits for a lock would run with no span and wait up to BusyTimeout. The +// rollback stays the driver's: database/sql rolls back on an ended context, and +// a rollback that stopped early would return a connection still inside a +// transaction to the pool. +type busyTx struct { + // Tx is the driver's transaction. + driver.Tx + // wait is the connection's wait. + wait *busyWait + // ctx is the context BeginTx was given. + ctx context.Context +} + +// Commit commits the transaction, and a wait for a lock ends with the transaction's context. +func (t *busyTx) Commit() error { + defer t.wait.during(t.ctx)() + return ended(t.ctx, t.Tx.Commit()) } // ExecContext runs a statement, and answers the error of ctx alone when it failed after ctx ended. diff --git a/dbkit/sqlite/busy_test.go b/dbkit/sqlite/busy_test.go index 71a91e2..0dc263d 100644 --- a/dbkit/sqlite/busy_test.go +++ b/dbkit/sqlite/busy_test.go @@ -565,6 +565,34 @@ func TestClosingTheRowsOfAWriteEndsItsWaitWithItsContext(t *testing.T) { } } +func TestACommitEndsItsWaitWithTheContextOfBeginTx(t *testing.T) { + t.Parallel() + + pinned := busyAttachHeld(t, rulesOpen(t, busyOptions(busyPoll)), "BEGIN", busyAuxRead) + ctx, cancel := context.WithTimeout(t.Context(), busyDeadline) + 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, "INSERT INTO aux.aux_rows VALUES (1)"); err != nil { + t.Fatalf("ExecContext() error = %v, want nil", err) + } + began := time.Now() + + err = tx.Commit() + + waited := time.Since(began) + if !errors.Is(err, context.DeadlineExceeded) { + t.Errorf("Commit() error = %v, want one matching context.DeadlineExceeded", err) + } + if waited > busyBound { + t.Errorf("Commit() answered after %v, want within a tenth of the busy timeout %v", waited, optionsBusyTimeout) + } +} + // busyColumn is what database/sql reports of one column. type busyColumn struct { // name is the column's name. From cecce1122800f17e1a5c1e25d3edd4a24006c48e Mon Sep 17 00:00:00 2001 From: bfs2021 <101915083+bfs2021@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:28:54 +0800 Subject: [PATCH 2/2] fix(dbkit/sqlite): keep a successful commit successful If the context ended after the driver's commit returned nil, ended() turned a committed transaction into a context error, so a caller could retry a write that already took effect. Return nil when the driver commit succeeded; only annotate a failed commit with the context error. --- dbkit/sqlite/busy.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dbkit/sqlite/busy.go b/dbkit/sqlite/busy.go index 2cc7ed0..03c8ba7 100644 --- a/dbkit/sqlite/busy.go +++ b/dbkit/sqlite/busy.go @@ -191,7 +191,13 @@ type busyTx struct { // Commit commits the transaction, and a wait for a lock ends with the transaction's context. func (t *busyTx) Commit() error { defer t.wait.during(t.ctx)() - return ended(t.ctx, t.Tx.Commit()) + err := t.Tx.Commit() + if err == nil { + // The commit took effect. A context that ended meanwhile must not turn + // it into an error the caller would retry. + return nil + } + return ended(t.ctx, err) } // ExecContext runs a statement, and answers the error of ctx alone when it failed after ctx ended.