[BUG] leaking unfinalized statements
as first noticed in GoToSocial here: https://github.com/superseriousbusiness/gotosocial/issues/2728
this specifically was noticed upgrading from v1.28.0 -> v1.29.x with the (correctly) updated context handling behaviour more reliably sending an interrupt signal in cases of cancelled context. though as some statements were not being finalized it was possible to leave an SQLite connection in a broken state always returning "interrupted" as the running statement count could never reach zero.
i spent quite some time trying to create a test case that could reliably repeat this behaviour and failed. running the GoToSocial testrig and spamming refresh on a database-accessing endpoint seems to be the most reliable way to trigger it as far as i have found.
i haven't opened an MR yet as i wasn't sure if this would be accepted without a reliable test case (maybe someone more familiar with the sqlite codebase has a better idea), but the following diff does fix the issue for gotosocial:
diff --git a/sqlite.go b/sqlite.go
index 86f5edd..5ead819 100644
--- a/sqlite.go
+++ b/sqlite.go
@@ -504,6 +504,17 @@ func (s *stmt) exec(ctx context.Context, args []driver.NamedValue) (r driver.Res
}
defer func() {
+ if pstmt != 0 {
+ // ensure stmt finalized.
+ e := s.c.finalize(pstmt)
+
+ if err == nil && e != nil {
+ // prioritize original
+ // returned error.
+ err = e
+ }
+ }
+
if ctx != nil && atomic.LoadInt32(&done) != 0 {
r, err = nil, ctx.Err()
}
@@ -553,7 +564,12 @@ func (s *stmt) exec(ctx context.Context, args []driver.NamedValue) (r driver.Res
return nil
}()
- if e := s.c.finalize(pstmt); e != nil && err == nil {
+ e := s.c.finalize(pstmt)
+ pstmt = 0 // done with
+
+ if err == nil && e != nil {
+ // prioritize original
+ // returned error.
err = e
}
@@ -603,6 +619,17 @@ func (s *stmt) query(ctx context.Context, args []driver.NamedValue) (r driver.Ro
var allocs []uintptr
defer func() {
+ if pstmt != 0 {
+ // ensure stmt finalized.
+ e := s.c.finalize(pstmt)
+
+ if err == nil && e != nil {
+ // prioritize original
+ // returned error.
+ err = e
+ }
+ }
+
if ctx != nil && atomic.LoadInt32(&done) != 0 {
r, err = nil, ctx.Err()
} else if r == nil && err == nil {
@@ -673,7 +700,13 @@ func (s *stmt) query(ctx context.Context, args []driver.NamedValue) (r driver.Ro
}
return nil
}()
- if e := s.c.finalize(pstmt); e != nil && err == nil {
+
+ e := s.c.finalize(pstmt)
+ pstmt = 0 // done with
+
+ if err == nil && e != nil {
+ // prioritize original
+ // returned error.
err = e
}