From the issue, which describes it as well as I could:
database/sql assumes that driver.Stmt.Close does not need the
connection.
see database/sql/sql.go:1308:
This puts the Rows' connection back into the idle pool, and
then calls the driver.Stmt.Close method of the Stmt it belongs
to. In the postgresql driver implementation
(https://github.com/lib/pq), Stmt.Close communicates with the
server (on the connection that was just put back into the idle
pool). Most of the time, this causes no problems, but if
another goroutine makes a query at the right (wrong?) time,
chaos results.
In any case, traffic is being sent on "free" connections
shortly after they are freed, leading to race conditions that
kill the driver code.
Fixes#5283
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/8633044
Now that revision 0c029965805f is in, it's easy
to guarantee that we never access a driver.Conn
concurrently, per the database/sql/driver contract,
so we can remove this overlarge mutex.
Fixes#3857
R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/7707047
The database/sql/driver docs make this promise:
"Conn is a connection to a database. It is not used
concurrently by multiple goroutines."
That promises exists as part of database/sql's overall
goal of making drivers relatively easy to write.
So far this promise has been kept without the use of locks by
being careful in the database/sql package, but sometimes too
careful. (cf. golang.org/issue/3857)
The CL associates a Mutex with each driver.Conn, and with the
interface value progeny thereof. (e.g. each driver.Tx,
driver.Stmt, driver.Rows, driver.Result, etc) Then whenever
those interface values are used, the Locker is locked.
This CL should be a no-op (aside from some new Lock/Unlock
pairs) and doesn't attempt to fix Issue 3857 or Issue 4459,
but should make it much easier in a subsequent CL.
Update #3857
R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/7803043
Simplifies the contract for Driver.Stmt.Close in
the process of fixing issue 3865.
Fixes#3865
Update #4459 (maybe fixes it; uninvestigated)
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/7363043
Tests that here should be automatic retries if a database
driver's connection returns ErrBadConn on Begin. See
"TestTxErrBadConn" in sql_test.go
R=golang-dev
CC=golang-dev
https://golang.org/cl/6942050
It was only being used for (*Stmt).Exec, not Query, and not for
the same two methods on *DB.
This unifies (*Stmt).Exec's old inline code into the old
subsetArgs function, renaming it in the process (changing the
old word "subset" to "driver", mostly converted earlier)
Fixes#3640
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/6258045
To make sure that there is no resource leak,
I suggest to fix the 'fakedb' driver such as it fails when any
Stmt is not closed.
First, add a check in fakeConn.Close().
Then, fix all missing Stmt.Close()/Rows.Close().
I am not sure that the strategy choose in fakeConn.Prepare/prepare* is ok.
The weak point in this patch is the change in Tx.Query:
- Tests pass without this change,
- I found it by manually analyzing the code,
- I just try to make Tx.Query look like DB.Query.
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/5759050
In a transaction, on a Stmt.Query error, it was possible for a
connection to be added to a db's freelist twice. Should use
the local releaseConn function instead.
Thanks to Gwenael Treguier for the failing test.
Also in this CL: propagate driver errors through releaseConn
into *DB.putConn, which conditionally ignores the freelist
addition if the driver signaled ErrBadConn, introduced in a
previous CL.
R=golang-dev, gary.burd
CC=golang-dev
https://golang.org/cl/5798049
Higher level tests for the pointer parameters
and scanning, complementing the existing ones
included in the previous CL.
R=golang-dev, dsymonds
CC=golang-dev
https://golang.org/cl/5646050
