javamultithreadingdining-philosopher

Dining Philosopher's solution ending up in deadlock


I've implemented the resource hierarchy solution to the Dining Philosopher's Problem. When I try to compare the two Chopsticks' n values, they end up in deadlock. However, if I use their hashCodes instead of n values, it runs smoothly. Why this difference? Aren't they both numbers at the end of the day?

import java.util.Random;

class Chopstick {
    public final int n;
    public Chopstick(int n) {
        this.n = n;
    }
}

class Philosopher extends Thread {
    private Chopstick left, right;
    private Random random;
    private final int n;

    public Philosopher(int n, Chopstick left, Chopstick right) {
        this.n = n;
        if (left.n > right.n) { // no deadlock if I replace this with left.hashCode() > right.hashCode()
            this.right = left;
            this.left = right;
        } else {
            this.left = left;
            this.right = right;
        }
        this.random = new Random();
    }

    @Override
    public void run() {
        try {
            while (true) {
                Thread.sleep(random.nextInt(10)); // Think
                synchronized(left) {
                    synchronized(right) {
                        System.out.println("P " + n + " eating");
                        Thread.sleep(random.nextInt(10));
                    }
                }
            }
        } catch(InterruptedException ie) {
            ie.printStackTrace();
        }
    }

}

class Main {
    public static void main(String[] args) {
        final int n = 3;
        Chopstick[] sticks = new Chopstick[n];
        Philosopher[] ps = new Philosopher[n];
        for (int i = 0; i < n; i++) {
            sticks[i] = new Chopstick(n);
        }
        for (int i = 0; i < n; i++) {
            ps[i] = new Philosopher(i, sticks[i], sticks[(i + 1) % n]);
            ps[i].start();
        }
    }
}

Solution

  • Your problem is related to the fact that you don't manage cases where left.n == right.n and unfortunately instead of initializing your Chopstick array with sticks[i] = new Chopstick(i), you used sticks[i] = new Chopstick(n) such that you have only cases of type left.n == right.n which are not properly managed so you get deadlocks.

    As you did not override the method hashCode(), using hashCode() helps to avoid the problem because they are different instances of Chopstick with different values of hashCode() but you could still meet cases where we have 2 different instances of Chopstick with the same hashCode(). So you still have to manage cases where we have equal values.

    The way to manage equal values properly is by using a third lock called the "tie breaking" lock

    class Philosopher extends Thread {
    
        // The tie breaking lock
        private static Object tieLock = new Object();
        ...
    
        private void printAndSleep() throws InterruptedException {
            synchronized(left) {
                synchronized(right) {
                    System.out.println("P " + n + " eating");
                    Thread.sleep(random.nextInt(10));
                }
            }
        }
    
        public void run() {
            ...
            if (left.n == right.n) {
                // Equal values so we need first to acquire the tie breaking lock
                synchronized (tieLock) {
                    printAndSleep();
                }
            } else {
                printAndSleep();
            }
            ...
        }
    }
    

    A more generic way to manage lock ordering is by relying on System.identityHashCode(obj) as value of each instance to sort instead of using a field's value or hashCode() because this way you won't depend on something specific to the target object's type.

    More details in chapter 10.1.2 Dynamic lock order deadlock of Java Concurrency in Practice from Brian Goetz