From 0cf218e26ee152d10334074048e27a9f825dac97 Mon Sep 17 00:00:00 2001 From: Ivan Trubach Date: Wed, 30 Sep 2020 12:31:01 +0300 Subject: [PATCH 1/4] database/sql: close driver.Connector if it implements an 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 --- src/database/sql/driver/driver.go | 3 +++ src/database/sql/fakedb_test.go | 9 +++++++++ src/database/sql/sql.go | 3 +++ src/database/sql/sql_test.go | 11 ++++++++++- 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/database/sql/driver/driver.go b/src/database/sql/driver/driver.go index 5bbcf20db2f07..d1702a624e066 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. +// +// A Connector may optionally implement io.Closer interface +// to release the resources when sql.DB is closed. 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 7605a2a6d23e0..1bfd1118aad68 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 726aadb8990e6..a1286ca650508 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -850,6 +850,9 @@ func (db *DB) Close() error { } } db.stop() + if c, ok := db.connector.(io.Closer); ok { + err = c.Close() + } return err } diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go index 99bfd62491fa3..c06e565ea9c7a 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 { From dc119f7a4541a0e713719ecb9d4de66aaa67b487 Mon Sep 17 00:00:00 2001 From: Ivan Trubach Date: Thu, 1 Oct 2020 14:44:25 +0300 Subject: [PATCH 2/4] =?UTF-8?q?Don=E2=80=99t=20overwrite=20possibly=20non-?= =?UTF-8?q?nil=20error=20with=20nil?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/database/sql/sql.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go index a1286ca650508..37bcb0d091864 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -851,7 +851,10 @@ func (db *DB) Close() error { } db.stop() if c, ok := db.connector.(io.Closer); ok { - err = c.Close() + err1 := c.Close() + if err1 != nil { + err = err1 + } } return err } From 2ebcad4739735b9ae8a1e7abd61e5850aff9c53e Mon Sep 17 00:00:00 2001 From: Ivan Trubach Date: Thu, 1 Oct 2020 14:51:20 +0300 Subject: [PATCH 3/4] Make doc for driver.Connector reflect the implementation --- src/database/sql/driver/driver.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/database/sql/driver/driver.go b/src/database/sql/driver/driver.go index d1702a624e066..a32ae4343d4d8 100644 --- a/src/database/sql/driver/driver.go +++ b/src/database/sql/driver/driver.go @@ -116,8 +116,8 @@ type DriverContext interface { // access to context and to avoid repeated parsing of driver // configuration. // -// A Connector may optionally implement io.Closer interface -// to release the resources when sql.DB is closed. +// If a Connector implements io.Closer interface, the sql package's +// DB.Close will call the Close method and return the error (if any). type Connector interface { // Connect returns a connection to the database. // Connect may return a cached connection (one previously From 962c785dfb3bb6ad98b2216bcedd84ba383fe872 Mon Sep 17 00:00:00 2001 From: Ivan Trubach Date: Thu, 25 Feb 2021 19:42:57 +0300 Subject: [PATCH 4/4] Update doc for dirver.Connector that implements io.Closer --- src/database/sql/driver/driver.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/database/sql/driver/driver.go b/src/database/sql/driver/driver.go index a32ae4343d4d8..f09396175acf8 100644 --- a/src/database/sql/driver/driver.go +++ b/src/database/sql/driver/driver.go @@ -116,8 +116,8 @@ type DriverContext interface { // access to context and to avoid repeated parsing of driver // configuration. // -// If a Connector implements io.Closer interface, the sql package's -// DB.Close will call the Close method and return the error (if any). +// 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