harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Boris (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HARMONY-6404) possible data-reordering in some hashCode-methods (e.g. String or URL)
Date Fri, 11 Dec 2009 06:52:18 GMT

    [ https://issues.apache.org/jira/browse/HARMONY-6404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12789144#action_12789144
] 

Boris commented on HARMONY-6404:
--------------------------------

Hi Nathan,

sorry, I might have been unhelpful to link to the LANG-bug because not everything there applies
here.

I think your right that this only applies for classes that are written to be thread-safe.
I was a bit hasty there and searched for every occurrence of the pattern without re-thinking
if it's really the same issue as in String.

> "The value is lazily initialized, but the calculation is done _on the cached value."

Sorry, this is a part that does not apply to harmony's String-implementation (only to BigInteger
and BigDecimal). The calculation is done right so that no other thread could see a partly
initialized value. The method can only ever return 0 or the actual hash.

The issue is hard to describe (it took my a while to understand it myself). Do you have a
copy of "Java Concurrency in practice"? It's described there in chapter 16.1.2 and 16.1.3.
There is an explanation online by a google-engineer at http://jeremymanson.blogspot.com/2008/12/benign-data-races-in-java.html
I can try to describe it more detailed this evening (UTC) after work.

> 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
>         Attachments: harmony_read-reorder.patch
>
>
> 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)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message