[GitHub] [pulsar] Jason918 commented on a change in pull request #14656: Fix NPE cause by duplicated removeSubscription

[GitBox <gi...@apache.org>]

Jason918 commented on a change in pull request #14656:
URL: https://github.com/apache/pulsar/pull/14656#discussion_r824400327



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java
##########
@@ -331,7 +331,6 @@ public synchronized void removeConsumer(Consumer consumer, boolean isResetCursor
                 // when topic closes: it iterates through concurrent-subscription map to close each subscription. so,
                 // topic.remove again try to access same map which creates deadlock. so, execute it in different thread.
                 topic.getBrokerService().pulsar().getExecutor().submit(() ->{
-                    topic.removeSubscription(subName);

Review comment:
       I think there may be some side effect to remove this line, if `removeConsumer` is not called from `doUnsubscribe`. Maybe it's better to make `topic.removeSubscription` support removing non-exists subName (Just add a null check in `removeSubscription`).




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org