Skip to content

Commit

Permalink
database/sql: fix case where Stmt.Close discards error
Browse files Browse the repository at this point in the history
Fixes a case where the Stmt.Close() function in database/sql discards any error generated by the Close() function of the contained driverStmt.

Fixes golang#12798

Change-Id: I40384d6165856665b062d15a643e4ecc09d63fda
Reviewed-on: https://go-review.googlesource.com/15178
Reviewed-by: Brad Fitzpatrick <[email protected]>
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
  • Loading branch information
iangudger authored and bradfitz committed Oct 2, 2015
1 parent f80ff56 commit 73fe612
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/database/sql/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -1576,9 +1576,9 @@ func (s *Stmt) Close() error {
s.closed = true

if s.tx != nil {
s.txsi.Close()
err := s.txsi.Close()
s.mu.Unlock()
return nil
return err
}
s.mu.Unlock()

Expand Down
38 changes: 38 additions & 0 deletions src/database/sql/sql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,44 @@ func TestStatementQueryRow(t *testing.T) {
}
}

type stubDriverStmt struct {
err error
}

func (s stubDriverStmt) Close() error {
return s.err
}

func (s stubDriverStmt) NumInput() int {
return -1
}

func (s stubDriverStmt) Exec(args []driver.Value) (driver.Result, error) {
return nil, nil
}

func (s stubDriverStmt) Query(args []driver.Value) (driver.Rows, error) {
return nil, nil
}

// golang.org/issue/12798
func TestStatementClose(t *testing.T) {
want := errors.New("STMT ERROR")

tests := []struct {
stmt *Stmt
msg string
}{
{&Stmt{stickyErr: want}, "stickyErr not propagated"},
{&Stmt{tx: &Tx{}, txsi: &driverStmt{&sync.Mutex{}, stubDriverStmt{want}}}, "driverStmt.Close() error not propagated"},
}
for _, test := range tests {
if err := test.stmt.Close(); err != want {
t.Errorf("%s. Got stmt.Close() = %v, want = %v", test.msg, err, want)
}
}
}

// golang.org/issue/3734
func TestStatementQueryRowConcurrent(t *testing.T) {
db := newTestDB(t, "people")
Expand Down

0 comments on commit 73fe612

Please sign in to comment.