bug 8014477 Race in String.contentEquals

[David Holmes]

Okay mea culpa - I was testing on the wrong JDK. I wrongly assumed this 
would impact 7 as well but it doesn't as the bug was introduced with the 
changes in 6914123.

Sorry about that.

Good to go. Has anyone offered to sponsor this yet?

David
-----

On 15/05/2013 5:50 PM, Peter Levart wrote:
> Mike, David,
>
> Could you try this variant of the test:
>
> http://cr.openjdk.java.net/~plevart/jdk8-tl/String.contentEquals/webrev.05/
>
>
> I tried it on 3 different machines (4-core i7 Linux, 8-core sparc T-2
> Solaris, 4-core amd64 Solaris) x 2 different JDK8 JVMs (64bit and 32bit)
> and it fails immediately on all 6 of them.
>
> Regards, Peter
>
> On 05/14/2013 05:39 PM, Mike Duigou wrote:
>> Good to see the test added. I was also unable to reproduce the failure
>> on my Core 2 Duo Mac laptop but the test and fix match up.
>>
>> Mike
>>
>>
>> On May 14 2013, at 04:34 , Peter Levart wrote:
>>
>>> 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
>>>>>>     -----
>>>>>>
>