harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Charles Lee <littlee1...@gmail.com>
Subject Re: [jira] Created: (HARMONY-6404) possible data-reordering in some hashCode-methods (e.g. String or URL)
Date Fri, 11 Dec 2009 04:42:36 GMT
On Fri, Dec 11, 2009 at 4:30 AM, Boris (JIRA) <jira@apache.org> 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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message