oficina / home Public

Evict diagnostic connections after uncertain BEGIN failure

Paul Campbell pushed by paul@keymail.dev 549a277773e03c42166c3ebde84589b4a2156359 parent 52aa7c7
2 files changed, +44 −1
  • internal/home/legacy_schema_diagnostic.go +10 −1
  • internal/home/legacy_schema_diagnostic_test.go +34 −0

CI passed — run details

diff --git a/internal/home/legacy_schema_diagnostic.go b/internal/home/legacy_schema_diagnostic.go
index 848f47f..31ebc49 100644
--- a/internal/home/legacy_schema_diagnostic.go
+++ b/internal/home/legacy_schema_diagnostic.go
@@ -82,7 +82,7 @@ func inspectLegacySchema(ctx context.Context, d *sql.DB) (result legacyDiagnosti
return
}
defer conn.Close()
- if _, err = conn.ExecContext(ctx, "BEGIN"); err != nil {
+ if !beginLegacyDiagnostic(ctx, conn) {
return
}
defer func() {
@@ -192,3 +192,12 @@ func rollbackLegacyDiagnostic(ctx context.Context, conn legacyDiagnosticConnecti
}
return true
}
+
+func beginLegacyDiagnostic(ctx context.Context, conn legacyDiagnosticConnection) bool {
+ if _, err := conn.ExecContext(ctx, "BEGIN"); err != nil {
+ // The driver can report cancellation after SQLite has opened the transaction.
+ conn.Raw(func(any) error { return driver.ErrBadConn })
+ return false
+ }
+ return true
+}
diff --git a/internal/home/legacy_schema_diagnostic_test.go b/internal/home/legacy_schema_diagnostic_test.go
index ef19708..f0e27a7 100644
--- a/internal/home/legacy_schema_diagnostic_test.go
+++ b/internal/home/legacy_schema_diagnostic_test.go
@@ -366,3 +366,37 @@ func TestLegacyDiagnosticRollbackAndEviction(t *testing.T) {
t.Fatal("failed diagnostic stranded writer", err)
}
}
+
+type legacyBeginProbe struct {
+ t *testing.T
+ err error
+ opened, evicted bool
+}
+
+func (p *legacyBeginProbe) ExecContext(ctx context.Context, query string, args ...any) (sql.Result, error) {
+ if query != "BEGIN" || len(args) != 0 {
+ p.t.Fatal("unexpected begin statement")
+ }
+ p.opened = true
+ return nil, p.err
+}
+func (p *legacyBeginProbe) Raw(f func(any) error) error {
+ if f(nil) != driver.ErrBadConn {
+ p.t.Fatal("uncertain BEGIN did not discard connection")
+ }
+ p.evicted = true
+ p.opened = false
+ return nil
+}
+func TestLegacyDiagnosticBeginFailureEvictsOpenTransaction(t *testing.T) {
+ for _, err := range []error{nil, context.DeadlineExceeded, errors.New("driver error after begin")} {
+ p := &legacyBeginProbe{t: t, err: err}
+ ok := beginLegacyDiagnostic(t.Context(), p)
+ if err != nil && (ok || !p.evicted || p.opened) {
+ t.Fatal("failed BEGIN left an open transaction in the pool")
+ }
+ if err == nil && (!ok || p.evicted || !p.opened) {
+ t.Fatal("successful BEGIN discarded its connection")
+ }
+ }
+}