On Wednesday, September 28, 2016, Carsten Varming <cvarm...@twitter.com>
wrote:
> Dear Vitaly and David,
>
> Looking at your webrev the original code is:
>
> public int hashCode() {
> if (hash == 0 && value.length > 0) {
> hash = isLatin1() ? StringLatin1.hashCode(value)
> }
> return hash;
> }
>
> There is a constructor:
>
> public String(String original) {
> this.value = original.value;
> this.coder = original.coder;
> this.hash = original.hash;
> }
>
> that might write zero to the mutable field "hash".
>
> The object created by this constructor might be shared using plain reads
> and writes between two threads[1] and the write of 0 in the constructor
> might be interleaved with the reads and write in hashCode. Does this
> capture the problem?
>
> [1]: Meaning the is no happens-before relationship established between
> object construction and another thread calling hashCode on the object.
>
I think the issue is more fundamental - multiple threads calling hashCode
on the same instance. AFAIK, Java prohibits replacing an explicit local
with a (re-) read of the underlying field. But, otherwise I believe
compiler has latitude to schedule loads and reloads as it pleases. But I
think this may need to be clarified since this aspect has been brought up.
>
> Carsten
>
> On Wed, Sep 28, 2016 at 10:13 AM, Vitaly Davidovich <vita...@gmail.com
> <javascript:_e(%7B%7D,'cvml','vita...@gmail.com');>> wrote:
>
>> On Wednesday, September 28, 2016, David Holmes <david.hol...@oracle.com
>> <javascript:_e(%7B%7D,'cvml','david.hol...@oracle.com');>>
>> 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.
>> >
>> > Your bug requires that the code act as-if written:
>> >
>> > int temp = hash;
>> > if (temp == 0) {
>> > hash = ...
>> > }
>> > return temp;
>>
>> It's the other way I think:
>>
>> int temp = hash; // read 0
>> if (hash == 0) // reread a non 0
>> hash = temp = ...
>> return temp // return 0
>>
>> It's unlikely but what prohibits that?
>>
>> >
>> > and I do not believe that is allowed.
>> >
>> > David
>> >
>> >
>> >> 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
>> >>
>> >>
>>
>> --
>> Sent from my phone
>>
>
>
--
Sent from my phone
