9 RFR: 8178147: Race conditions in timeout handling code in http/2 incubator client

[Daniel Fuchs]

On 07/04/2017 14:31, Chris Hegarty wrote:
> Daniel,
>
> On 06/04/17 11:32, Daniel Fuchs wrote:
>> ...
>>
>> webrev:
>> http://cr.openjdk.java.net/~dfuchs/webrev_8170940/webrev.00/
>
> Looks good Daniel. Just a few comments.
>
> 1) Http1Exchange.java
>
>   Can 'operations' now be made private, and not a synchronizedList?
>   Now that it is operated on only within synchronized blocks.

Yes - will do and retest before pushing. Good catch.

> 2) Exchange.java  L197 : synchronized(this)
>
>   I'm not sure what this synchronized block gets you? The code
>   is racy, and both fields are volatile. Can this be removed?

No. This is the double-locking idiom and the synchronized is necessary
for atomic behavior (in order to get a consistent snapshot of the
two fields).
It's equivalent to a CAS where the compare would be done on two fields
and the set on only one of them.

best,

-- daniel

>
> -Chris.