Skip to content

Commit bcf964d

Browse files
kirk91kardianos
authored andcommitted
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]>
1 parent dd7cbf3 commit bcf964d

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
@@ -1913,14 +1913,20 @@ func (tx *Tx) closePrepared() {
19131913

19141914
// Commit commits the transaction.
19151915
func (tx *Tx) Commit() error {
1916-
if !atomic.CompareAndSwapInt32(&tx.done, 0, 1) {
1917-
return ErrTxDone
1918-
}
1916+
// Check context first to avoid transaction leak.
1917+
// If put it behind tx.done CompareAndSwap statement, we cant't ensure
1918+
// the consistency between tx.done and the real COMMIT operation.
19191919
select {
19201920
default:
19211921
case <-tx.ctx.Done():
1922+
if atomic.LoadInt32(&tx.done) == 1 {
1923+
return ErrTxDone
1924+
}
19221925
return tx.ctx.Err()
19231926
}
1927+
if !atomic.CompareAndSwapInt32(&tx.done, 0, 1) {
1928+
return ErrTxDone
1929+
}
19241930
var err error
19251931
withLock(tx.dc, func() {
19261932
err = tx.txi.Commit()

0 commit comments

Comments
 (0)