rueian commented on a change in pull request #248:
URL: https://github.com/apache/pulsar-client-go/pull/248#discussion_r426214384
##########
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:
Yes, I agree that moving the above code into the run loop is better and
simpler. Actually, it was my first attempt to solve this problem.
However, I am not sure we can move only the above code from `Close()` into
the run loop. There are still some cleanup on the `c.listeners` and
`c.consumerHandlers` maps in the following code. Should we move them into the
run loop together to prevent creating another race condition in case of they
have ordering requirement between each other?
Also, If we move any of them into the run loop, the `Close()` will become an
async operation which I am not sure whether it would break the other code or
not.
Any thought?
----------------------------------------------------------------
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
