OK. With that in mind, here's an update where I leave contentEquals(
StringBuffer ) in place (adding a clarifying spec), and defer the
synchronization as I had it to the contentEquals( CharSequence ) method:
diff --git a/src/share/classes/java/lang/String.java
b/src/share/classes/java/lang/String.java
--- a/src/share/classes/java/lang/String.java
+++ b/src/share/classes/java/lang/String.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1994, 2010, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 1994, 2012, 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
@@ -983,11 +983,11 @@
}
return false;
}
-
- /**
+ /**
* Compares this string to the specified {@code StringBuffer}.
The result
* is {@code true} if and only if this {@code String} represents
the same
- * sequence of characters as the specified {@code StringBuffer}.
+ * sequence of characters as the specified {@code StringBuffer}.
This method
+ * synchronizes on the {@code StringBuffer}.
*
* @param sb
* The {@code StringBuffer} to compare this {@code String}
against
@@ -999,15 +999,29 @@
* @since 1.4
*/
public boolean contentEquals(StringBuffer sb) {
- synchronized (sb) {
- return contentEquals((CharSequence) sb);
+ return contentEquals((CharSequence) sb);
+ }
+
+ private boolean nonSyncContentEquals(AbstractStringBuilder sb) {
+ char v1[] = value;
+ char v2[] = sb.getValue();
+ int i = 0;
+ int n = value.length;
+ while (n-- != 0) {
+ if (v1[i] != v2[i]) {
+ return false;
+ }
+ i++;
}
+ return true;
}
/**
- * Compares this string to the specified {@code CharSequence}. The
result
- * is {@code true} if and only if this {@code String} represents
the same
- * sequence of char values as the specified sequence.
+ * Compares this string to the specified {@code CharSequence}. The
+ * result is {@code true} if and only if this {@code String}
represents the
+ * same sequence of char values as the specified sequence. Note
that if the
+ * {@code CharSequence} is a {@code StringBuffer} then the method
+ * synchronizes on it.
*
* @param cs
* The sequence to compare this {@code String} against
@@ -1023,16 +1037,13 @@
return false;
// Argument is a StringBuffer, StringBuilder
if (cs instanceof AbstractStringBuilder) {
- char v1[] = value;
- char v2[] = ((AbstractStringBuilder) cs).getValue();
- int i = 0;
- int n = value.length;
- while (n-- != 0) {
- if (v1[i] != v2[i])
- return false;
- i++;
+ if (cs instanceof StringBuffer) {
+ synchronized(cs) {
+ return nonSyncContentEquals((AbstractStringBuilder)cs);
+ }
+ } else {
+ return nonSyncContentEquals((AbstractStringBuilder)cs);
}
- return true;
}
// Argument is a String
if (cs.equals(this))
On 07/26/2012 04:28 PM, Mike Duigou wrote:
This would appear to be source compatible but not binary compatible. I don't
believe we can remove the contentsEqual(StringBuffer) overload. Code compiled
against the existing interface would fail to find the CharSequence interface at
runtime and fail.
I believe it would be reasonable to add locking on StringBuffer into the
contentsEqual(CharSequence) overload. This would make the behaviour consistent
between the two invocations which is important because it will not always be
possible for developers to tell which overload is being selected.
Mike
