How to prevent this simple deadlock in java?

Question

I have simple example which illustrates how to cause deadlock:

class Player {
    private String name;

    public Player(String name) {
        super();
        this.name = name;
    }

    public synchronized void passTo(Player p) {
        System.out.println(this.name + " passes to " + p.name);
        // to imitate long task
        for (int i = 0; i < 1000000000; i++)
            ;
        p.passBack(this);
    }

    private synchronized void passBack(Player p) {
        System.out.println(this.name + " passes back to " + p.name);
    }
}

public class Deadlock {

    public static void main(String[] args) {
        final Player ivan = new Player("Ivan");
        final Player petro = new Player("Petro");

        new Thread(new Runnable() {
            public void run() {
                ivan.passTo(petro);
            }
        }).start();
        new Thread(new Runnable() {
            public void run() {
                petro.passTo(ivan);
            }
        }).start();
    }
}

When I run this program it causes deadlock.

What are possible solutions to prevent this simple deadlock?

Thank you!

Not using the synchronized keyword ? In your case you only have 1 ball. So the first pass has to be complete before the second pass can take place. Its the right thing to do. To make it seamless reduce the amount its called. Or reduce the time it takes. It seems to do the right thing currently — exussum, May 18 '14 at 07:35
Why do you need `passBack` to the synchronized? it's private and it's only called from `passTo` — morgano, May 18 '14 at 07:36
There is no deadlock in you code. To have a deadlock, your passBack() method has to call passTo(). Then the methods will wait for each other. In your case, you always call passTo() first, there will never be a deadlock. — sergej shafarenka, May 18 '14 at 08:11

score 1 · Accepted Answer · answered May 18 '14 at 07:30

1

You could make public synchronized void passTo(Player p) method as static. So at a time only one player can call the passTo() method and there will be no deadlock i.e you remove cyclic dependency. So change it to

public static synchronized void passTo(Player p)

answered May 18 '14 at 07:30

Aniket Thakur

66,731
38
279
289

`this` is used in `passTo()` and `passBack()` methods. – Braj May 18 '14 at 08:05
which is why lock obtained will be instance level lock which will led to deadlock. To resolve this issue you need to get class level lock. So that even if there are two instances of Player only one will get lock at a time avoiding deadlock. – Aniket Thakur May 18 '14 at 09:24

Braj · Answer 2 · 2014-05-18T08:04:28.520

You need lock on the object that is independent of objects of the class and that it is class itself.

To get the lock on the class itself you have to make the method static as well along with synchronized

Sample code: (Adjust the code as per your requirement)

class Player {
    private String name;

    public Player(String name) {
        super();
        this.name = name;
    }

    public static synchronized void passTo(Player to, Player from) {
        System.out.println(from.name + " passes to " + to.name);
        Player.passBack(from, to);
    }

    private static synchronized void passBack(Player from, Player to) {
        System.out.println(from.name + " passes back to " + to.name);
    }

}

final Player ivan = new Player("Ivan");
final Player petro = new Player("Petro");

new Thread(new Runnable() {
    public void run() {
        Player.passTo(petro, ivan);
    }
}).start();
new Thread(new Runnable() {
    public void run() {
        Player.passTo(ivan, petro);
    }
}).start();

output:

Ivan passes to Petro
Ivan passes back to Petro
Petro passes to Ivan
Petro passes back to Ivan

How to prevent this simple deadlock in java?

2 Answers2