I have a friend who said that all static methods should be synchronized in the context of a Java web application. Is that true? I have read many other stack overflow pages regarding this. What I have come to believe is that you only need to synchronize if you have:
- Multiple Threads (As in a Sevlet Container with a thread pool)
- Single ClassLoader
- Shared data between threads, whether it is Session data or static member data.
- Shared data must be mutable. Read only data is ok to share.
Based on this I think that static members should be synchronized, but not static methods.
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
public class ThreadTest {
static String staticString = "";
// This static method is safe b/c it only uses local data.
// It does not use any shared mutable data.
// It even uses a string builder.
static String safeStaticMethod(String in) {
// This also proves that StringBuilder is safe
// When used locally by a thread.
StringBuilder sb = new StringBuilder();
sb.append("Hello: ");
sb.append(in);
return sb.toString();
}
// This static method is not safe b/c it updates and reads
// shared mutable data among threads.
// Adding synchronized will make this safe.
static String unsafeStaticMethod(String in) {
staticString = in;
StringBuffer sb = new StringBuffer();
sb.append("Hello: ");
sb.append(staticString);
return sb.toString();
}
public static void main(String[] args) {
ThreadTest test = new ThreadTest();
test.staticMethodWithLocalData();
test.staticMethodWithStaticData();
}
public void staticMethodWithLocalData() {
ExecutorService executor = Executors.newFixedThreadPool(2);
final int iterations = 100000;
executor.submit(new Runnable() {
@Override
public void run() {
for (int index = 0; index < iterations; ++index) {
if (!safeStaticMethod("Thread1").equals("Hello: Thread1")) {
System.out.println("safeStaticMethod at " + index);
}
}
}
});
executor.submit(new Runnable() {
@Override
public void run() {
for (int index = 0; index < iterations; ++index) {
if (!safeStaticMethod("Thread2").equals("Hello: Thread2")) {
System.out.println("safeStaticMethod at " + index);
}
}
}
});
}
public void staticMethodWithStaticData() {
ExecutorService executor = Executors.newFixedThreadPool(2);
final int iterations = 100000;
executor.submit(new Runnable() {
@Override
public void run() {
for (int index = 0; index < iterations; ++index) {
if (!unsafeStaticMethod("Thread1").equals("Hello: Thread1")) {
System.out.println("unsafeStaticMethod at " + index);
}
}
}
});
executor.submit(new Runnable() {
@Override
public void run() {
for (int index = 0; index < iterations; ++index) {
if (!unsafeStaticMethod("Thread2").equals("Hello: Thread2")) {
System.out.println("unsafeStaticMethod at " + index);
}
}
}
});
}
}
Does this code prove the point?
EDIT: This is only some throwaway code I hacked up to prove the point.
synchronizedin random places. – PrepucesafeStaticMethodas written would still be safe using aStringBuffer, as the buffer is not shared between threads. it is local to that particular method invocation. – Anorthitesynchronizedmay be insufficient - it may still be incorrect if two related fields are accessed in the wrong order or if a single change should affect both of them consistently. (Recall the classical example of transferring money between bank accounts.) There's also different ways of sharing data in a thread-safe way that may be more appropriate. (Like retrieving consistent read-only snapshots instead of accessing it directly, or using lock-free iterators that survive concurrent modification.) – Grata