[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3035: [DUBBO-3023]: problem in ActiveLimitFilter

["beiwei30 (GitHub)" <gi...@apache.org>]

If we add max>0 in the condition there it will cause the issue because in finally block we always reducing the endCount irrespective of max value. 

e.g. 
1)max value is 0 so in RpcStatus.java's begin count we will not increment the counter but in ActiveLimitFilter's invoke's finally block we will be always reducing so the final value for single invocation will landed up -1 active count.

I think not  including max>0 and adding 

```java
} finally {      
    if (max > 0) {
                RpcStatus.endCount(url, methodName, System.currentTimeMillis() - begin, isSuccess);
                synchronized (count) {
                    count.notifyAll();
                   }
               }
         }
```

In ActiveLimitFilter should also work, but I beleive you can rewrite this into more clear way then we suggest.


I think we might have another issue too (not because of your changes), in a scenario if max>0 (in RpcStatus.java) get satisfied (or lets max>0 condition is not even present in code as earlier version) and the `methodStatus.active.incrementAndGet() > max` return `false` then code of else block will be executing and again it will increment the count. So for one call we are eventually incrementing counter twice. Do you feel this might be also a another problem in future?Correct me if my understanding not correct.

[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3035 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org