I am trying to understand the difference between StringBuilder and StringBuffer. The goal of the below program is that 2 threads (Jack and Jill) compete to mutate a StringBuffer and StringBuilder values. If the original value is already modified then the thread will not modify that variable. Why is my StringBuffer variable not synchronized. Where as StringBuilder variable behaves as synchronized
My expected output is …
StringBuffer: Jack won StringBuilder: Jack wonJill won
Where as the actual output is the other way round.
public class StringBufferIsThreadSafe{
public static void main(String[] args) throws InterruptedException {
Hello hello = new Hello();
Thread jackThread = new Thread(new Jack(hello.strBuf, hello.strBuilder));
Thread jillThread = new Thread(new Jill(hello.strBuf, hello.strBuilder));
jackThread.start();
jillThread.start();
jackThread.join();
jillThread.join();
System.out.println("StringBuffer: "+hello.strBuf);
System.out.println("StringBuilder: "+hello.strBuilder);
}
}
class Jack implements Runnable{
private StringBuffer strBuf;
private StringBuilder strBuilder;
public Jack(StringBuffer strBuf, StringBuilder strBuilder) {
this.strBuf = strBuf;
this.strBuilder = strBuilder;
}
@Override
public void run() {
try {
Thread.sleep(3000);
if(this.strBuf.toString().equals("")) {
this.strBuf.append("Jack won");
}
if(this.strBuilder.toString().equals("")) {
this.strBuilder.append("Jack won");
}
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}
class Jill implements Runnable{
private StringBuffer strBuff;
private StringBuilder strBuilder;
public Jill(StringBuffer strBuff, StringBuilder strBuilder) {
this.strBuff = strBuff;
this.strBuilder = strBuilder;
}
@Override
public void run() {
try {
Thread.sleep(3000);
if(this.strBuff.toString().equals("")) {
this.strBuff.append("Jill won");
}
if(this.strBuilder.toString().equals("")) {
this.strBuilder.append("Jill won");
}
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}
class Hello {
StringBuffer strBuf = new StringBuffer();
StringBuilder strBuilder = new StringBuilder();
public Hello() {
this.strBuf.append("");
this.strBuilder.append("");
}
}
Advertisement
Answer
I think you misunderstand what ‘internally synchronized’ means.
‘internal synchronization’ isn’t voodoo magic. There is simply no way for the code in StringBuffder to affect your code. This becomes a problem when you write:
if (strBuffer.toString().equals("")) {
strBuffer.append("Jill won");
}
You get no guarantees here, specifically you seem to think that this entire operation is now somehow atomic (as in, after this, it would be impossible for strBuffer to contain, say, Jack wonJill won). But that is not the case. It is entirely possible that after this code, the contents are now Jack wonJill won.
‘internally synchronized’ merely means that with StringBuilder you can theoretically end up with JJaicllk wwonon, because the two append calls run simultaneously and make a right mess out of it all, whereas with StringBuffer that can’t happen – any single method call into a StringBuffer blocks any other method call into it.
In other words, the problem with that if block is that you have 2 method calls there (toString() in the if condition, then append in the body), and StringBuffer doesn’t make that work, and no implementation of StringBuffer could exist that makes this work.
There are 2 solutions to this problem:
[1] use locks explicitly
synchronized (strBuf) {
if (strBuf.toString().isEmpty()) strBuf.append("Jill won");
}
works great. In doing so, you don’t actually need the internal synchronization aspect anymore. You’ve already taken care of it.
[2] make single methods that do the entire job
This would be for the authors of StringBuffer, you can’t fix this. But, they could have had a method named, e.g. appendIfEmpty, and you could then write strBuffer.appendIfEmpty("Jill won"). Now it is a single method and the StringBuffer code can take care of business, and ensure it’s atomic/synchronized.
Check out the javadoc of HashMap which has a bunch of these ‘atomic’ methods, such as putIfAbsent.
Closing thoughts on internal sync, and StringBuffer/StringBuilder
Without synchronization, you simply get no guarantees at all. The code can do essentially anything. Ending up with JJaicllk wwoonn in the buffer isn’t even the worst that could happen: Exceptions can happen too, as can a StringBuilder in an invalid state (one that reports that it is, say, size 9 but when printed is larger, or which seems fine but when you call toString() on it, then exceptions occur, etcetera). In this sense StringBuffer is ‘safer’, except this isn’t useful because code that does arbitrary stuff isn’t useful. Just about every API in existence, be it Map, List, or StringBuilder, lends itself to ‘compound’ operations where you need to invoke at least 2 methods to do the job, at which point internal synchronization is completely useless.
That means internal synchronization (which is what StringBuffer, Vector, Hashtable, and Collections.synchronizedList/Map/Set give you) is almost entirely pointless, and that is why the JVM has started steering away from it, making ArrayList, HashSet and co no longer internally synchronized and making StringBuilder.
For what its worth, since the introduction of e.g. StringBuilder, VMs have been improved. Nowadays, locking on an object when no other thread ever does is essentially free, or at least the performance impact is infinitesemal. Thus, using StringBuffer where a StringBuilder would have been just as good is no longer a significant performance penalty.
Thus, the reason to forget about StringBuffer and only use StringBuilder is as follows:
- Consider StringBuffer obsolete because it is. The java core libs do not get marked as deprecated or removed even if obsolete. See Vector, Hashtable, java.util.Date, etc. They just don’t – the openjdk dev team finds ‘code that was written 10 years ago should preferably still compile, and do the same thing, when compiling on today’s javac’ so important, that they don’t do this. So don’t make the mistake of: “Ooh, StringBuffer is a thing that exists and isn’t explicitly marked off as obsolete, therefore there must be some point to it”. No, there is not. Do not ever use it.
- Internal synchronization is almost entirely useless, see your very code snippet where it isn’t doing what you expected / wanted.
- The java community as a whole just uses StringBuilder, everywhere. When there are 2 different ways to do the same thing and they seem identical in effort required, performance, and code style (as it is here), you should do what the community likes to do. It lowers learning curves for the team, avoids style differences that are just annoying (where you wrote one way, your buddy writes another way, and every time one of you reads the other’s code it looks weird for no reason), and walking the well trodden path is how you get to code that has less issues with bugs in other libraries, works with more libraries, and gets the performance benefits, because the OpenJDK engineers that work on making java fast focus on common code patterns. Call it the When in rome, act like a roman rule.