Understanding synchronized blocks

Question

I have a problem with understanding why it is better to use synchronized(syncObject) than synchronized(this). For example, this class:

public class Pool implements ObjectPool {
    private Object[] pool;
    private int initialCapacity;
    private int available = 0;
    private int waiting = 0;
    private final Object syncObject = new Object();

    public Pool(int initialCapacity) {
        this.initialCapacity = initialCapacity;
        pool = new Object[initialCapacity]; 
    }

    public void releaseObject(Object o) throws Exception {
        synchronized (syncObject) {
            pool[available] = o;
            available++;
        }

        if (waiting > 0) {
            notify();
        }
    }
}

Answer 1

Because if you use this, then another thread trying to execute another method will have to wait, whereas if you use an object as lock, you only restrict this critical section.

Answer 2

If you are using synchronized(this), any code having a reference to your object can interact with your synchronization via synchronized(yourObject). This can have unintended side-effects or encourage other developers to write code that relies on your code using synchronized(this).

By using synchronized(myLockObj) whereas myLockObj is an object privately held within your object, no-one else can synchronize on the same object. So there’s no interaction with your locking and no dependency of code outside your class to the way you enforce thread-safety. In other words, you may change your implementation at a later time without breaking other code outside your class.

---

To name an example, Hashtable is a class where all methods are synchronized on the Hashtable itself. So the following works:

Hashtable t;
…
synchronized(t) {
  if(!t.containsKey(k)) t.put(k,v);
}

And due to the fact that this is guaranteed, the implementation can never be changed.

In contrast, ConcurrentHashMap does not provide any possibility to lock the map from the outside. So you have to use the provided putIfAbsent method to achieve similar. This allows future improvements on the implementation to improve the throughput and, in fact, such under the hood improvements were already made.

Answer 3

You must understand why you use synchronization. You do it to ensure there are no data races. If a single data race is possible for only one single field of an object, and there are several such races, there is no point synchronizing on the whole object, because you slow down the execution. Consider this code :

class SynchTestThis {
  Collection col1 = new ArrayList();
  Collection col2 = new ArrayList();

  public void addCol1(Object obj) {
    synchronized(this) {
      col1.add(obj);
    }
  }

  public void addCol2(Object obj) {
    synchronized(this) {
      col2.add(obj);
    }
  }
}

class SynchTestObj {
  Collection col1 = new ArrayList();
  Collection col2 = new ArrayList();

  public void addCol1(Object obj) {
    synchronized(col1) {
      col1.add(obj);
    }
  }

  public void addCol2(Object obj) {
    synchronized(col2) {
      col2.add(obj);
    }
  }

}

In case of SynchTestThis adding an element to both collections simultaneously is impossible. In case of SynchTestObj it can be done.

In other words, choosing an object to synchronize on is a problem of properly identifying and securing a critical section.

Answer 4

As pointed out in other answers: By synchronizing on this, you are exposing the object hat you are locking on.

But to point out again why this may be a problem: It can cause deadlocks. Imagine, for example, these two methods being executed by two different threads:

private final Object localMonitor = new Object();
private final Pool pool = new Pool(); 

void methodA()
{
    synchronized (localMonitor)
    {
        pool.releaseObject(null);
    }
}

void methodB()
{
    synchronized (pool)
    {
        synchronized (localMonitor)
        {
            System.out.println("Performing some work...");
        }
    }
}

The first thread will synchronize on the localMonitor and then try to call the method from the Pool class. The second thread will synchronize on the pool instance, and then try to synchronize on the localMonitor.

For itself, this code is "valid". This would actually be fine. Unless the method from the Pool class is synchronized as well. Then, the threads will run into a deadlock. Such a situation can be avoided by using a dedicated syncObject.

Illustrated here again, as a runnable example: Just change the object to be synchronized on by switching the commented lines to see the difference.

class Pool 
{
    private final Object syncObject = new Object();
    public void releaseObject(Object o) 
    {
        //synchronized (syncObject) // <----------- This will work 
        synchronized (this) //      // <----------- This will cause a deadlock
        {
            System.out.println("Modify pool");
        }
    }
}

class SimpleSynchronizeExample
{
    public static void main(String[] args)
    {
        SimpleSynchronizeExample s = new SimpleSynchronizeExample();
        s.start();
    }

    private final Object localMonitor = new Object();
    private final Pool pool = new Pool(); 

    void methodA()
    {
        synchronized (localMonitor)
        {
            try { Thread.sleep(100); } catch (Exception e) {}
            pool.releaseObject(null);
        }
    }

    void methodB()
    {
        synchronized (pool)
        {
            try { Thread.sleep(100); } catch (Exception e) {}
            synchronized (localMonitor)
            {
                System.out.println("Performing some work...");
            }
        }
    }



    private void start()
    {
        Thread tA = new Thread(new Runnable() 
        {
            @Override
            public void run()
            {
                methodA();
            }
        });
        Thread tB = new Thread(new Runnable()
        {
            @Override
            public void run()
            {
                methodB();
            }
        });

        tA.start();
        tB.start();

        try
        {
            tA.join();
            tB.join();
        }
        catch (InterruptedException e)
        {
            e.printStackTrace();
        }
        System.out.println("Done");
    }

}