Skip to content

Commit

Permalink
connectionpool: fix deadlock on locked drain (#1047)
Browse files Browse the repository at this point in the history
Don't hold the pools mutex while closing the connections to prevent lock
reenetry from HandlErrors which can be called when the connection errors
while closing.
  • Loading branch information
Zariel committed Jan 6, 2018
1 parent 0b27912 commit 6c01199
Showing 1 changed file with 20 additions and 20 deletions.
40 changes: 20 additions & 20 deletions connectionpool.go
Expand Up @@ -339,14 +339,32 @@ func (pool *hostConnPool) Size() int {
//Close the connection pool
func (pool *hostConnPool) Close() {
pool.mu.Lock()
defer pool.mu.Unlock()

if pool.closed {
pool.mu.Unlock()
return
}
pool.closed = true

pool.drainLocked()
// ensure we dont try to reacquire the lock in handleError
// TODO: improve this as the following can happen
// 1) we have locked pool.mu write lock
// 2) conn.Close calls conn.closeWithError(nil)
// 3) conn.closeWithError calls conn.Close() which returns an error
// 4) conn.closeWithError calls pool.HandleError with the error from conn.Close
// 5) pool.HandleError tries to lock pool.mu
// deadlock

// empty the pool
conns := pool.conns
pool.conns = nil

pool.mu.Unlock()

// close the connections
for _, conn := range conns {
conn.Close()
}
}

// Fill the connection pool
Expand Down Expand Up @@ -551,21 +569,3 @@ func (pool *hostConnPool) HandleError(conn *Conn, err error, closed bool) {
}
}
}

func (pool *hostConnPool) drainLocked() {
// empty the pool
conns := pool.conns
pool.conns = nil

// close the connections
for _, conn := range conns {
conn.Close()
}
}

// removes and closes all connections from the pool
func (pool *hostConnPool) drain() {
pool.mu.Lock()
defer pool.mu.Unlock()
pool.drainLocked()
}

0 comments on commit 6c01199

Please sign in to comment.