database/sql: allow simultaneous queries, etc in a Tx
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
diff --git a/src/pkg/database/sql/sql.go b/src/pkg/database/sql/sql.go
index a4c4102..556580e 100644
--- a/src/pkg/database/sql/sql.go
+++ b/src/pkg/database/sql/sql.go
@@ -590,7 +590,7 @@
releaseConn(err)
return nil, err
}
- // Note: ownership of ci passes to the *Rows, to be freed
+ // Note: ownership of dc passes to the *Rows, to be freed
// with releaseConn.
rows := &Rows{
db: db,
@@ -689,15 +689,9 @@
// dc is owned exclusively until Commit or Rollback, at which point
// it's returned with putConn.
- // TODO(bradfitz): golang.org/issue/3857
dc *driverConn
txi driver.Tx
- // cimu is held while somebody is using ci (between grabConn
- // and releaseConn)
- // TODO(bradfitz): golang.org/issue/3857
- cimu sync.Mutex
-
// done transitions from false to true exactly once, on Commit
// or Rollback. once done, all operations fail with
// ErrTxDone.
@@ -720,14 +714,9 @@
if tx.done {
return nil, ErrTxDone
}
- tx.cimu.Lock()
return tx.dc, nil
}
-func (tx *Tx) releaseConn() {
- tx.cimu.Unlock()
-}
-
// Commit commits the transaction.
func (tx *Tx) Commit() error {
if tx.done {
@@ -774,7 +763,6 @@
if err != nil {
return nil, err
}
- defer tx.releaseConn()
dc.Lock()
si, err := dc.ci.Prepare(query)
@@ -817,7 +805,6 @@
if err != nil {
return &Stmt{stickyErr: err}
}
- defer tx.releaseConn()
dc.Lock()
si, err := dc.ci.Prepare(stmt.query)
dc.Unlock()
@@ -840,7 +827,6 @@
if err != nil {
return nil, err
}
- defer tx.releaseConn()
if execer, ok := dc.ci.(driver.Execer); ok {
dargs, err := driverArgs(nil, args)
@@ -871,14 +857,12 @@
// Query executes a query that returns rows, typically a SELECT.
func (tx *Tx) Query(query string, args ...interface{}) (*Rows, error) {
- ci, err := tx.grabConn()
+ dc, err := tx.grabConn()
if err != nil {
return nil, err
}
-
- releaseConn := func(err error) { tx.releaseConn() }
-
- return tx.db.queryConn(ci, releaseConn, query, args)
+ releaseConn := func(error) {}
+ return tx.db.queryConn(dc, releaseConn, query, args)
}
// QueryRow executes a query that is expected to return at most one row.
@@ -980,7 +964,7 @@
if err != nil {
return
}
- releaseConn = func(error) { s.tx.releaseConn() }
+ releaseConn = func(error) {}
return ci, releaseConn, s.txsi.si, nil
}
diff --git a/src/pkg/database/sql/sql_test.go b/src/pkg/database/sql/sql_test.go
index 57300bc..5d3df72 100644
--- a/src/pkg/database/sql/sql_test.go
+++ b/src/pkg/database/sql/sql_test.go
@@ -736,3 +736,28 @@
t.Logf("stmt = %#v", stmt)
}
}
+
+// Issue 3857
+// This used to deadlock.
+func TestSimultaneousQueries(t *testing.T) {
+ db := newTestDB(t, "people")
+ defer closeDB(t, db)
+
+ tx, err := db.Begin()
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer tx.Rollback()
+
+ r1, err := tx.Query("SELECT|people|name|")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer r1.Close()
+
+ r2, err := tx.Query("SELECT|people|name|")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer r2.Close()
+}
