On Fri, Dec 11, 2009 at 4:30 AM, Boris (JIRA) <[email protected]> wrote:
That's interesting. possible data-reordering in some hashCode-methods (e.g. String or URL) > ---------------------------------------------------------------------- > > Key: HARMONY-6404 > URL: https://issues.apache.org/jira/browse/HARMONY-6404 > Project: Harmony > Issue Type: Bug > Reporter: Boris > > > First I have to say that I don't know if the Java Memory Model is relevant > for Harmony, so please reject the bug if this is not the case. > > The current implementation of several of Harmony's classes that try to > cache hashCode in an optimized way are affected by a reordering-bug that can > occur because of the way the hashCode is cached and retrieved. > The problem is that the method contains no memory barriers, so that the VM > may reorder the data-reads at its own will. With an unlucky reordering the > method could return 0 although the actual hashCode is NOT 0. > > Or to speak in code: This actual code: > if (hashCode == 0) { > // calculate hash > hashCode = hash; > } > return hashCode; > could be reordered to something like > return hashCode; > if (hashCode == 0) { > // calculate hash > hashCode = hash; > } > (which is of course no valid Java-code, but that is what the VM might do > internally). > I guess the reorder is not on the java language level. It maybe on the byte code level. According to the JMM, will jvm reorder two reads if there is a write between them in one thread? Here is my test case, I have tested for 20 times, the message printed out are all same: public class HashCodeTest { final String s = "harmony"; class MyRunnable implements Runnable { private int value = -1; @Override public void run() { value = s.hashCode(); } public void print() { System.out.println(value); } } public static void main(String[] args) { final HashCodeTest test = new HashCodeTest(); MyRunnable run1 = test.new MyRunnable(); MyRunnable run2 = test.new MyRunnable(); Thread thread1 = new Thread(run1); Thread thread2 = new Thread(run2); thread1.start(); thread2.start(); run1.print(); run2.print(); } } > > One common solution is to assign the field but then return the temp-value > (in this case the variable "hash") and NOT the field itself. So that the > read can not be reordered. (This way might be even faster because it may be > one memory-read less) > Another solution would be to make hashCode volatile or to not cache the > hashCode, but these have a performance cost. > > I'll attach a patch I wrote. I could not get harmony to compile, so these > are untested. > BTW: This fix also fixes a more likely bug in BigInteger and BigDecimal: > Callers of hashCode might have seen half-constructed hashCodes there. > > (This bug was found via the entry LANG-481 see there for some more details) > > -- > This message is automatically generated by JIRA. > - > You can reply to this email to add a comment to the issue online. > > -- Yours sincerely, Charles Lee
