diff options
author | kirk <kirk91.han@gmail.com> | 2017-12-04 10:53:53 +0800 |
---|---|---|
committer | Andrew Bonventre <andybons@golang.org> | 2018-01-22 20:25:05 +0000 |
commit | f548fb287bdd415d6278d9bb9cc726e754c93c8a (patch) | |
tree | b180dd9b481cc1697149d4fb47361ace2e8268ad | |
parent | 0b25e97d03b520d8507342d3c7df9b98ebcbe2c0 (diff) | |
download | go-f548fb287bdd415d6278d9bb9cc726e754c93c8a.tar.gz go-f548fb287bdd415d6278d9bb9cc726e754c93c8a.zip |
[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 <bradfitz@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Reviewed-on: https://go-review.googlesource.com/88323
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
-rw-r--r-- | src/database/sql/sql.go | 12 |
1 files changed, 9 insertions, 3 deletions
diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go index 89976c7fd0..97f4016fbd 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -1769,14 +1769,20 @@ func (tx *Tx) closePrepared() { // Commit commits the transaction. func (tx *Tx) Commit() error { - if !atomic.CompareAndSwapInt32(&tx.done, 0, 1) { - return ErrTxDone - } + // Check context first to avoid transaction leak. + // If put it behind tx.done CompareAndSwap statement, we cant't ensure + // the consistency between tx.done and the real COMMIT operation. select { default: case <-tx.ctx.Done(): + if atomic.LoadInt32(&tx.done) == 1 { + return ErrTxDone + } return tx.ctx.Err() } + if !atomic.CompareAndSwapInt32(&tx.done, 0, 1) { + return ErrTxDone + } var err error withLock(tx.dc, func() { err = tx.txi.Commit() |