JDK-7100996 : (spec str) IndexOutOfBoundsException when using a StringBuffer from multiple threads
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.lang
  • Affected Version: 7
  • Priority: P4
  • Status: Closed
  • Resolution: Fixed
  • OS: linux
  • CPU: x86
  • Submitted: 2011-10-14
  • Updated: 2012-08-27
  • Resolved: 2012-08-27
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availabitlity Release.

To download the current JDK release, click here.
JDK 8
8 b47Fixed
Description
FULL PRODUCT VERSION :
java version "1.7.0"
Java(TM) SE Runtime Environment (build 1.7.0-b147)
Java HotSpot(TM) Server VM (build 21.0-b17, mixed mode)

The problem also exists in:
java version "1.6.0_27"
Java(TM) SE Runtime Environment (build 1.6.0_27-b07)
Java HotSpot(TM) Server VM (build 20.2-b06, mixed mode)

ADDITIONAL OS VERSION INFORMATION :
Linux 2.6.38-2-686-bigmem #1 SMP i686 GNU/Linux


EXTRA RELEVANT SYSTEM CONFIGURATION :
Intel(R) Core(TM)2 Duo CPU     E8500  @ 3.16GHz

A DESCRIPTION OF THE PROBLEM :
StringBuffer's insert() method is not thread-safe when the receiver object is passed as the source sequence. As a result, concurrently calling a StringBuffer instance can give behavior not possible during any sequential ordering of the same calls. This contradicts the thread safety guarantee specified in the API documentation.

The documentation says: "Whenever an operation occurs involving a source sequence (such as appending or inserting from a source sequence) this class synchronizes only on the string buffer performing the operation, not on the source. "
This sentence doesn't consider the case where the string buffer performing the operation is the same as the source sequence. In this case, the implementation performs parts of the insert operation (a call to s.length()) without any synchronization, leading to unexpected, non-thread-safe behavior.

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
See the test case below.

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
The code should run without any output.
ACTUAL -
The code throws an IndexOutOfBoundsException.

ERROR MESSAGES/STACK TRACES THAT OCCUR :
Exception in thread "Thread-6526" java.lang.IndexOutOfBoundsException: start 0, end 3, s.length() 2
	at java.lang.AbstractStringBuilder.insert(AbstractStringBuilder.java:1104)
	at java.lang.StringBuffer.insert(StringBuffer.java:478)
	at java.lang.StringBuffer.insert(StringBuffer.java:468)
	at newbugs.StringBufferTest$1.run(StringBufferTest.java:14)
	at java.lang.Thread.run(Thread.java:662)


REPRODUCIBILITY :
This bug can be reproduced occasionally.

---------- BEGIN SOURCE ----------
package newbugs;

public class StringBufferTest {
	public static void main(String[] args) throws Exception {
		new StringBufferTest().run();
	}
	
	private void run() throws Exception {
		for (int i = 0; i < 10000000; i++) {
			final StringBuffer sb = new StringBuffer("abc");
			
			Thread t1 = new Thread(new Runnable() {
				public void run() {
					sb.insert(1, sb);
				}
			});
			Thread t2 = new Thread(new Runnable() {
				public void run() {
					sb.deleteCharAt(0);
				}
			});
			t1.start();
			t2.start();
			try {
				t1.join();
				t2.join();
			} catch (InterruptedException ie) {}
		}
	}
}
---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Don't pass a string buffer to itself when using multiple threads.

Comments
EVALUATION http://hg.openjdk.java.net/hsx/hotspot-comp/jdk/rev/9e15068b6946
2012-08-14

EVALUATION Also from Brian: "Note that trying to do some sort of locking on the target as well as the source is a loser from the get-go; it almost certainly introduces risk of deadlock. Imagine if append() also locked its argument: SB append(CharSequence source) { synchronized(this) { synchronized(source) { ... } } } Now, if Thread A does sb1.append(sb2), and thread B does sb2.append(sb1), boom, inconsistent lock order. Oops. So, basically what we are saying is that SB manages its own state in a thread-safe manner, but the caller is responsible for ensuring that the source not be seen to change its state during the call. Which can be accomplished by the standard tricks -- don't share, don't mutate, or lock appropriately. "
2012-06-20

EVALUATION According to Brian -- "The implicit assumption of the spec cited -- and reasonably so -- is that the contents of source sequence must be consistently visible to the current thread for the duration of the operation. This could be achieved in many ways, such as thread-confinement, immutability, or if shared, the current thread holds a lock on it. StringBuffer is designed to handle shared StringBuffers, but not shared targets. This is a reasonable choice. As a workaround, the client could do this: synchronized(sb) { sb.append(sb); } This relies on the specified behavior, that the lock used to guard the StringBuffer state is the StringBuffer itself. This pattern does not necessarily work for all situations in which this sort of problem could arise -- the receiver class must commit to guarding its state with 'this'. Alternately, the client could do this: sb.append(sb.toString()) The poster's claim -- that StringBuffer does not conform to its own claims of thread-safety -- is overstated. The poster is imagining a definition of thread-safety that does not require the client to know anything about which state is mutable and which is not. (Imagine the user wrote a class that was identical to StringBuffer, and then passed one that was subject to concurrent mutation to StringBuffer.append. The same failure would happen; but the fault lies with passing state that might mutate during the call, not with StringBuffer.) It is conceivable that we could do a check for "if target==this" but I don't think it is worthwhile; the caller is violating the rules, and expecting StringBuffer to magically figure that out. " We'll improve the spec language to convey this info.
2012-06-20

EVALUATION Have asked Brian Goetz to evaluate this.
2012-06-20

WORK AROUND CUSTOMER SUBMITTED WORKAROUND : Don't pass a string buffer to itself when using multiple threads.
2012-06-05