Avoid synchronized(this) in Java?

Question

Whenever a question pops up on SO about Java synchronization, some people are very eager to point out that synchronized(this) should be avoided. Instead, they claim, a lock on a private reference is to be preferred.

Some of the given reasons are:

• some evil code may steal your lock (very popular this one, also has an "accidentally" variant)
• all synchronized methods within the same class use the exact same lock, which reduces throughput
• you are (unnecessarily) exposing too much information

Other people, including me, argue that synchronized(this) is an idiom that is used a lot (also in Java libraries), is safe and well understood. It should not be avoided because you have a bug and you don't have a clue of what is going on in your multithreaded program. In other words: if it is applicable, then use it.

I am interested in seeing some real-world examples (no foobar stuff) where avoiding a lock on this is preferable when synchronized(this) would also do the job.

Therefore: should you always avoid synchronized(this) and replace it with a lock on a private reference?

---

Some further info (updated as answers are given):

• we are talking about instance synchronization
• both implicit (synchronized methods) and explicit form of synchronized(this) are considered
• if you quote Bloch or other authorities on the subject, don't leave out the parts you don't like (e.g. Effective Java, item on Thread Safety: Typically it is the lock on the instance itself, but there are exceptions.)
• if you need granularity in your locking other than synchronized(this) provides, then synchronized(this) is not applicable so that's not the issue

Answer 1

The reason not to synchronize on this is that sometimes you need more than one lock (the second lock often gets removed after some additional thinking, but you still need it in the intermediate state). If you lock on this, you always have to remember which one of the two locks is this; if you lock on a private Object, the variable name tells you that.

From the reader's viewpoint, if you see locking on this, you always have to answer the two questions:

1. what kind of access is protected by this?
2. is one lock really enough, didn't someone introduce a bug?

An example:

class BadObject {
    private Something mStuff;
    synchronized setStuff(Something stuff) {
        mStuff = stuff;
    }
    synchronized getStuff(Something stuff) {
        return mStuff;
    }
    private MyListener myListener = new MyListener() {
        public void onMyEvent(...) {
            setStuff(...);
        }
    }
    synchronized void longOperation(MyListener l) {
        ...
        l.onMyEvent(...);
        ...
    }
}

If two threads begin longOperation() on two different instances of BadObject, they acquire their locks; when it's time to invoke l.onMyEvent(...), we have a deadlock because neither of the threads may acquire the other object's lock.

In this example we may eliminate the deadlock by using two locks, one for short operations and one for long ones.

Answer 2

It depends on the task you want to do, but I wouldn't use it. Also, check if the thread-save-ness you want to accompish couldn't be done by synchronize(this) in the first place? There are also some nice locks in the API that might help you :)

Answer 3

A good example for use synchronized(this).

// add listener
public final synchronized void addListener(IListener l) {listeners.add(l);}
// remove listener
public final synchronized void removeListener(IListener l) {listeners.remove(l);}
// routine that raise events
public void run() {
   // some code here...
   Set ls;
   synchronized(this) {
      ls = listeners.clone();
   }
   for (IListener l : ls) { l.processEvent(event); }
   // some code here...
}

As you can see here, we use synchronize on this to easy cooperate of lengthly (possibly infinite loop of run method) with some synchronized methods there.

Of course it can be very easily rewritten with using synchronized on private field. But sometimes, when we already have some design with synchronized methods (i.e. legacy class, we derive from, synchronized(this) can be the only solution).

Answer 4

1. Make your data immutable if it is possible ( final variables)
2. If you can't avoid mutation of shared data across multiple threads, use high level programming constructs [e.g. granular Lock API ]

A Lock provides exclusive access to a shared resource: only one thread at a time can acquire the lock and all access to the shared resource requires that the lock be acquired first.

Sample code to use ReentrantLock which implements Lock interface

 class X {
   private final ReentrantLock lock = new ReentrantLock();
   // ...

   public void m() {
     lock.lock();  // block until condition holds
     try {
       // ... method body
     } finally {
       lock.unlock()
     }
   }
 }

Advantages of Lock over Synchronized(this)

1. The use of synchronized methods or statements forces all lock acquisition and release to occur in a block-structured way.

2. Lock implementations provide additional functionality over the use of synchronized methods and statements by providing

   1. A non-blocking attempt to acquire a lock (tryLock())
   2. An attempt to acquire the lock that can be interrupted (lockInterruptibly())
   3. An attempt to acquire the lock that can timeout (tryLock(long, TimeUnit)).

3. A Lock class can also provide behavior and semantics that is quite different from that of the implicit monitor lock, such as

   1. guaranteed ordering
   2. non-re entrant usage
   3. Deadlock detection

Have a look at this SE question regarding various type of Locks:

Synchronization vs Lock

You can achieve thread safety by using advanced concurrency API instead of Synchronied blocks. This documentation page provides good programming constructs to achieve thread safety.

Lock Objects support locking idioms that simplify many concurrent applications.

Executors define a high-level API for launching and managing threads. Executor implementations provided by java.util.concurrent provide thread pool management suitable for large-scale applications.

Concurrent Collections make it easier to manage large collections of data, and can greatly reduce the need for synchronization.

Atomic Variables have features that minimize synchronization and help avoid memory consistency errors.

ThreadLocalRandom (in JDK 7) provides efficient generation of pseudorandom numbers from multiple threads.

Refer to java.util.concurrent and java.util.concurrent.atomic packages too for other programming constructs.

Answer 5

I only want to mention a possible solution for unique private references in atomic parts of code without dependencies. You can use a static Hashmap with locks and a simple static method named atomic() that creates required references automatically using stack information (full class name and line number). Then you can use this method in synchronize statements without writing new lock object.

// Synchronization objects (locks)
private static HashMap<String, Object> locks = new HashMap<String, Object>();
// Simple method
private static Object atomic() {
    StackTraceElement [] stack = Thread.currentThread().getStackTrace(); // get execution point 
    StackTraceElement exepoint = stack[2];
    // creates unique key from class name and line number using execution point
    String key = String.format("%s#%d", exepoint.getClassName(), exepoint.getLineNumber()); 
    Object lock = locks.get(key); // use old or create new lock
    if (lock == null) {
        lock = new Object();
        locks.put(key, lock);
    }
    return lock; // return reference to lock
}
// Synchronized code
void dosomething1() {
    // start commands
    synchronized (atomic()) {
        // atomic commands 1
        ...
    }
    // other command
}
// Synchronized code
void dosomething2() {
    // start commands
    synchronized (atomic()) {
        // atomic commands 2
        ...
    }
    // other command
}

Answer 6

The java.util.concurrent package has vastly reduced the complexity of my thread safe code. I only have anecdotal evidence to go on, but most work I have seen with synchronized(x) appears to be re-implementing a Lock, Semaphore, or Latch, but using the lower-level monitors.

With this in mind, synchronizing using any of these mechanisms is analogous to synchronizing on an internal object, rather than leaking a lock. This is beneficial in that you have absolute certainty that you control the entry into the monitor by two or more threads.