Terry
Terry

Reputation: 875

How to make the static method thread safe in JAVA?

I am creating a web application and meet a thread safe problem. After reading several similar questions, I am still confusing about my case. I am using the java spring framework to setup the REST web service. All the request (JSON of Person Object) will be pass to the checkIfGoodName function, like Checker.checkIfGoodName(person). They are all static method call. I am wondering that, is this function, Checker.checkIfGoodName THREAD SAFE ? If no, how to modify the code ? I have code as below:

Checker.java

public class Checker {

    public static void checkIfGoodName(Person person){

        checkingName(Person person);

    }

    private static void checkingName(Person person){

        if(person.getName()==null){
            PersonUtils.addErrorMessage(Person person, new String("Name is empty"));
        }

    }

}

PersonUtils.java

public class PersonUtils {

     public static void addErrorMessage(Person person, String errorMessage){

         List<Message> msg = person.getMessageList();
         if(msg!=null){
             msg.add(buildMessage(errorMessage));
         }
     }

     public static void buildMessage(String errorMessage){
         if(errorMessage != null){
             Message msg = new Message();
             msg.setMsg(errorMessage);
         } 
     }

}

Upvotes: 6

Views: 16262

Answers (2)

Solomon Slow
Solomon Slow

Reputation: 27210

Don't think about making methods thread safe. Thread safety is about protecting the integrity of data. More specifically, it is about preventing threads from accessing data when some other thread is in the process of changing the data.

Your PersonUtils.addErrorMessage(person, message) method modifies List instances belonging to Person instances. Access to the lists should be synchronized If the same list can be modified by two different threads, or if it can be modified by one thread and accessed by other threads.

Adding an item to a list takes several steps, and the list almost certainly will appear to be in an illegal state if thread A is able to see it at a point when thread B has performed some, but not all of the steps. It's worse if two threads attempt to modify the list at the same time: That could leave the list in a permanent, illegal state.

You would still need synchronization even if threads that operate on the same Person instance don't actually do it at the same time. The reason is, without synchronization, the computer hardware and the operating system do not guarantee that changes made in memory by one thread will immediately be visible to other threads. But, synchronized comes to your rescue:

Whatever changes thread A makes before it leaves a synchronized(foo) block will be visible to thread B after thread B enters a synchronized(foo) block.

The simplest thing for you to do, again if different threads access the same Person instance, would be to synchronize on the Person object in your addErrorMessage(...) method:

  public static void addErrorMessage(Person person, String errorMessage){
      synchronized(person) {
          List<Message> msg = person.getMessageList();
          if(msg!=null){
             msg.add(buildMessage(errorMessage));
          }
      }
  }

Upvotes: 7

Jigar Joshi
Jigar Joshi

Reputation: 240996

I don't see PersonUtils has a state, however it could be the case where you have same Person instance being passed twice simultaneously so better to make a lock on Person instance,

assuming you actually have message list associated with instance and not a static property

also it is not good to associate error message with Model itself, try exploring some standard practices with the framework you mentioned

Upvotes: 2

Related Questions