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);
}
}
}
If you see Example 2 I am modifying 1st parameter. Now if you want to make this method really thread safe then one simple thing you can do. Either use non-mutable variables / objects or do not change / modify any method parameters.
Static variables are not thread safe. Instance variables do not require thread synchronization unless shared among threads. But, static variables are always shared by all the threads in the process. Hence, access to static variable is not thread safe.
It is well know that static methods with Immutable Objects as parameters are thread safe and Mutable Objects are not. If I have a utility method for some manipulation of java.
There is no rule that makes the code thread safe, the only thing you can do is make sure that your code will work no matter how many times is it being actively executed, each thread can be interrupted at any point, with each thread being in its own state/location, and this for each function (static or otherwise) that ...
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));
}
}
}
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
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With