Race in String.contentEquals ( was Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks)

[Peter Levart]

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> 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/~plevart/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/~plevart/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
> -----
>