The synchronized
modifier is really a bad idea and should be avoided at all costs. I think it is commendable that Sun tried to make locking a little easier to acheive, but synchronized
just causes more trouble than it is worth.
The issue is that a synchronized
method is actually just syntax sugar for getting the lock on this
and holding it for the duration of the method. Thus, public synchronized void setInstanceVar()
would be equivalent to something like this:
public void setInstanceVar() {
synchronized(this) {
instanceVar++;
}
}
This is bad for two reasons:
- All
synchronized
methods within the same class use the exact same lock, which reduces throughput
- Anyone can get access to the lock, including members of other classes.
There is nothing to prevent me from doing something like this in another class:
MyClass c = new MyClass();
synchronized(c) {
...
}
Within that synchronized
block, I am holding the lock which is required by all synchronized
methods within MyClass
. This further reduces throughput and dramatically increases the chances of a deadlock.
A better approach is to have a dedicated lock
object and to use the synchronized(...)
block directly:
public class MyClass {
private int instanceVar;
private final Object lock = new Object(); // must be final!
public void setInstanceVar() {
synchronized(lock) {
instanceVar++;
}
}
}
Alternatively, you can use the java.util.concurrent.Lock
interface and the java.util.concurrent.locks.ReentrantLock
implementation to achieve basically the same result (in fact, it is the same on Java 6).