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();
  }
}

There is nothing in your code that needs to be synchronized, so you can just remove the `synchronized` keyword everywhere. If you have an actual problem with real-world code, where this happens, then people may be able to offer you suggestions on how to resolve the problem. — Erwin Bolwidt, Jun 18 '15 at 04:46
This is just a pattern. There may be things need to be synchronized in a real-life code — Konstantin Pavlov, Jun 18 '15 at 04:54
there is no deadlock in your code. put code snippet relevant to your problem. — juggernaut, Jun 18 '15 at 05:01
@ErwinBolwidt some methods must be synchronized **methodA, methodB, A.last()**. Are there any solution? — Tan Le, Jun 18 '15 at 06:12
@kpavlov This is an anti-pattern, clearly, as it results in a deadlock. If you get this in real code, you've modelled something wrong. It depends on the actual code what exactly you've modelled wrong and how to resolve it. There is no generic solution. — Erwin Bolwidt, Jun 18 '15 at 06:16

score 1 · Answer 1 · answered Jun 18 '15 at 18:44

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).

Joshua · Answer 2 · 2015-06-18T17:49:47.570

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();
  }
}

are there any solutions? This code uses in embedded systems so I can not use **ReentrantLock** — Tan Le, Jun 18 '15 at 06:14

how to resolve deadlock causes by the synchronized method

2 Answers2