Dear David,
See inline.
On Wed, Sep 28, 2016 at 7:47 PM, David Holmes <david.hol...@oracle.com>
wrote:
> On 29/09/2016 3:44 AM, Carsten Varming 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?
>>
>
> Because String has final fields there is a freeze action at the end of
> construction so that String instances are always safely published even if
> not "safely published".
>
I always thought that the freeze action only freezes final fields. The hash
field in String is not final and example 17.5-1 is applicable as far as I
can see (
https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5).
Has the memory model changed in JDK9 to invalidate example 17.5-1 or I am
missing something about String.
Carsten
> David
>
>
> [1]: Meaning the is no happens-before relationship established between
>> object construction and another thread calling hashCode on the object.
>>
>> Carsten
>>
>> On Wed, Sep 28, 2016 at 10:13 AM, Vitaly Davidovich <vita...@gmail.com
>> <mailto:vita...@gmail.com>> wrote:
>>
>> On Wednesday, September 28, 2016, David Holmes
>> <david.hol...@oracle.com <mailto: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-
>> <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
>> <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
>> <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
>>
>>
>>
