harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vijay Menon <...@google.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:09:48 GMT
Perhaps I'm missing some context, but I don't see any problem.  I don't
believe that this:

        if (hashCode == 0) {
            // calculate hash
            hashCode = hash;
        }
        return hashCode;

can ever return 0 (assuming hash is non-zero), regardless of memory fences.
 The JMM won't allow visible reordering in a single threaded program.

- Vijay

On Thu, Dec 10, 2009 at 7:36 PM, Regis <xu.regis@gmail.com> wrote:

> On 2009-12-11 4:30, Boris (JIRA) wrote:
>
>> 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).
>>
>> 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)
>>
>>
> It's interesting topic. In my understand, re-order would be a issue only if
> hashCode is object reference. Because when "retrun hasCode" the value of
> "hasCode" pushed to stack, if hasCode is primitive type I don't think vm
> would to such re-order - the return value would be always 0, that's
> incorrect.
>
> --
> Best Regards,
> Regis.
>

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