Skip to content

Commit f548fb2

Browse files
kirk91andybons
authored andcommitted
[release-branch.go1.9] database/sql: fix transaction leak
When the user context which passed in (*DB)BeginTx is canceled or timeout, the current implementation could cause db transaction leak in some extreme scenario. Goroutine 1: Call (*DB) BeginTx begins a transaction with a userContext. In (*DB)BeginTx, a new goroutine (*Tx)awaitDone which monitor context and rollback tx if needed will be created Goroutine 2(awaitDone): block on tx.ctx.Done() Goroutine 1: Execute some insert or update sqls on the database Goroutine 1: Commit the transaction, (*Tx)Commit set the atomic variable tx.done to 1 Goroutine 3(maybe global timer): Cancel userContext which be passed in Tx Goroutine 1: (*Tx)Commit checks tx.ctx.Done(). Due to the context has been canceled, it will return context.Canceled or context.DeadlineExceeded error immediately and abort the real COMMIT operation of transaction Goroutine 2: Release with tx.ctx.Done() signal, execute (*Tx)rollback. However the atomic variable tx.done is 1 currently, it will return ErrTxDone error immediately and abort the real ROLLBACK operation of transaction Fixes #22976 Change-Id: I3bc23adf25db823861d91e33d3cca6189fb1171d Reviewed-on: https://go-review.googlesource.com/81736 Run-TryBot: Brad Fitzpatrick <[email protected]> Reviewed-by: Daniel Theophanes <[email protected]> Reviewed-on: https://go-review.googlesource.com/88323 Run-TryBot: Andrew Bonventre <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent 0b25e97 commit f548fb2

File tree

1 file changed

+9
-3
lines changed

1 file changed

+9
-3
lines changed

src/database/sql/sql.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1769,14 +1769,20 @@ func (tx *Tx) closePrepared() {
17691769

17701770
// Commit commits the transaction.
17711771
func (tx *Tx) Commit() error {
1772-
if !atomic.CompareAndSwapInt32(&tx.done, 0, 1) {
1773-
return ErrTxDone
1774-
}
1772+
// Check context first to avoid transaction leak.
1773+
// If put it behind tx.done CompareAndSwap statement, we cant't ensure
1774+
// the consistency between tx.done and the real COMMIT operation.
17751775
select {
17761776
default:
17771777
case <-tx.ctx.Done():
1778+
if atomic.LoadInt32(&tx.done) == 1 {
1779+
return ErrTxDone
1780+
}
17781781
return tx.ctx.Err()
17791782
}
1783+
if !atomic.CompareAndSwapInt32(&tx.done, 0, 1) {
1784+
return ErrTxDone
1785+
}
17801786
var err error
17811787
withLock(tx.dc, func() {
17821788
err = tx.txi.Commit()

0 commit comments

Comments
 (0)