Jason918 commented on a change in pull request #12654:
URL: https://github.com/apache/pulsar/pull/12654#discussion_r747429235
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java
##########
@@ -474,7 +475,13 @@ public void registerListener(TopicName topicName,
TopicPolicyListener<TopicPolic
@Override
public void unregisterListener(TopicName topicName,
TopicPolicyListener<TopicPolicies> listener) {
- listeners.computeIfAbsent(topicName, k ->
Lists.newCopyOnWriteArrayList()).remove(listener);
+ List<TopicPolicyListener<TopicPolicies>> topicListeners =
listeners.get(topicName);
+ if (topicListeners != null && topicListeners.remove(listener)) {
+ //delete topic from listeners if it's empty.
+ if (topicListeners.isEmpty()) {
Review comment:
The difference is that 'isEmpty' in ConcurrentHashMap just count the map
size, no lock required. But the `remove()` will find the key first and there is
a lock as it's a modification ops.
By `computeIfAbsent`, do you mean like `map.computeIfAbsent(k,k->{...
map.remove ...})`. Actually it's forbidden that call any modification method in
the computeIfAbsent. There is a risk of dead lock issue. See here:
--
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]
