Skip to content

[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
 		}