Skip to content

Commit

Permalink
Fix a deadlock in Session.Close() during control connection reconnect
Browse files Browse the repository at this point in the history
We switched from separate mutex for closing to sessionStateMu in
312a614.
This change introduced a deadlock.

We don't need to hold the mutex for the whole duration of Close(),
we only need to update the status atomically.
Previously IsClosed() returned true only after all closing is done
(because of the deferred unlock).
We can emulate that by setting isClosed at the end of Close().
We need a new variable to ensure that Close() is only executed once.

Fixes #1687
  • Loading branch information
martin-sucha committed Apr 11, 2023
1 parent 1c9ec8f commit b8c4e16
Showing 1 changed file with 11 additions and 4 deletions.
15 changes: 11 additions & 4 deletions session.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,10 @@ type Session struct {

// sessionStateMu protects isClosed and isInitialized.
sessionStateMu sync.RWMutex
// isClosed is true once Session.Close is called.
// isClosed is true once Session.Close is finished.
isClosed bool
// isClosing bool is true once Session.Close is started.
isClosing bool
// isInitialized is true once Session.init succeeds.
// you can use initialized() to read the value.
isInitialized bool
Expand Down Expand Up @@ -463,11 +465,12 @@ func (s *Session) Bind(stmt string, b func(q *QueryInfo) ([]interface{}, error))
func (s *Session) Close() {

s.sessionStateMu.Lock()
defer s.sessionStateMu.Unlock()
if s.isClosed {
if s.isClosing {
s.sessionStateMu.Unlock()
return
}
s.isClosed = true
s.isClosing = true
s.sessionStateMu.Unlock()

if s.pool != nil {
s.pool.Close()
Expand All @@ -488,6 +491,10 @@ func (s *Session) Close() {
if s.cancel != nil {
s.cancel()
}

s.sessionStateMu.Lock()
s.isClosed = true
s.sessionStateMu.Unlock()
}

func (s *Session) Closed() bool {
Expand Down

0 comments on commit b8c4e16

Please sign in to comment.