From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Mon, 18 Mar 2013 18:39:00 +0000 (-0700)
Subject: database/sql: allow simultaneous queries, etc in a Tx
X-Git-Tag: go1.1rc2~462
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=a7a803c7b7b5322c90d093cc603ebafd0c3c320a;p=gostls13.git

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 a4c410267c..556580eaec 100644
--- a/src/pkg/database/sql/sql.go
+++ b/src/pkg/database/sql/sql.go
@@ -590,7 +590,7 @@ func (db *DB) queryConn(dc *driverConn, releaseConn func(error), query string, a
 				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 @@ type Tx struct {
 
 	// 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 @@ func (tx *Tx) grabConn() (*driverConn, error) {
 	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 @@ func (tx *Tx) Prepare(query string) (*Stmt, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer tx.releaseConn()
 
 	dc.Lock()
 	si, err := dc.ci.Prepare(query)
@@ -817,7 +805,6 @@ func (tx *Tx) Stmt(stmt *Stmt) *Stmt {
 	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 @@ func (tx *Tx) Exec(query string, args ...interface{}) (Result, error) {
 	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 @@ func (tx *Tx) Exec(query string, args ...interface{}) (Result, error) {
 
 // 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 @@ func (s *Stmt) connStmt() (ci *driverConn, releaseConn func(error), si driver.St
 		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 57300bc4d9..5d3df721ed 100644
--- a/src/pkg/database/sql/sql_test.go
+++ b/src/pkg/database/sql/sql_test.go
@@ -736,3 +736,28 @@ func TestIssue4902(t *testing.T) {
 		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()
+}