Avoid synchronized(this) in Java?

I'll cover each point separately.

1. Some evil code may steal your lock (very popular this one, also has an "accidentally" variant)

   I'm more worried about accidentally. What it amounts to is that this use of this is part of your class' exposed interface, and should be documented. Sometimes the ability of other code to use your lock is desired. This is true of things like Collections.synchronizedMap (see the javadoc).

2. All synchronized methods within the same class use the exact same lock, which reduces throughput

   This is overly simplistic thinking; just getting rid of synchronized(this) won't solve the problem. Proper synchronization for throughput will take more thought.

3. You are (unnecessarily) exposing too much information

   This is a variant of #1. Use of synchronized(this) is part of your interface. If you don't want/need this exposed, don't do it.

Well, firstly it should be pointed out that:

public void blah() {  synchronized (this) {    // do stuff  }}

is semantically equivalent to:

public synchronized void blah() {  // do stuff}

which is one reason not to use synchronized(this). You might argue that you can do stuff around the synchronized(this) block. The usual reason is to try and avoid having to do the synchronized check at all, which leads to all sorts of concurrency problems, specifically the double checked-locking problem, which just goes to show how difficult it can be to make a relatively simple check threadsafe.

A private lock is a defensive mechanism, which is never a bad idea.

Also, as you alluded to, private locks can control granularity. One set of operations on an object might be totally unrelated to another but synchronized(this) will mutually exclude access to all of them.

synchronized(this) just really doesn't give you anything.

While you are using synchronized(this) you are using the class instance as a lock itself. This means that while lock is acquired by thread 1, the thread 2 should wait.

Suppose the following code:

public void method1() {    // do something ...    synchronized(this) {        a ++;          }    // ................}public void method2() {    // do something ...    synchronized(this) {        b ++;          }    // ................}

Method 1 modifying the variable a and method 2 modifying the variable b, the concurrent modification of the same variable by two threads should be avoided and it is. BUT while thread1 modifying a and thread2 modifying b it can be performed without any race condition.

Unfortunately, the above code will not allow this since we are using the same reference for a lock; This means that threads even if they are not in a race condition should wait and obviously the code sacrifices concurrency of the program.

The solution is to use 2 different locks for two different variables:

public class Test {    private Object lockA = new Object();    private Object lockB = new Object();    public void method1() {        // do something ...        synchronized(lockA) {            a ++;              }        // ................    }    public void method2() {        // do something ...        synchronized(lockB) {            b ++;              }        // ................    }}

The above example uses more fine grained locks (2 locks instead one (lockA and lockB for variables a and b respectively) and as a result allows better concurrency, on the other hand it became more complex than the first example ...