From: Ian Gudger Date: Thu, 1 Oct 2015 10:29:27 +0000 (-0700) Subject: database/sql: fix case where Stmt.Close discards error X-Git-Tag: go1.6beta1~932 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=73fe61233be672db13eeaa277aebb98c16c95996;p=gostls13.git database/sql: fix case where Stmt.Close discards error Fixes a case where the Stmt.Close() function in database/sql discards any error generated by the Close() function of the contained driverStmt. Fixes #12798 Change-Id: I40384d6165856665b062d15a643e4ecc09d63fda Reviewed-on: https://go-review.googlesource.com/15178 Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go index 8a2d781330..fbb0e594a5 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -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() diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go index b4135a3078..e1063bbc6b 100644 --- a/src/database/sql/sql_test.go +++ b/src/database/sql/sql_test.go @@ -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")