Avoiding deadlock using ReentrantLock

Question

I was doing some practice assignments and dabbling around with some dummy code for trying to develop a better understanding of the concepts of threads and locks. Following is a piece of code that (sometimes) goes into deadlock.

A.java

public class A {

    private B b;

    public void setB(B b) {
        this.b = b;
    }

    public synchronized void foo(boolean callBar) {
        System.out.println("foo");
        if (callBar) {
            b.bar(false);
        }
    }
}

B.java

public class B {

    private A a;

    public void setA(A a) {
        this.a = a;
    }

    public synchronized void bar(boolean callFoo) {
        System.out.println("bar");
        if (callFoo) {
            a.foo(false);
        }
    }
}

Demo.java

public class Demo {
    public static void main(String[] args) {
    A a = new A();
    B b = new B();

    a.setB(b);
    b.setA(a);

    new Thread(() -> {
        a.foo(true);
    }).start();

    new Thread(() -> {
        b.bar(true);
    }).start();
    }
}

Solution: I used Locks instead of synchronized.

A.java

public class A {

    private final ReentrantLock lock = new ReentrantLock();
    private B b;

    public void setB(B b) {
        this.b = b;
    }

    public ReentrantLock lock() {
        return lock;
    }

    public boolean impendingExecute() {
        Boolean thisLock = false;
        Boolean otherLock = false;
        try {
            thisLock = lock.tryLock();
            otherLock = b.lock().tryLock();
        } finally {
            if (!(thisLock && otherLock)) {
                if (thisLock) {
                    lock.unlock();
                }
                if (otherLock) {
                    b.lock().unlock();
                }
            }
        }
        return thisLock && otherLock;
    }

    public void foo(boolean callBar) {
        System.out.println("foo");
        if (callBar && impendingExecute()) {
            try {
                b.bar(false);
            } finally {
                lock.unlock();
                b.lock().unlock();
            }
        }
    }
}

B.java

public class B {

    private final ReentrantLock lock = new ReentrantLock();
    private A a;

    public void setA(A a) {
        this.a = a;
    }

    public ReentrantLock lock() {
        return lock;
    }

    public boolean impendingExecute() {
        Boolean thisLock = false;
        Boolean otherLock = false;
        try {
            thisLock = lock.tryLock();
            otherLock = a.lock().tryLock();
        } finally {
            if (!(thisLock && otherLock)) {
                if (thisLock) {
                    lock.unlock();
                }
                if (otherLock) {
                    a.lock().unlock();
                }
            }
        }
        return thisLock && otherLock;
    }

    public void bar(boolean callFoo) {
        System.out.println("bar");
        if (callFoo && impendingExecute()) {
            try {
                a.foo(false);
            } finally {
                lock.unlock();
                a.lock().unlock();
            }
        }
    }
}

After making the above changes, the code doesn't lead to a deadlock. Is it the proper way to implement this (Basically, I want the impendingExecute() method to be reviewed.)? Also, (deviating a bit from the review) are there any real world scenarios I can encounter this?

Note: I had posted this question on Code Review but it seems reviewal of dummy code is off-topic.

Answer 1

You can use java.util.concurrent.locks.ReentrantLock. This design permits the the method to attempt to acquire the locks of both the classes, to release the locks if it fails, and to try again later if necessary. If you are required to attempt until successful, then you need to put a it in a loop and terminate with some policy.

while (true) {
    if (this.lock.tryLock()) {
        try {
          if (ba.lock.tryLock()) {
            try {
             //some logic
              break;
            } finally {
              ba.lock.unlock();
            }
          }
        } finally {
          this.lock.unlock();
        }
    }
    int n = number.nextInt(1000);
    int TIME = 1000 + n; // 1 second + random delay to prevent livelock
    Thread.sleep(TIME);
}

or you can use this solution which ensures that multiple locks are acquired and released in the same order:

if (compareTo(ba) < 0) {
        former = this;
        latter = ba;
    } else {
        former = ba;
        latter = this;
    }
    synchronized (former) {
        synchronized (latter) {
            //Some logic
        }
    }
}