Skip to content

Commit

Permalink
make sure Close always removes the runtime finalizer
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
charlievieth committed Dec 11, 2024
1 parent ab13d63 commit 69c42ee
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 22 deletions.
37 changes: 15 additions & 22 deletions sqlite3.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}

Expand Down
8 changes: 8 additions & 0 deletions sqlite3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 69c42ee

Please sign in to comment.