[GitHub] [pulsar-client-go] cckellogg commented on a change in pull request #248: Clean callbacks of connection after run loop stopped (#239)

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org