[GitHub] [rocketmq] aftersss opened a new pull request #1431: Fix thread unsafe issue in `ConsumerFilterManager`

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

aftersss opened a new pull request #1431: Fix thread unsafe issue in `ConsumerFilterManager`
URL: https://github.com/apache/rocketmq/pull/1431
 
 
   `ConsumerFilterManager` is not thread safe:
   `ConsumerFilterManager.clean` may remove entry of filterDataByTopic, when this occur, the following code may throw NullPointerException:
   
   ```
   
       public final Collection<ConsumerFilterData> get(final String topic) {
           if (!this.filterDataByTopic.containsKey(topic)) {
               return null;
           }
           //`this.filterDataByTopic.get(topic)` may be null here because it is removed in `ConsumerFilterManager.clean` by another thread.
           if (this.filterDataByTopic.get(topic).getGroupFilterData().isEmpty()) {
               return null;
           }
   
           return this.filterDataByTopic.get(topic).getGroupFilterData().values();
       }
   ```
   
   I have also fixed some other places where may throw `NullPointerException`.
   
   
   
   And `ConsumerFilterManager.register` may loss some registries:
   
   ```
   public boolean register(final String topic, final String consumerGroup, final String expression,
           final String type, final long clientVersion) {
           if (ExpressionType.isTagType(type)) {
               return false;
           }
   
           if (expression == null || expression.length() == 0) {
               return false;
           }
   
           FilterDataMapByTopic filterDataMapByTopic = this.filterDataByTopic.get(topic);
   
           if (filterDataMapByTopic == null) { 
               FilterDataMapByTopic temp = new FilterDataMapByTopic(topic);
               FilterDataMapByTopic prev = this.filterDataByTopic.putIfAbsent(topic, temp);
               filterDataMapByTopic = prev != null ? prev : temp;
           }
   
   // If filterDataMapByTopic.getGroupFilterData() is empty here, and another thread runs `clean`, filterDataMapByTopic will be removed from filterDataByTopic, the current registry will be lost.
   
           BloomFilterData bloomFilterData = bloomFilter.generate(consumerGroup + "#" + topic);
   
           return filterDataMapByTopic.register(consumerGroup, expression, type, bloomFilterData, clientVersion);
       }
   ```
   
   Please notice the comment in the above code, it shows how the registry lost happens.
   I modified the code to use `compute`, this will fix this. 
   
   
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services