Skip to content

Commit

Permalink
fix: Leak in AutoReconnect
Browse files Browse the repository at this point in the history
Wrong order in client.run and goroutine leak
  • Loading branch information
philippseith committed Nov 17, 2021
1 parent 8dfba5d commit 63fb61e
Showing 1 changed file with 14 additions and 21 deletions.
35 changes: 14 additions & 21 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,6 @@ func (c *client) Start() {
case <-c.ctx.Done():
return
}
// Clear last connection
c.mx.Lock()
c.conn = nil
c.mx.Unlock()
c.setState(ClientConnecting)
}
}()
Expand All @@ -186,15 +182,17 @@ func (c *client) run() error {
}()
// Run the loop
err = loop.Run(isLoopConnected)
if err != nil {
return err
}
err = loop.hubConn.Close("", false) // allowReconnect value is ignored as servers never initiate a connection

// Reset conn to allow reconnecting
c.mx.Lock()
c.conn = nil
c.mx.Unlock()

if err != nil {

This comment has been minimized.

Copy link
@andig

andig Nov 17, 2021

Contributor

You could simplify:

if err == nil { err = hubConn.Close() }
return err
}
err = loop.hubConn.Close("", false) // allowReconnect value is ignored as servers never initiate a connection

return err
}

Expand Down Expand Up @@ -254,21 +252,15 @@ func (c *client) setState(state ClientState) {
c.state = state
for _, ch := range c.stateChangeChans {
go func(ch chan struct{}) {
select {
case ch <- struct{}{}:
c.mx.RLock()
found := false
for _, cch := range c.stateChangeChans {
if cch == ch {
found = true
break
c.mx.Lock()
defer c.mx.Unlock()
for _, cch := range c.stateChangeChans {
if cch == ch {
select {
case ch <- struct{}{}:
case <-c.ctx.Done():
}
}
if !found {
close(ch)
}
c.mx.RUnlock()
case <-c.ctx.Done():
}
}(ch)
}
Expand All @@ -289,6 +281,7 @@ func (c *client) cancelObserveStateChanged(ch chan struct{}) {
for i, cch := range c.stateChangeChans {
if cch == ch {
c.stateChangeChans = append(c.stateChangeChans[:i], c.stateChangeChans[i+1:]...)
close(ch)
break
}
}
Expand Down

0 comments on commit 63fb61e

Please sign in to comment.