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