database/sql: fix double connection free on Stmt.Query error

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

1 files changed, 43 insertions, 0 deletions

diff --git a/src/pkg/database/sql/sql_test.go b/src/pkg/database/sql/sql_test.go
index 02ab20cd7c..90a40efa28 100644
--- a/src/pkg/database/sql/sql_test.go
+++ b/src/pkg/database/sql/sql_test.go
@@ -5,13 +5,35 @@
 package sql
 
 import (
+	"database/sql/driver"
 	"fmt"
 	"reflect"
+	"runtime"
 	"strings"
 	"testing"
 	"time"
 )
 
+func init() {
+	type dbConn struct {
+		db *DB
+		c  driver.Conn
+	}
+	freedFrom := make(map[dbConn]string)
+	putConnHook = func(db *DB, c driver.Conn) {
+		for _, oc := range db.freeConn {
+			if oc == c {
+				// print before panic, as panic may get lost due to conflicting panic
+				// (all goroutines asleep) elsewhere, since we might not unlock
+				// the mutex in freeConn here.
+				println("double free of conn. conflicts are:\nA) " + freedFrom[dbConn{db, c}] + "\n\nand\nB) " + stack())
+				panic("double free of conn.")
+			}
+		}
+		freedFrom[dbConn{db, c}] = stack()
+	}
+}
+
 const fakeDBName = "foo"
 
 var chrisBirthday = time.Unix(123456789, 0)
@@ -358,6 +380,22 @@ func TestTxQuery(t *testing.T) {
 	}
 }
 
+func TestTxQueryInvalid(t *testing.T) {
+	db := newTestDB(t, "")
+	defer closeDB(t, db)
+
+	tx, err := db.Begin()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer tx.Rollback()
+
+	_, err = tx.Query("SELECT|t1|name|")
+	if err == nil {
+		t.Fatal("Error expected")
+	}
+}
+
 // Tests fix for issue 2542, that we release a lock when querying on
 // a closed connection.
 func TestIssue2542Deadlock(t *testing.T) {
@@ -562,3 +600,8 @@ func nullTestRun(t *testing.T, spec nullTestSpec) {
 		}
 	}
 }
+
+func stack() string {
+	buf := make([]byte, 1024)
+	return string(buf[:runtime.Stack(buf, false)])
+}