On Thu, 10 Mar 2022 10:08:16 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> tryRelease can be called multiple time - it will only decrement the ref >> counting once. It could happen when different threads notice that the >> operation is finished (usually due to some exceptions being propagated) and >> try concurrently to decrease the ref counter. It is very important to >> decrease the ref counter (otherwise the HttpClient may never shutdown when >> it's released) and equally very important not to decrease it twice for the >> same operation. Here we don't need to check whether it's been acquired >> because we know it's been acquired if we reach here. IIRC what we don't want >> to do is call tryRelease() if it's never been acquired. > >> Here we don't need to check whether it's been acquired because we know it's >> been acquired if we reach here. > > Hello Daniel, may be I am misreading the diff but from what I can see, the > `acquire()` method now returns a `boolean` which we are assigning to the > local `acquire`. But we aren't checking it for `true` for doing the > subsequent operation. So when it reaches the finally block here, as far as I > can see, it's not guaranteed that we did indeed `acquire` it. The acquire() method will return true the first time it's been called. And it is called only once. So we only need to check whether `acquired` is true at places where we are in doubt about whether the method has been called. The code that calls acquire() above could simply have ignored the result of `acquire()` and set the boolean to `true`. That said, it would not be wrong to check whether `acquired==true` here too - maybe I should do it for consistency... ------------- PR: https://git.openjdk.java.net/jdk/pull/7196
