how to resolve deadlock causes by the synchronized method

Question

I encountered the issue like the Deadlocks and Synchronized methods. In this case, methodA, methodB, A.last() all must be the synchronized method. So I am going to resolve this issue by removing synchronized in the method B.last(). Any deadlock in this solution? Could you please let me know any solution to resolve this better?

Class A
{
  synchronized void methodA(B b)
  {
    b.last();
  }

   synchronized void last()
  {
    System.out.println(“ Inside A.last()”);
  }
}

Class B
{
  synchronized void methodB(A a)
  {
    a.last();
  }

  synchronized void last()
  {
    System.out.println(“ Inside B.last()”);
      }
}


Class Deadlock implements Runnable 
{
  A a = new A(); 
  B b = new B();

  // Constructor
  Deadlock()
  {
    Thread t = new Thread(this); 
    t.start();
    a.methodA(b);
  }

  public void run()
  {
    b.methodB(a);
  }

  public static void main(String args[] )
  {
    new Deadlock();
  }
}

Answer 1

You can use a common mutex such as a ReentrantLock or synchronized blocks between the two methods instead of synchronized.

ReentrantLock example:

Class A
{
  A(Lock lock) {
    this.lock = lock;
  }
  private Lock lock;

  void methodA(B b)
  {
    lock.lock();
    try {
      b.last();      
    } finally {
      lock.unlock();
    }
  }

  void last()
  {
    lock.lock();
    try {
      System.out.println(“ Inside A.last()”);
    } finally {
      lock.unlock();
    }
  }
}

Class B
{
  B(Lock lock) {
    this.lock = lock;
  }
  private Lock lock;
  void methodB(A a)
  {
    lock.lock();
    try {
      a.last();   
    } finally {
      lock.unlock();
    }
  }

  void last()
  {
    lock.lock();
    try {
      System.out.println(“ Inside B.last()”);     
    } finally {
      lock.unlock();
    }
  }
}


Class Deadlock implements Runnable 
{
  Lock lock = new ReentrantLock();
  A a = new A(lock); 
  B b = new B(lock);

  // Constructor
  Deadlock()
  {
    Thread t = new Thread(this); 
    t.start();
    a.methodA(b);
  }

  public void run()
  {
    b.methodB(a);
  }

  public static void main(String args[] )
  {
    new Deadlock();
  }
}

synchronized block example:

Class A
{
  A(Object lock) {
    this.lock = lock;
  }
  private Object lock;

  void methodA(B b)
  {
    synchronized(lock){
      b.last();      
    }
  }

  void last()
  {
    synchronized(lock){
      System.out.println(“ Inside A.last()”);
    }
  }
}

Class B
{
  B(Object lock) {
    this.lock = lock;
  }
  private Object lock;
  void methodB(A a)
  {
    synchronized(lock){
      a.last();   
    }
  }

  void last()
  {
    synchronized(lock){
      System.out.println(“ Inside B.last()”);     
    }
  }
}


Class Deadlock implements Runnable 
{
  Object lock = new Object();
  A a = new A(lock); 
  B b = new B(lock);

  // Constructor
  Deadlock()
  {
    Thread t = new Thread(this); 
    t.start();
    a.methodA(b);
  }

  public void run()
  {
    b.methodB(a);
  }

  public static void main(String args[] )
  {
    new Deadlock();
  }
}

Answer 2

In general, to avoid deadlocks, either use only one lock at all, or make sure that locks are always acquired in the same order.

Assuming that you decide A always has to be locked before B, a minimally invasive bugfix for your example (assuming that nothing else synchronizes against A or B objects) would be this in class B:

void methodB(A a) {
    synchronized(a) {
        synchronized(this) {
            // do whatever was in methodB before, including...
            a.last();
        }
    }
}

That way, if both locks are required, lock of A is always acquired first, causing no deadlocks.

You can also do the same with the Java 5+ java.util.concurrent locks. Removing a synchronized where not needed is of course also an option to solve the deadlock (but if synchronization was needed, it will cause race conditions instead which are usually worse than a deadlock).