I am not that familiar with the code, but it seems to me that if we have a freeConn, it should always be used. If so, maybe we should:
* finally, make the new driverConn.
This allows for simplification of the buggy if condition. To be concrete:
diff -r 87dea3f5ebe7 src/pkg/database/sql/sql.go
--- a/src/pkg/database/sql/sql.go Fri Nov 29 08:32:31 2013 +1100
+++ b/src/pkg/database/sql/sql.go Tue Dec 10 20:00:45 2013 -0500
@@ -619,9 +619,18 @@
return nil, errDBClosed
}
- // If db.maxOpen > 0 and the number of open connections is over the limit
- // or there are no free connection, then make a request and wait.
- if db.maxOpen > 0 && (db.numOpen >= db.maxOpen || db.freeConn.Len() == 0) {
+ if f := db.freeConn.Front(); f != nil {
+ conn := f.Value.(*driverConn)
+ conn.listElem = nil
+ db.freeConn.Remove(f)
+ conn.inUse = true
+ db.mu.Unlock()
+ return conn, nil
+ }
+
+ // If db.maxOpen > 0 and the number of open connections is over the limit,
+ // then make a request and wait.
+ if db.maxOpen > 0 && db.numOpen >= db.maxOpen {
// Make the connRequest channel. It's buffered so that the
// connectionOpener doesn't block while waiting for the req to be read.
ch := make(chan interface{}, 1)
@@ -643,15 +652,6 @@
}
}
- if f := db.freeConn.Front(); f != nil {
- conn := f.Value.(*driverConn)
- conn.listElem = nil
- db.freeConn.Remove(f)
- conn.inUse = true
- db.mu.Unlock()
- return conn, nil
- }
-
db.mu.Unlock()
ci, err := db.driver.Open(db.dsn)
if err != nil {
Does this seem reasonable to those with more experience? If so, I can send a change if helpful.