Commit 1c441e25 authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

exp/sql: fix statement leak

Also verified in external test suite that this fixes MySQL
resource exhaustion problems, and also exposed a double-free
bug in the gosqlite3 driver (where gosqlite3 either got lucky
before, or was working around this bug)

R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/5544057
parent 0575cd9d
...@@ -77,6 +77,17 @@ type fakeConn struct { ...@@ -77,6 +77,17 @@ type fakeConn struct {
db *fakeDB // where to return ourselves to db *fakeDB // where to return ourselves to
currTx *fakeTx currTx *fakeTx
// Stats for tests:
mu sync.Mutex
stmtsMade int
stmtsClosed int
}
func (c *fakeConn) incrStat(v *int) {
c.mu.Lock()
*v++
c.mu.Unlock()
} }
type fakeTx struct { type fakeTx struct {
...@@ -338,6 +349,7 @@ func (c *fakeConn) Prepare(query string) (driver.Stmt, error) { ...@@ -338,6 +349,7 @@ func (c *fakeConn) Prepare(query string) (driver.Stmt, error) {
cmd := parts[0] cmd := parts[0]
parts = parts[1:] parts = parts[1:]
stmt := &fakeStmt{q: query, c: c, cmd: cmd} stmt := &fakeStmt{q: query, c: c, cmd: cmd}
c.incrStat(&c.stmtsMade)
switch cmd { switch cmd {
case "WIPE": case "WIPE":
// Nothing // Nothing
...@@ -358,7 +370,10 @@ func (s *fakeStmt) ColumnConverter(idx int) driver.ValueConverter { ...@@ -358,7 +370,10 @@ func (s *fakeStmt) ColumnConverter(idx int) driver.ValueConverter {
} }
func (s *fakeStmt) Close() error { func (s *fakeStmt) Close() error {
s.closed = true if !s.closed {
s.c.incrStat(&s.c.stmtsClosed)
s.closed = true
}
return nil return nil
} }
......
...@@ -243,8 +243,13 @@ func (db *DB) Query(query string, args ...interface{}) (*Rows, error) { ...@@ -243,8 +243,13 @@ func (db *DB) Query(query string, args ...interface{}) (*Rows, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
defer stmt.Close() rows, err := stmt.Query(args...)
return stmt.Query(args...) if err != nil {
stmt.Close()
return nil, err
}
rows.closeStmt = stmt
return rows, nil
} }
// QueryRow executes a query that is expected to return at most one row. // QueryRow executes a query that is expected to return at most one row.
...@@ -706,9 +711,10 @@ type Rows struct { ...@@ -706,9 +711,10 @@ type Rows struct {
releaseConn func() releaseConn func()
rowsi driver.Rows rowsi driver.Rows
closed bool closed bool
lastcols []interface{} lastcols []interface{}
lasterr error lasterr error
closeStmt *Stmt // if non-nil, statement to Close on close
} }
// Next prepares the next result row for reading with the Scan method. // Next prepares the next result row for reading with the Scan method.
...@@ -789,6 +795,9 @@ func (rs *Rows) Close() error { ...@@ -789,6 +795,9 @@ func (rs *Rows) Close() error {
rs.closed = true rs.closed = true
err := rs.rowsi.Close() err := rs.rowsi.Close()
rs.releaseConn() rs.releaseConn()
if rs.closeStmt != nil {
rs.closeStmt.Close()
}
return err return err
} }
......
...@@ -276,3 +276,21 @@ func TestIssue2542Deadlock(t *testing.T) { ...@@ -276,3 +276,21 @@ func TestIssue2542Deadlock(t *testing.T) {
} }
} }
} }
func TestQueryRowClosingStmt(t *testing.T) {
db := newTestDB(t, "people")
defer closeDB(t, db)
var name string
var age int
err := db.QueryRow("SELECT|people|age,name|age=?", 3).Scan(&age, &name)
if err != nil {
t.Fatal(err)
}
if len(db.freeConn) != 1 {
t.Fatalf("expected 1 free conn")
}
fakeConn := db.freeConn[0].(*fakeConn)
if made, closed := fakeConn.stmtsMade, fakeConn.stmtsClosed; made != closed {
t.Logf("statement close mismatch: made %d, closed %d", made, closed)
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment