Hi Peter; I tried it on my i7 box and was able to get it to fail right away. I didn't try the previous test on this box so can't say whether it would have also failed.
Applying the String changes resolved the issue. I think this one is ready! Mike On May 15 2013, at 00:50 , 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.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 >>>>>> ----- >>>>>> >
