On Wednesday, September 28, 2016, Carsten Varming <cvarm...@twitter.com>
wrote:
> Dear David,
>
> See inline.
>
> On Wed, Sep 28, 2016 at 7:47 PM, David Holmes <david.hol...@oracle.com
> <javascript:_e(%7B%7D,'cvml','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.
>
Right - the spec only talks about final fields being ok in such cases. In
practice, compilers put a freeze action at the end of the constructor and
non-final fields come along for the ride, AFAIK.
However, I'm not sure this example is all that interesting. Suppose you
published a String copied from another, but the hash value was reordered
and didn't appear to the reader. That ought to be fine because hashCode is
supposed to recompute it. At worst, you perform the same computation
"unnecessarily".
The possible compiler reordering, as discussed in this thread, within
hashCode itself is problematic because it can, theoretically, yield 0
erroneously.
>
> 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
>>> <javascript:_e(%7B%7D,'cvml','vita...@gmail.com');>
>>> <mailto: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');> <mailto:
>>> 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-
>>> <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
>>>
>>>
>>>
>
--
Sent from my phone
