Skip to content

Commit

Permalink
abci: minor cleanups in the socket client (#3758)
Browse files Browse the repository at this point in the history
Follow up from #3512

Specifically:

    cli.conn.Close() need not be under the mutex (#3512 (comment))
    call the reqRes callback after the resCb so they always happen in the same order (#3512)

Fixes #3513
  • Loading branch information
melekes committed Jul 2, 2019
1 parent d041476 commit e645442
Showing 1 changed file with 7 additions and 15 deletions.
22 changes: 7 additions & 15 deletions abci/client/socket_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,6 @@ func NewSocketClient(addr string, mustConnect bool) *socketClient {
}

func (cli *socketClient) OnStart() error {
if err := cli.BaseService.OnStart(); err != nil {
return err
}

var err error
var conn net.Conn
RETRY_LOOP:
Expand All @@ -82,15 +78,12 @@ RETRY_LOOP:
}

func (cli *socketClient) OnStop() {
cli.BaseService.OnStop()

cli.mtx.Lock()
defer cli.mtx.Unlock()
if cli.conn != nil {
// does this really need a mutex?
cli.conn.Close()
}

cli.mtx.Lock()
defer cli.mtx.Unlock()
cli.flushQueue()
}

Expand Down Expand Up @@ -209,19 +202,18 @@ func (cli *socketClient) didRecvResponse(res *types.Response) error {
reqres.Done() // Release waiters
cli.reqSent.Remove(next) // Pop first item from linked list

// Notify client listener if set (global callback).
if cli.resCb != nil {
cli.resCb(reqres.Request, res)
}

// Notify reqRes listener if set (request specific callback).
// NOTE: it is possible this callback isn't set on the reqres object.
// at this point, in which case it will be called after, when it is set.
// TODO: should we move this after the resCb call so the order is always consistent?
if cb := reqres.GetCallback(); cb != nil {
cb(res)
}

// Notify client listener if set (global callback).
if cli.resCb != nil {
cli.resCb(reqres.Request, res)
}

return nil
}

Expand Down

0 comments on commit e645442

Please sign in to comment.