1

I am working on understanding deadlock basics so I came up with below code. I have two threads acquiring locks in opposite order but they're not deadlocking. When I run it I see all the printouts. What am I doing wrong?

public class DeadlockBasics {
  private Lock lockA = new ReentrantLock();
  private Lock lockB = new ReentrantLock();

  public static void main(String[] args) {
    DeadlockBasics dk = new DeadlockBasics();
    dk.execute();
  }

  private void execute() {
    new Thread(this::processThis).start();
    new Thread(this::processThat).start();
  }

  // called by thread 1
  public void processThis() {
    lockA.lock();
    // process resource A
    System.out.println("resource A -Thread1");

    lockB.lock();
    // process resource B
    System.out.println("resource B -Thread1");

    lockA.unlock();
    lockB.unlock();
  }

  // called by thread 2
  public void processThat() {
    lockB.lock();
    // process resource B
    System.out.println("resource B -Thread2");

    lockA.lock();
    // process resource A
    System.out.println("resource A -Thread2");

    lockA.unlock();
    lockB.unlock();
  }
}
John Kugelman
  • 349,597
  • 67
  • 533
  • 578
flash
  • 1,455
  • 11
  • 61
  • 132

4 Answers4

2

First of all there is no garantee which threads is start first. To get the deadlock one of the thread has to take a lock on lockA and then the second thread has to take a lock on lockB or visa versa.

public void processThis() {
    lockA.lock();
    // here the control should be switched to another thread
    System.out.println("resource A -Thread1");

    lockB.lock();
    ...

But there may not be enough time to switch between thread because you have just a few lines of code.. It is too fast. To emulate some long work add delay before the second lock to both methods

lockA.lock();
Thread.sleep(200);  // 200 milis

Then the second thread will be able to lock lockB before the first release both of them

Ruslan
  • 6,090
  • 1
  • 21
  • 36
1

This could indeed result in a deadlock but not always, for example if the processThis() is completely executed and then the processThat() or vice versa there is no deadlock. You can try to add a Thread.delay(100) or a Thread.yield() to steer the threads execution towards the deadlock or even removing the unlocks to a certain deadlock.

1

Your code is a good example of dead lock, since ReenttrantLock is a mutual exclusion lock with same behavior as the implicit monitor lock access by using synchronized. However you don't see the deadlock because of this part:

private void execute() {
      new Thread(this::processThis).start();
      new Thread(this::processThat).start();
}

After the first thread is created and started, it will takes a while to create the second thread. It takes the JVM about 50 us or maybe even less to create a new thread, it sounds very short, but it is enough for the first thread to be finished and therefore a dead lock will not happen.

I added a Thread.sleep(); into your code so the both threads could be executed somehow parallely.

package com.company;

import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

public class DeadlockBasics {
    private Lock lockA = new ReentrantLock();
    private Lock lockB = new ReentrantLock();

    public static void main(String[] args) {
        DeadlockBasics dk = new DeadlockBasics();
        dk.execute();
    }

    private void execute() {
        new Thread(this::processThis).start();
        new Thread(this::processThat).start();
    }

    // called by thread 1
    private void processThis() {
        lockA.lock();
        // process resource A
        try {
            Thread.sleep(1000); //Wait for thread 2 to be executed
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println("Thread 1 will own lock a");

        lockB.lock();
        // process resource B
        System.out.println("Thread 1 will own lock b");

        lockA.unlock();
        lockB.unlock();

        // Both locks will now released from thread 1
    }

    // called by thread 2
    private void processThat() {
        lockB.lock();
        // process resource B
        try {
            Thread.sleep(1000);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println("Thread 2 will own lock b");

        lockA.lock();
        // process resource A
        System.out.println("Thread 2 will own lock a");

        lockA.unlock();
        lockB.unlock();

        // Both locks are released by thread 2
    }
}
curiouscupcake
  • 1,157
  • 13
  • 28
1

Two points:

  1. Release locks in the reverse order of acquiring them. That is, processThis should reverse the order of removing the locks. For your example, the order doesn't matter. But if processThis attempted to acquire a new lock on A before releasing the lock on B a deadlock could again occur. More generally, you'll find it easier to think about locks by considering their scope and by avoiding overlapping but non-enclosing scopes.
  2. To better highlight the problem, I would put in call to wait after acquiring the first lock in each of threads, and have execute launch both threads then invoke notify on both threads.
Thomas Bitonti
  • 1,179
  • 7
  • 14