RFR: 8166842: String.hashCode() has a non-benign data race

[Aleksey Shipilev]

Peter, David,

Would you mind discussing the theoretical topics on
concurrency-interest@, for the benefits of others who don't track
high-traffic OpenJDK list?

Thanks,
-Aleksey

On 09/28/2016 03:24 PM, Peter Levart wrote:
> 
> On 09/28/2016 03:05 PM, David Holmes wrote:
>> On 28/09/2016 10:44 PM, Peter Levart wrote:
>>> Hi,
>>>
>>> According to discussion here:
>>>
>>> http://cs.oswego.edu/pipermail/concurrency-interest/2016-September/015414.html
>>>
>>>
>>>
>>> it seems compact strings introduced (at least theoretical) non-benign
>>> data race into String.hasCode() method.
>>>
>>> Here is a proposed patch:
>>>
>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/8166842_String.hashCode/webrev.01/
>>>
>>
>> I'm far from convinced that the bug exists - theoretical or otherwise
>> - but the "fix" is harmless.
>>
>> When we were working on JSR-133 one of the conceptual models is that
>> every write to a variable goes into the set of values that a read may
>> potentially return (so no out-of-thin-air for example). happens-before
>> establishes constraints on which value can legally be returned - the
>> most recent. An additional property was that once a value was
>> returned, a later read could not return an earlier value - in essence
>> once a read returns a given value, all earlier written values are
>> removed from the set of potential values that can be read.
> 
> That would be a nice property, yes.
> 
>>
>> Your bug requires that the code act as-if written:
>>
>> int temp = hash;
>> if (temp == 0) {
>>    hash = ...
>> }
>> return temp;
>>
>> and I do not believe that is allowed.
>>
>> David
> 
> Well, I can't find anything like that in JMM description. Could you
> point me to it? Above example only reads once from hash. The code in
> question is this:
> 
> if (hash == 0) { // 1st read
>     hash = ...
> }
> return hash; // 2nd read
> 
> And the bug requires the code to act like:
> 
> int temp2 = hash; // 2nd read
> int temp1 = hash; // 1st read
> if (temp1 == 0) {
>     return (hash = ...);
> }
> return temp2;
> 
> 
> Is this allowed?
> 
> Regards, Peter
> 
>>
>>>
>>> For the bug:
>>>
>>>     https://bugs.openjdk.java.net/browse/JDK-8166842
>>>
>>>
>>>
>>> JDK 8 did not have this problem, so no back-porting necessary.
>>>
>>> Regards, Peter
>>>
>