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".
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 <[email protected] <mailto:[email protected]>> wrote: On Wednesday, September 28, 2016, David Holmes <[email protected] <mailto:[email protected]>> 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
