dlg99 commented on code in PR #3365:
URL: https://github.com/apache/bookkeeper/pull/3365#discussion_r911609146
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java:
##########
@@ -402,6 +402,7 @@ public void registerLedgerMetadataListener(long ledgerId,
LedgerMetadataListener
}
}
synchronized (listenerSet) {
+ listenerSet = listeners.computeIfAbsent(ledgerId, k -> new
HashSet<>());
Review Comment:
with this
```
Set<LedgerMetadataListener> listenerSet =
listeners.computeIfAbsent(ledgerId, k -> new HashSet<>());
synchronized (listenerSet) {
listenerSet = listeners.computeIfAbsent(ledgerId, k -> new
HashSet<>());
listenerSet.add(listener);
}
```
it looks like the codes assume that between two calls of
`listeners.computeIfAbsent` it may need to create a new instance (listeners can
be modified/removed concurrently.)
so you are getting (or creating) one instance to synchronize on, meanwhile,
listeners change, now synchronize locks on one instance of listenerSet while
modifying another.
so either the second call to computeIfAbsent is not needed or the locking
needs to be done differently
--
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]
