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

[Carsten Varming]

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.

Carsten

On Wed, Sep 28, 2016 at 10:13 AM, Vitaly Davidovich <vitalyd at gmail.com>
wrote:

> On Wednesday, September 28, 2016, David Holmes <david.holmes at 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
>