RobertIndie commented on code in PR #963:
URL: https://github.com/apache/pulsar-client-go/pull/963#discussion_r1122775677
##########
pulsar/internal/connection.go:
##########
@@ -936,23 +936,29 @@ func (c *connection) ResetLastActive() {
}
func (c *connection) isIdle() bool {
- c.pendingLock.Lock()
- if len(c.pendingReqs) != 0 {
- return false
+ {
+ c.pendingLock.Lock()
+ defer c.pendingLock.Unlock()
+ if len(c.pendingReqs) != 0 {
+ return false
+ }
}
- c.pendingLock.Unlock()
- c.listenersLock.RLock()
- if len(c.listeners) != 0 {
- return false
+ {
+ c.listenersLock.RLock()
Review Comment:
```
c.pendingLock.Lock()
if len(c.pendingReqs) != 0 {
return false
}
c.pendingLock.Unlock()
```
In the previous code, I forgot to unlock pendingLock when pendingReqs is not
empty. So it's flaky because there may be some message in the pendingReqs when
triggering the idle check.
> I don't understand why it is in {}.
Using `{}` here is to make sure we only acquire one lock at a time to avoid
deadlock. So the lock will be automatically unlocked when out of this scope.
##########
pulsar/internal/connection.go:
##########
@@ -936,23 +936,29 @@ func (c *connection) ResetLastActive() {
}
func (c *connection) isIdle() bool {
- c.pendingLock.Lock()
- if len(c.pendingReqs) != 0 {
- return false
+ {
+ c.pendingLock.Lock()
+ defer c.pendingLock.Unlock()
+ if len(c.pendingReqs) != 0 {
+ return false
+ }
}
- c.pendingLock.Unlock()
- c.listenersLock.RLock()
- if len(c.listeners) != 0 {
- return false
+ {
+ c.listenersLock.RLock()
Review Comment:
```
c.pendingLock.Lock()
if len(c.pendingReqs) != 0 {
return false
}
c.pendingLock.Unlock()
```
In the previous code, I forgot to unlock pendingLock when pendingReqs is not
empty. So it's flaky because there may be some message in the pendingReqs when
triggering the idle check.
> I don't understand why it is in {}.
Using `{}` here is to make sure we only acquire one lock at a time to avoid
deadlock. So the lock will be automatically unlocked when out of this scope.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
