bug 8014477 Race in String.contentEquals
Peter Levart
peter.levart at gmail.com
Tue May 14 11:34:16 UTC 2013
On 05/14/2013 01:17 PM, David Holmes wrote:
> Thanks Peter this looks good to me.
>
> One minor thing please set the correct copyright year in the test, it
> should just be
>
> Copyright (c) 2013, Oracle ...
Ok, fixed:
http://cr.openjdk.java.net/~plevart/jdk8-tl/String.contentEquals/webrev.04/
>
> I couldn't get the test to actually fail, but I can see that it could.
Hm, do you have a multi-core chip? On my computer (i7 CPU) it fails
immediately and always. You could try varying the number of concurrent
threads and or the time to wait for exception to be thrown...
>
> David
>
> On 14/05/2013 9:04 PM, Peter Levart wrote:
>> Ok, here's the corrected fix:
>>
>> http://cr.openjdk.java.net/~plevart/jdk8-tl/String.contentEquals/webrev.03/
>>
>>
>> I also added a reproducer for the bug.
>>
>> Regards, peter
>>
>> On 05/14/2013 07:53 AM, Peter Levart wrote:
>>>
>>> Right, sb.length() should be used. I will correct that as soon as I
>>> get to the keyboard.
>>>
>>> Regards, Peter
>>>
>>> On May 14, 2013 7:22 AM, "David Holmes" <david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>> wrote:
>>>
>>> Thanks Peter! I've filed
>>>
>>> 8014477
>>> Race condition in String.contentEquals when comparing with
>>> StringBuffer
>>>
>>> On 14/05/2013 8:34 AM, Peter Levart wrote:
>>>
>>> On 05/14/2013 12:09 AM, Peter Levart wrote:
>>>
>>> I noticed a synchronization bug in String.contentEquals
>>> method. If
>>> called with a StringBuffer argument while concurrent
>>> thread is
>>> modifying the StringBuffer, the method can either throw
>>> ArrayIndexOutOfBoundsException or return true even though
>>> the content
>>> of the StringBuffer has never been the same as the
>>> String's.
>>>
>>> Here's a proposed patch:
>>>
>>> http://cr.openjdk.java.net/~plevart/jdk8-tl/String.contentEquals/webrev.01/
>>> <http://cr.openjdk.java.net/%7Eplevart/jdk8-tl/String.contentEquals/webrev.01/>
>>>
>>> Regards, Peter
>>>
>>>
>>> Or even better (with some code clean-up):
>>>
>>> http://cr.openjdk.java.net/~plevart/jdk8-tl/String.contentEquals/webrev.02/
>>> <http://cr.openjdk.java.net/%7Eplevart/jdk8-tl/String.contentEquals/webrev.02/>
>>>
>>>
>>> This part is not correct I believe:
>>>
>>> 1010 private boolean
>>> nonSyncContentEquals(AbstractStringBuilder sb) {
>>> 1011 char v1[] = value;
>>> 1012 char v2[] = sb.getValue();
>>> 1013 int n = v1.length;
>>> 1014 if (n != v2.length) {
>>> 1015 return false;
>>> 1016 }
>>>
>>> v2 can be larger than v1 if v2 is not being fully used (ie count <
>>> length).
>>>
>>> David
>>> -----
>>>
>>
More information about the core-libs-dev
mailing list