From 194b636f8f1ff7d6b709b5b9010d1d14b3919e66 Mon Sep 17 00:00:00 2001 From: Ivan Trubach Date: Thu, 25 Feb 2021 19:15:04 +0000 Subject: [PATCH] database/sql: close driver.Connector if it implements io.Closer This change allows driver implementations to manage resources in driver.Connector, e.g. to share the same underlying database handle between multiple connections. That is, it allows embedded databases with in-memory backends like SQLite and Genji to safely release the resources once the sql.DB is closed. This makes it possible to address oddities with in-memory stores in SQLite and Genji drivers without introducing too much complexity in the driver implementations. See also: - https://github.com/mattn/go-sqlite3/issues/204 - https://github.com/mattn/go-sqlite3/issues/511 - https://github.com/genjidb/genji/issues/210 Fixes #41790 Change-Id: Idbd19763134438ed38288b9d44f16608e4e97fd7 GitHub-Last-Rev: 962c785dfb3bb6ad98b2216bcedd84ba383fe872 GitHub-Pull-Request: golang/go#41710 Reviewed-on: https://go-review.googlesource.com/c/go/+/258360 Reviewed-by: Emmanuel Odeke Reviewed-by: Daniel Theophanes Trust: Emmanuel Odeke Run-TryBot: Emmanuel Odeke TryBot-Result: Go Bot --- src/database/sql/driver/driver.go | 3 +++ src/database/sql/fakedb_test.go | 9 +++++++++ src/database/sql/sql.go | 6 ++++++ src/database/sql/sql_test.go | 11 ++++++++++- 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/database/sql/driver/driver.go b/src/database/sql/driver/driver.go index 5bbcf20db2f07d..f09396175acf86 100644 --- a/src/database/sql/driver/driver.go +++ b/src/database/sql/driver/driver.go @@ -115,6 +115,9 @@ type DriverContext interface { // DriverContext's OpenConnector method, to allow drivers // access to context and to avoid repeated parsing of driver // configuration. +// +// If a Connector implements io.Closer, the sql package's DB.Close +// method will call Close and return error (if any). type Connector interface { // Connect returns a connection to the database. // Connect may return a cached connection (one previously diff --git a/src/database/sql/fakedb_test.go b/src/database/sql/fakedb_test.go index 7605a2a6d23e03..1bfd1118aad68b 100644 --- a/src/database/sql/fakedb_test.go +++ b/src/database/sql/fakedb_test.go @@ -56,6 +56,7 @@ type fakeConnector struct { name string waiter func(context.Context) + closed bool } func (c *fakeConnector) Connect(context.Context) (driver.Conn, error) { @@ -68,6 +69,14 @@ func (c *fakeConnector) Driver() driver.Driver { return fdriver } +func (c *fakeConnector) Close() error { + if c.closed { + return errors.New("fakedb: connector is closed") + } + c.closed = true + return nil +} + type fakeDriverCtx struct { fakeDriver } diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go index 726aadb8990e68..37bcb0d0918645 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -850,6 +850,12 @@ func (db *DB) Close() error { } } db.stop() + if c, ok := db.connector.(io.Closer); ok { + err1 := c.Close() + if err1 != nil { + err = err1 + } + } return err } diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go index 99bfd62491fa3b..c06e565ea9c7a5 100644 --- a/src/database/sql/sql_test.go +++ b/src/database/sql/sql_test.go @@ -4059,9 +4059,18 @@ func TestOpenConnector(t *testing.T) { } defer db.Close() - if _, is := db.connector.(*fakeConnector); !is { + c, ok := db.connector.(*fakeConnector) + if !ok { t.Fatal("not using *fakeConnector") } + + if err := db.Close(); err != nil { + t.Fatal(err) + } + + if !c.closed { + t.Fatal("connector is not closed") + } } type ctxOnlyDriver struct {