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?
If we move one of them into the run loop, the `Close()` will become an async
operation which I am not sure whether it will also break the other code.
Any feedback?
----------------------------------------------------------------
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
