Dining Philosopher's solution ending up in deadlock

Question

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

Just a style comment: You have a variable named "left" that could either refer to the chopstick on the philosopher's left hand side or, to the one on the philosopher's right. Likewise for the variable named "right". It would make more sense to name the variables "first" and "second," where the names describe the order in which the philosopher picks them up. — Solomon Slow, Nov 28 '16 at 05:17

score 5 · Accepted Answer · edited May 23 '17 at 12:02

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

"A more generic way to avoid "Dynamic lock order deadlock" is by using System.identityHashCode(obj)" no it's not. Never expect any kind of hash to be unique, there's no guarantee for that to be the case. It's also clearly impossible: The hash code is only an int so on 64 bit VMs it's impossible for it to be unique. — Voo, Nov 26 '16 at 14:06

score 3 · Answer 2 · answered Nov 26 '16 at 08:23

3

The BUG is that you have

sticks[i] = new Chopstick(n);

when it should be

sticks[i] = new Chopstick(i);

The hash values of the objects will still be unique even if their data is the same since you haven't overridden the hashCode function.

answered Nov 26 '16 at 08:23

Ralph Ritoch

3,260
27
37

Dining Philosopher's solution ending up in deadlock

2 Answers2