How to avoid the Deadlock in the below program

[murali1253]

Hello Friends,

I am learning java now.I know the following program has deadlock, now i trying the avoid the deadlock for this
program so please post your some suggestions how to avoid the deadlock for the below.

class A
{
synchronized void foo(B b)
{
System.out.println(Thread.currentThread().getName( )+" Entered into A.foo()");

try
{
Thread.sleep(1000);
}
catch (InterruptedException e)
{
System.out.println("A Caught Exception");
}

System.out.println(Thread.currentThread().getName( )+" Now trying to Access b.last()");
b.last();
System.out.println(Thread.currentThread().getName( )+" Now trying to Access b.Info()");
b.info();

}
synchronized void last()
{
System.out.println(Thread.currentThread().getName( )+" Entered into A.last()");

}
void msg()
{
System.out.println(Thread.currentThread().getName( )+" Entered into A.msg()");
}
}
class B
{
synchronized void bar(A a)
{
System.out.println(Thread.currentThread().getName( )+" Entered into B.bar()");

try
{
Thread.sleep(1000);
}
catch (Exception e)
{
System.out.println("B Caught Exception");
}
System.out.println(Thread.currentThread().getName( )+" Now trying to Access A.last()");
a.last();
System.out.println(Thread.currentThread().getName( )+" Now trying to Access A.msg()");
a.msg();
}
synchronized void last()
{
System.out.println(Thread.currentThread().getName( )+" Entered into B.last()");
}
void info()
{
System.out.println(Thread.currentThread().getName( )+" Entered into B.info()");
}
}
class DeadLockTest2 implements Runnable
{
A a=new A();
B b=new B();

DeadLockTest2()
{
Thread.currentThread().setName("Thread1");
System.out.println(Thread.currentThread().getName( )+" Is Now Running");
new Thread(this,"Thread2").start();
System.out.println(Thread.currentThread().getName( )+" Is Now Running");
System.out.println(Thread.currentThread().getName( )+" is trying to get the lock on A.foo()");
a.foo(b);
System.out.println(Thread.currentThread().getName( )+" is got the lock on A.foo()");
nonSyn();
}
public void run()
{
System.out.println(Thread.currentThread().getName( )+" Is Now Running");
System.out.println(Thread.currentThread().getName( )+" is trying to get the lock on B.bar()");
b.bar(a);
System.out.println(Thread.currentThread().getName( )+" is got the lock on B.bar()");
nonSyn();

}
public void nonSyn()
{
a.msg();
b.info();
}
public static void main(String[] args)
{
new DeadLockTest2();
}
}

[iluxa]

when you have 2 locks, you must always access them in the same order:

void method1 () {
  synchronized (lock1) {
    synchronized (lock2) {
      // do stuff
    }
  }
}
void method2 () {
  synchronized (lock1) {
    synchronized (lock2) {
      // do other stuff
    }
  }
}

works fine when you call it from different threads, whereas

void method1 () {
  synchronized (lock1) {
    synchronized (lock2) {
      // do stuff
    }
  }
}
void method2 () {
  synchronized (lock2) {
    synchronized (lock1) {
      // do other stuff
    }
  }
}

will result in a dead lock. You're using the second approach.

[murali1253]

Hi Iluxa,

Thank you so much for your valuable time and I understood the way you avoid the deadlock.I would like to know Is this the only of way(ordering of locks) to avoid the deadlock or do we have any other approach to avoid deadlocks.Please let me know.

[iluxa]

the ordering of locks is pretty much the only way (at least that I'm aware of) to GUARANTEE things will not go wrong. If you implement another mechanism and you're extremely careful and think of all possible race conditions, you could be OK as well, but this approach is a good simple safety net.

side note: it's considered bad practice to do things like

public synchronized void doStuff () {
  //...
}

because it's not really clear what you're locking on. previous snippet of code is equivalent to

public void doStuff () {
  synchronized (this) {
    //...
  }
}

it's not considered good practice to synchronize on "this". Instead, create a meaningful object and use that:

class MyClass {
  private final Object readLock = new Object ();
  private final Object writeLock = new Object ();

  public void readStuff () {
    synchronized (readLock) {
      //...
    }
  }
  public void writeStuff () {
    synchronized (writeLock) {
      //...
    }
  }
}