3

I am writing a static utility method. I know methods isEmpty() and isNew() are threadsafe. In the getTotal(...) method I am using StringBuilder as parameter along with String and int. StringBuilder is mutable. Is getTotal() threadsafe? If so explain why it is even though StringBuilder is mutable. I am not sure if getCharge() is thread safe because it's invoking the method getTotal(). Can someone tell if it is threadsafe or not?

public class NewStringUtils { public static boolean isEmpty(String s){ return (s == null || s.trim().length()==0); } public static boolean isNew(String code){ return( "1009".equals(code) || "1008".equals(code) ); } //StringBuilder is mutable -- is the below method threadsafe public static int getTotal(StringBuilder sb,String oldCode ,int a, int b){ //Is it Threadsafe or not .If so just bcz every thread invoking this method will have its own copy and other threads can't see its value ?? int k =0; if("1011".equals(oldCode) && "1021".equals(sb.toString()) { k = a+b; } return k; } // is the below method threadsafe public static int getCharge(String code,String oldCode,int charge1,int charge2){ int total =0; StringBuilder sb = new StringBuilder("1021"); if(!NewStringUtils.isEmpty(code)){ if(NewStringUtils.isNew(code)){ //here invoking a static method which has StringBuilder(Mutable) as a parameter total = NewStringUtils.getTotal(sb,oldCode,charge1,charge2); } } return total; } }
Improve this question

4 Answers 4

Reset to default
5

getTotal is not thread-safe because it is possible that two or more than two threads are passing same reference of StringBuilder to the getTotal method as arguement and modifying the StringBuilder before passing it ...
And your getCharge is completely Thread-safe because here each thread is making local copy of StringBuilder Object in there own stack. So not to worry about thread safety of getCharge.
Here is the short Demo which is demonstrating why getTotal is not ThreadSafe:

class ThreadSafe { public static int getTotal(StringBuilder sb,String oldCode ,int a, int b) { int k =0; try { System.out.println(Thread.currentThread().getName() + " have sb as " + sb); Thread.sleep(100);//Added intentionally to show why it is not thread safe. } catch (Exception ex) { System.out.println(ex); } if("1011".equals(oldCode) && "1021".equals(sb.toString())) { System.out.println(Thread.currentThread().getName()+" is within if loop");//Thread1 should be within this if block but it's not. k = a+b; } return k; } public static void main(String[] args) { final StringBuilder sBuilder = new StringBuilder(); Thread th1 = new Thread(new Runnable() { public void run() { sBuilder.append("1021"); getTotal(sBuilder,"1011",10,20); } },"Thread1"); Thread th2 = new Thread(new Runnable() { public void run() { sBuilder.append("22"); getTotal(sBuilder,"1011",10,20); } },"Thread2"); th1.start(); th2.start(); } }

At my system , the output is:

Thread1 have sb as 1021 Thread2 have sb as 102122

Assuming that Thread1 started before Thread2 which indeed happened here(as seen in output), If getTotal would have been thread-safe then , the statement Thread1 is within if loop must have been printed in output. But it is not . WHY? Because after Thread1(with sb = "1021") went to sleep in getTotal method the thread is preempted by Thread2. Thread2 appended 22 to the existing StringBuilder object sBuilder(i.e the new value now is 102122). Again when sleep method is called for Thread2, it is preempted by Thread1. Thread1 goes to if construct , but till now the content of sBuilder is changed to 102122. So if condition becomes false for Thread1. And it does'nt print that line Thread1 is within if loop which we were expecting if we consider getTotal to be thread-safe. Hence it proves that getTotal is not Thread-safe.

How can we make getTotal ThreadSafe?

  1. By passing String instead of StringBuilder.
Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

@KevinMangold: yeah I rephrased my sentence.
@sarath: About which method are you asking?
so will getTotal() be threadsafe if I always create a new StringBuffer object and pass that as argument
If you change getTotal() to accept 2 strings instead of a StringBuilder and String, it will be thread-safe. StringBuffer is also a thread-safe version of String Builder.
A helpful link that describes the correct way to write static classes:link Correct way to write static classes
|
3

getTotal() is thread-safe in terms of being re-entrant (i.e., it can be called by multiple threads simultaneously). In terms of doing its job correctly, however, getTotal() is not thread-safe if another thread modifies the "sb" argument. This conditional safety should be explicitly stated in the documentation for that method.

If getTotal() were private rather than public and only called by getCharge(), then it would be unconditionally thread-safe.

Improve this answer

2 Comments

"In terms of doing its job correctly, however, getTotal() is not thread-safe if another thread modifies the "sb" argument." By this do you mean to say the sb argument being modified by an other thread which invoked the same static getTotal() method by passing a different StringBuffer argument and modifying it.In which case the results can be unpredictable which is hard to replicate.In such a scenario the results of one thread will be receieved wrongly by another thread
To prevent getTotal() from doing its job correctly, the second thread only needs to modify the "sb" argument that the first thread is using: the second thread doesn't need to call getTotal(). This is because StringBuilder isn't thread-safe; consequently, simultaneous access by multiple threads results in undefined behavior: calling "sb.toString()" in getTotal() might see all, some, or none of the changes from the second thread -- there's no a priori way to tell.
1

The problem with getTotal is that another thread may change the StringBuilder parameter. But you already know that. StringBuilder is usually implemented as a class with two fields let's say int count and char[] value. The Java Memory Model does not guarantee that changes to these variables from one thread will be published in the same order to the other threads. That makes it even possible to have count bigger than value.length for some threads. In this case you may see an exception. That means it can create problems even if you don't care about the latest value of the StringBuilder. In another cases count may be observed as 0 (for example) even though it has a different value for the modifying thread. This will lead to unexpected results.

Improve this answer

Comments

1

The below is what Kevin wants to suggest. This is threadsafe:

//StringBuilder is mutable -- is the below method threadsafe public static int getTotal(String sb,String oldCode ,int a, int b){ StringBuilder sb1 = new StringBuilder(sb); //we build out a new StringBuilder each time int k =0; if("1011".equals(oldCode) && "1021".equals(sb1.toString()) { k = a+b; } return k; }
Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.