bug 8014477 Race in String.contentEquals
David Holmes
david.holmes at oracle.com
Tue May 14 11:17:44 UTC 2013
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 ...
I couldn't get the test to actually fail, but I can see that it could.
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