From ae9712c76688a07e1149958994a9ab3e5a99c37f Mon Sep 17 00:00:00 2001 From: Daniel Theophanes Date: Thu, 1 Dec 2016 11:07:11 -0800 Subject: [PATCH] database/sql: ensure Commit and Rollback return ErrTxDone Ensure documented behavior of returning ErrTxDone if the Tx has already been committed or rolled back. Fixes #18147 Change-Id: I07dc75bef4dbd4dd88dd252c96dc8ab99f28c00e Reviewed-on: https://go-review.googlesource.com/33793 TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick --- src/database/sql/sql.go | 6 +++--- src/database/sql/sql_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go index e11a9dadd0..3d957e1450 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -1449,14 +1449,14 @@ func (tx *Tx) closePrepared() { // Commit commits the transaction. func (tx *Tx) Commit() error { + if tx.isDone() { + return ErrTxDone + } select { default: case <-tx.ctx.Done(): return tx.ctx.Err() } - if tx.isDone() { - return ErrTxDone - } var err error withLock(tx.dc, func() { err = tx.txi.Commit() diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go index 02746a2e30..1ec6217858 100644 --- a/src/database/sql/sql_test.go +++ b/src/database/sql/sql_test.go @@ -683,6 +683,37 @@ func TestQueryRow(t *testing.T) { } } +func TestTxRollbackCommitErr(t *testing.T) { + db := newTestDB(t, "people") + defer closeDB(t, db) + + tx, err := db.Begin() + if err != nil { + t.Fatal(err) + } + err = tx.Rollback() + if err != nil { + t.Errorf("expected nil error from Rollback; got %v", err) + } + err = tx.Commit() + if err != ErrTxDone { + t.Errorf("expected %q from Commit; got %q", ErrTxDone, err) + } + + tx, err = db.Begin() + if err != nil { + t.Fatal(err) + } + err = tx.Commit() + if err != nil { + t.Errorf("expected nil error from Commit; got %v", err) + } + err = tx.Rollback() + if err != ErrTxDone { + t.Errorf("expected %q from Rollback; got %q", ErrTxDone, err) + } +} + func TestStatementErrorAfterClose(t *testing.T) { db := newTestDB(t, "people") defer closeDB(t, db)