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 <[email protected]> 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.
>
