You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Boris (JIRA)" <ji...@apache.org> on 2009/01/23 20:49:59 UTC
[jira] Created: (LANG-481) Possible race-conditions in hashCode of
the range classes
Possible race-conditions in hashCode of the range classes
---------------------------------------------------------
Key: LANG-481
URL: https://issues.apache.org/jira/browse/LANG-481
Project: Commons Lang
Issue Type: Bug
Affects Versions: 2.4
Reporter: Boris
Priority: Minor
The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
An unlucky sequence of Code could be something like
{{T1: if (hashCode == 0)
T1: hashCode = 17;
T2: if (hashCode == 0)
T2: return hashCode;
T1: hashCode = 37 * hashCode...........}}
where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
Affected classes are
org.apache.commons.lang.math.DoubleRange
org.apache.commons.lang.math.FloatRange
org.apache.commons.lang.math.IntRange
org.apache.commons.lang.math.LongRange
org.apache.commons.lang.math.NumberRange
org.apache.commons.lang.math.Range
Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Reopened: (LANG-481) Possible race-conditions in hashCode of
the range classes
Posted by "Boris (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Boris reopened LANG-481:
------------------------
I'm sorry, I just dicovered that there is another data-race in that methods that I overlooked before.
In short: Due to the Java Memory Model and allowed reorderings it is possible that the hashCode() can return 0 instead of the correct value and thus breaking HashMaps etc. in this case.
Please see [this post|http://jeremymanson.blogspot.com/2008/12/benign-data-races-in-java.html] for details on this issue and String.hashCode() in the java-sources as a reference how to do it right. He explained it better than I can.
I'll attach a patch against trunk to fix this.
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
> Attachments: LANG-481.patch
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-481) Possible race-conditions in hashCode
of the range classes
Posted by "Henri Yandell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742243#action_12742243 ]
Henri Yandell commented on LANG-481:
------------------------------------
What's the open topic on this issue? Which classes have the problem?
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-481) Possible race-conditions in hashCode
of the range classes
Posted by "James Carman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666915#action_12666915 ]
James Carman commented on LANG-481:
-----------------------------------
If this is indeed an issue, there are other cached values also that have the same problem (toString, maxObject, minObject) in LongRange This JIRA issue should encompass those, also.
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-481) Possible race-conditions in hashCode
of the range classes
Posted by "Boris (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666954#action_12666954 ]
Boris commented on LANG-481:
----------------------------
Sebb, you're right. What I said is not true for arbitrary Objects. Sorry.
I think it's only true for immutable Objects (that use final fields). And only for atomic assignment operations (assignments of primitive longs and doubles on a non-volatile field is AFAIK not atomic, but object-reference assignment is indeed). But I guess there are better explanations out there then mine :-/
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-481) Possible race-conditions in hashCode
of the range classes
Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12789211#action_12789211 ]
Tim Ellison commented on LANG-481:
----------------------------------
Boris,
Harmony's String#hashCode() is fine, see the discussions on the dev list
http://markmail.org/thread/3ckdhgonbh7xtulg
http://markmail.org/thread/3ckdhgonbh7xtulg
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
> Attachments: LANG-481-reordering_datarace.patch, LANG-481.patch
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (LANG-481) Possible race-conditions in hashCode of
the range classes
Posted by "Henri Yandell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Yandell updated LANG-481:
-------------------------------
Fix Version/s: 3.0
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-481) Possible race-conditions in hashCode
of the range classes
Posted by "Boris (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742521#action_12742521 ]
Boris commented on LANG-481:
----------------------------
Henri, as far as I can see all the classes I mentioned in my report have the problem. All other classes I looked at not because they don't use the member-variable as a temp to calculate the value.
There is one exception from my first list (at least in current trunk): org.apache.commons.lang.math.Range looks safe
So it comes down to this classes:
org.apache.commons.lang.math.DoubleRange
org.apache.commons.lang.math.FloatRange
org.apache.commons.lang.math.IntRange
org.apache.commons.lang.math.LongRange
org.apache.commons.lang.math.NumberRange
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (LANG-481) Possible race-conditions in hashCode of
the range classes
Posted by "Henri Yandell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Yandell closed LANG-481.
------------------------------
Resolution: Fixed
Patch applied - thanks Boris :)
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
> Attachments: LANG-481.patch
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-481) Possible race-conditions in hashCode
of the range classes
Posted by "Boris (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12788868#action_12788868 ]
Boris commented on LANG-481:
----------------------------
Sorry for the delay of my answer.
The hashCode() of Harmony has the same reorder-bug as described above. It loads from a non-volatile field without synchronisation, so the code can be reordered and 0 could be wrongly returned as a hashCode.
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
> Attachments: LANG-481-reordering_datarace.patch, LANG-481.patch
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (LANG-481) Possible race-conditions in hashCode of
the range classes
Posted by "Boris (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Boris updated LANG-481:
-----------------------
Description:
The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
An unlucky sequence of Code could be something like
T1: if (hashCode == 0)
T1: hashCode = 17;
T2: if (hashCode == 0)
T2: return hashCode;
T1: hashCode = 37 * hashCode...........
where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
Affected classes are
org.apache.commons.lang.math.DoubleRange
org.apache.commons.lang.math.FloatRange
org.apache.commons.lang.math.IntRange
org.apache.commons.lang.math.LongRange
org.apache.commons.lang.math.NumberRange
org.apache.commons.lang.math.Range
Possible fix: calculate the hash on a temporary variable and finally assign it to the member
was:
The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
An unlucky sequence of Code could be something like
{{T1: if (hashCode == 0)
T1: hashCode = 17;
T2: if (hashCode == 0)
T2: return hashCode;
T1: hashCode = 37 * hashCode...........}}
where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
Affected classes are
org.apache.commons.lang.math.DoubleRange
org.apache.commons.lang.math.FloatRange
org.apache.commons.lang.math.IntRange
org.apache.commons.lang.math.LongRange
org.apache.commons.lang.math.NumberRange
org.apache.commons.lang.math.Range
Possible fix: calculate the hash on a temporary variable and finally assign it to the member
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0)
> T1: hashCode = 17;
> T2: if (hashCode == 0)
> T2: return hashCode;
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (LANG-481) Possible race-conditions in hashCode of
the range classes
Posted by "Boris (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Boris updated LANG-481:
-----------------------
Description:
The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
An unlucky sequence of Code could be something like
T1: if (hashCode == 0) // true
T1: hashCode = 17;
T2: if (hashCode == 0) // now false because hashCode was already set to 17
T2: return hashCode; // return 17
T1: hashCode = 37 * hashCode...........
where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
Affected classes are
org.apache.commons.lang.math.DoubleRange
org.apache.commons.lang.math.FloatRange
org.apache.commons.lang.math.IntRange
org.apache.commons.lang.math.LongRange
org.apache.commons.lang.math.NumberRange
org.apache.commons.lang.math.Range
Possible fix: calculate the hash on a temporary variable and finally assign it to the member
was:
The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
An unlucky sequence of Code could be something like
T1: if (hashCode == 0)
T1: hashCode = 17;
T2: if (hashCode == 0)
T2: return hashCode;
T1: hashCode = 37 * hashCode...........
where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
Affected classes are
org.apache.commons.lang.math.DoubleRange
org.apache.commons.lang.math.FloatRange
org.apache.commons.lang.math.IntRange
org.apache.commons.lang.math.LongRange
org.apache.commons.lang.math.NumberRange
org.apache.commons.lang.math.Range
Possible fix: calculate the hash on a temporary variable and finally assign it to the member
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-481) Possible race-conditions in hashCode
of the range classes
Posted by "Boris (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666941#action_12666941 ]
Boris commented on LANG-481:
----------------------------
toString, maxObject and minObject do not have *this* kind of race where you could end up with *wrong* data.
Of course there is race too, but in the worst case, you may only end up calculating the value every time the method is called and the cached value is not visible to your Thread. Of course you get no caching, but you'll never get wrong data.
So this worst case is pretty much like not caching it at all, but with some luck you get the value cached - at least once for each thread.
Looking at other classes this seems to be a common technique in commons-lang.
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-481) Possible race-conditions in hashCode
of the range classes
Posted by "Henri Yandell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12777861#action_12777861 ]
Henri Yandell commented on LANG-481:
------------------------------------
*Range classes deleted. Need to figure out a fix for the hashCode data-race reordering. Seems that we need to cache the value or work it out in the constructor.
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
> Attachments: LANG-481-reordering_datarace.patch, LANG-481.patch
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (LANG-481) Possible race-conditions in hashCode of
the range classes
Posted by "Boris (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Boris updated LANG-481:
-----------------------
Attachment: LANG-481.patch
Possible patch to this issue.
This uses the same technique used elsewhere in commons-lang. Using a non-volatile field to cache the value (in the worst case it may be calculated every time, in the best case only once) and calculating the value on a temp variable so that there is never an inconsistent value stored in the field.
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
> Attachments: LANG-481.patch
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-481) Possible race-conditions in hashCode
of the range classes
Posted by "Henri Yandell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12774801#action_12774801 ]
Henri Yandell commented on LANG-481:
------------------------------------
Currently considering deleting the *Range classes and replacing with a single Range class. Presumably it doesn't resolve this issue - need to check post LANG-551.
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
> Attachments: LANG-481-reordering_datarace.patch, LANG-481.patch
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-481) Possible race-conditions in hashCode
of the range classes
Posted by "Sebb (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666948#action_12666948 ]
Sebb commented on LANG-481:
---------------------------
I'm not sure that would be true for an arbitrary object: if an object is written in one thread, another thread may see a partially updated object unless the two threads synchronise on the same lock.
However, the objects in this case (Long) are final which I think means that their contents will be published correctly - i.e. other threads will see the updated object in full or not at all.
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-481) Possible race-conditions in hashCode
of the range classes
Posted by "Boris (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12777947#action_12777947 ]
Boris commented on LANG-481:
----------------------------
Henri,
The current hashCode() is fine of course :) And as optimum long as speed is no concern it's also the best. Just my 2ct on this topic:
Caching or pre-calculating the value will be a trade-off on how you want to use the class. If you pre-calculate the value and never use hashCode() the "effort" was pointless. If you cache it, it will MAY be re-calculated on every call (but this is rather unlikely).
I think the cached-approach is the best (if it's done right of course :)), but the constructor-approach is easier to get right.
But as one can use String.hashCode() as a boilerplate it should be easy to implement optimistic caching right.
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
> Attachments: LANG-481-reordering_datarace.patch, LANG-481.patch
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (LANG-481) Possible race-conditions in hashCode of
the range classes
Posted by "Boris (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Boris updated LANG-481:
-----------------------
Attachment: LANG-481-reordering_datarace.patch
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
> Attachments: LANG-481-reordering_datarace.patch, LANG-481.patch
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (LANG-481) Possible race-conditions in hashCode of
the range classes
Posted by "Henri Yandell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Yandell closed LANG-481.
------------------------------
Resolution: Fixed
Closing out with Boris' 2nd patch effectively applied to the new Range class while adding hashCode caching (r899897).
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Components: lang.*
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
> Attachments: LANG-481-reordering_datarace.patch, LANG-481.patch
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-481) Possible race-conditions in hashCode
of the range classes
Posted by "Henri Yandell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LANG-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12777994#action_12777994 ]
Henri Yandell commented on LANG-481:
------------------------------------
Link to license useable String.java:
http://svn.apache.org/repos/asf/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/String.java
> Possible race-conditions in hashCode of the range classes
> ---------------------------------------------------------
>
> Key: LANG-481
> URL: https://issues.apache.org/jira/browse/LANG-481
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: Boris
> Priority: Minor
> Fix For: 3.0
>
> Attachments: LANG-481-reordering_datarace.patch, LANG-481.patch
>
>
> The hashCode() methods of the range classes look very suspicious to me. The value is lazily initialized, but the calculation is done _on the cached value. With some unlucky timing a caller may get an incomplete hash.
> An unlucky sequence of Code could be something like
> T1: if (hashCode == 0) // true
> T1: hashCode = 17;
> T2: if (hashCode == 0) // now false because hashCode was already set to 17
> T2: return hashCode; // return 17
> T1: hashCode = 37 * hashCode...........
> where T1 and T2 are different threads accessing the method in parallel and T2 gets the wrong hash "17".
> Affected classes are
> org.apache.commons.lang.math.DoubleRange
> org.apache.commons.lang.math.FloatRange
> org.apache.commons.lang.math.IntRange
> org.apache.commons.lang.math.LongRange
> org.apache.commons.lang.math.NumberRange
> org.apache.commons.lang.math.Range
> Possible fix: calculate the hash on a temporary variable and finally assign it to the member
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.