From a66fe30908ccded09ebfe1fdd8f7a34d83666911 Mon Sep 17 00:00:00 2001 From: Charlie Vieth Date: Sun, 8 Dec 2024 19:47:14 -0500 Subject: [PATCH] make sure Close always removes the runtime finalizer This commit fixes a bug in {SQLiteConn,SQLiteStmt}.Close that would lead to the runtime finalizer not being removed if there was an error closing the connection or statement. This commit also makes it safe to call SQLiteConn.Close multiple times. --- sqlite3.go | 37 +++++++++++++++---------------------- sqlite3_test.go | 8 ++++++++ 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/sqlite3.go b/sqlite3.go index 3025a500..c02af5ab 100644 --- a/sqlite3.go +++ b/sqlite3.go @@ -1782,26 +1782,20 @@ func (d *SQLiteDriver) Open(dsn string) (driver.Conn, error) { } // Close the connection. -func (c *SQLiteConn) Close() error { +func (c *SQLiteConn) Close() (err error) { + c.mu.Lock() + defer c.mu.Unlock() + if c.db == nil { + return nil // Already closed + } + runtime.SetFinalizer(c, nil) rv := C.sqlite3_close_v2(c.db) if rv != C.SQLITE_OK { - return c.lastError() + err = c.lastError() } deleteHandles(c) - c.mu.Lock() c.db = nil - c.mu.Unlock() - runtime.SetFinalizer(c, nil) - return nil -} - -func (c *SQLiteConn) dbConnOpen() bool { - if c == nil { - return false - } - c.mu.Lock() - defer c.mu.Unlock() - return c.db != nil + return err } // Prepare the query string. Return a new statement. @@ -1901,16 +1895,15 @@ func (s *SQLiteStmt) Close() error { return nil } s.closed = true - if !s.c.dbConnOpen() { - return errors.New("sqlite statement with already closed database connection") - } - rv := C.sqlite3_finalize(s.s) + conn := s.c + stmt := s.s + s.c = nil s.s = nil + runtime.SetFinalizer(s, nil) + rv := C.sqlite3_finalize(stmt) if rv != C.SQLITE_OK { - return s.c.lastError() + return conn.lastError() } - s.c = nil - runtime.SetFinalizer(s, nil) return nil } diff --git a/sqlite3_test.go b/sqlite3_test.go index 94de7386..7f66df5e 100644 --- a/sqlite3_test.go +++ b/sqlite3_test.go @@ -338,6 +338,14 @@ func TestClose(t *testing.T) { if err == nil { t.Fatal("Failed to operate closed statement") } + // Closing a statement should not error even if the db is closed. + if err := stmt.Close(); err != nil { + t.Fatal(err) + } + // Second close should be a no-op + if err := stmt.Close(); err != nil { + t.Fatal(err) + } } func TestInsert(t *testing.T) {