From 90ecddac74c1f1f7b9bb7a4b44b21ce27376a561 Mon Sep 17 00:00:00 2001 From: plevart Date: Fri, 17 May 2013 14:41:39 +0200 Subject: [PATCH] 8014477: (str) Race condition in String.contentEquals when comparing with StringBuffer Reviewed-by: alanb, mduigou, dholmes --- src/share/classes/java/lang/String.java | 23 ++-- .../lang/String/StringContentEqualsBug.java | 107 ++++++++++++++++++ 2 files changed, 119 insertions(+), 11 deletions(-) create mode 100644 test/java/lang/String/StringContentEqualsBug.java diff --git a/src/share/classes/java/lang/String.java b/src/share/classes/java/lang/String.java index c738a9eb6..748c8a4bb 100644 --- a/src/share/classes/java/lang/String.java +++ b/src/share/classes/java/lang/String.java @@ -1010,13 +1010,14 @@ public final class String private boolean nonSyncContentEquals(AbstractStringBuilder sb) { char v1[] = value; char v2[] = sb.getValue(); - int i = 0; - int n = value.length; - while (n-- != 0) { + int n = v1.length; + if (n != sb.length()) { + return false; + } + for (int i = 0; i < n; i++) { if (v1[i] != v2[i]) { return false; } - i++; } return true; } @@ -1038,8 +1039,6 @@ public final class String * @since 1.5 */ public boolean contentEquals(CharSequence cs) { - if (value.length != cs.length()) - return false; // Argument is a StringBuffer, StringBuilder if (cs instanceof AbstractStringBuilder) { if (cs instanceof StringBuffer) { @@ -1055,12 +1054,14 @@ public final class String return true; // Argument is a generic CharSequence char v1[] = value; - int i = 0; - int n = value.length; - while (n-- != 0) { - if (v1[i] != cs.charAt(i)) + int n = v1.length; + if (n != cs.length()) { + return false; + } + for (int i = 0; i < n; i++) { + if (v1[i] != cs.charAt(i)) { return false; - i++; + } } return true; } diff --git a/test/java/lang/String/StringContentEqualsBug.java b/test/java/lang/String/StringContentEqualsBug.java new file mode 100644 index 000000000..660d8d7ee --- /dev/null +++ b/test/java/lang/String/StringContentEqualsBug.java @@ -0,0 +1,107 @@ +/* + * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8014477 + * @summary test String.contentEquals(StringBuffer) + */ +public class StringContentEqualsBug { + + static abstract class Task extends Thread { + volatile StringBuffer sb; + volatile Exception exception; + + Task(StringBuffer sb) { + this.sb = sb; + } + + @Override + public void run() { + try { + StringBuffer sb; + while ((sb = this.sb) != null) { + doWith(sb); + } + } + catch (Exception e) { + exception = e; + } + } + + protected abstract void doWith(StringBuffer sb); + } + + static class Tester extends Task { + Tester(StringBuffer sb) { + super(sb); + } + + @Override + protected void doWith(StringBuffer sb) { + "AA".contentEquals(sb); + } + } + + static class Disturber extends Task { + Disturber(StringBuffer sb) { + super(sb); + } + + @Override + protected void doWith(StringBuffer sb) { + sb.setLength(0); + sb.trimToSize(); + sb.append("AA"); + } + } + + + public static void main(String[] args) throws Exception { + StringBuffer sb = new StringBuffer(); + Task[] tasks = new Task[3]; + (tasks[0] = new Tester(sb)).start(); + for (int i = 1; i < tasks.length; i++) { + (tasks[i] = new Disturber(sb)).start(); + } + + try + { + // wait at most 5 seconds for any of the threads to throw exception + for (int i = 0; i < 20; i++) { + for (Task task : tasks) { + if (task.exception != null) { + throw task.exception; + } + } + Thread.sleep(250L); + } + } + finally { + for (Task task : tasks) { + task.sb = null; + task.join(); + } + } + } +} -- GitLab