cckellogg commented on a change in pull request #248:
URL: https://github.com/apache/pulsar-client-go/pull/248#discussion_r429725687
##########
File path: pulsar/internal/connection.go
##########
@@ -656,18 +662,20 @@ func (c *connection) Close() {
c.log.Info("Connection closed")
c.state = connectionClosed
- if c.cnx != nil {
- c.cnx.Close()
- }
+ c.TriggerClose()
c.pingTicker.Stop()
c.pingCheckTicker.Stop()
for _, listener := range c.listeners {
listener.ConnectionClosed()
}
- for _, req := range c.pendingReqs {
+ if c.runLoopStoppedCh != nil {
Review comment:
Do you see the run loop ever getting stuck for a period of time when
close is called? I think we should probably only pendingReqs cleanup in the run
loop since that map is only accessed in that go routine . We could keep the
others where they are since they are protected by locks. We just need to add
some comments on why this is the case and why the runLoopStoppedCh is needed.
Thoughts?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
