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.hol...@oracle.com
<mailto:david.hol...@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
    -----


Reply via email to