How to fix this deadlock code in java?

Question

I got this deadlock sample codes from oracle.

class Thread_Test {
    static class Friend {
        private final String name;
        public Friend(String name) {
            this.name = name;
        }
        public String getName() {
            return this.name;
        }
       public synchronized void bow(Friend bower) {
            System.out.format("%s: %s" + "  has bowed to me!%n", this.name, bower.getName());

            bower.bowBack(this);
        }
        public synchronized void bowBack(Friend bower) {
            System.out.format("%s: %s" + " has bowed back to me!%n", this.name, bower.getName());
        }
    }

    public static void main(String[] args) {
        final Friend alphonse = new Friend("Alphonse");
        final Friend gaston = new Friend("Gaston");
        new Thread(new Runnable() {
            public void run() { alphonse.bow(gaston); }
        }).start();
        new Thread(new Runnable() {
            public void run() { gaston.bow(alphonse); }
        }).start();
    }
}

If I change function bow like below, I can resolve the deadlock issue.

public void bow(Friend bower) {
    synchronized(this) {
        System.out.format("%s: %s" + "  has bowed to me!%n", this.name, bower.getName());
    }
    bower.bowBack(this);
}

Can I use other approaches to resolve this deadlock issue.

Especially, I wonder that Thread.sleep() can be a solution for deadlock and how I can use it.

Could you let me know about the possibility of Thread.sleep() and other approaches?

Thanks alot

Thread.sleep is _never_ a solution for a deadlock. there are many solutions to deadlocks (e.g. lock ordering, more granular locks, non-locking structures, etc, too many to detail here), which depend on your situation. — jtahlborn, Aug 15 '14 at 15:42
possible duplicate of [Java Deadlock code explanation](http://stackoverflow.com/questions/25312797/java-deadlock-code-explanation) — Durandal, Aug 15 '14 at 15:51
another way to fix this problem is to delete the code. it all depends on what you want to accomplish. if you change the granularity of the locking then you have changed the behavior and the program isn't doing the same thing. so it's unclear what "solutions" consist of here. voting to close as unclear. — Nathan Hughes, Aug 15 '14 at 16:01

Mshnik · Answer 1 · 2014-08-15T17:44:23.297

The deadlock is occurring because the following method calls occur:

Alphonse calls bow(), getting Alphonse/bow lock
Gaston calls bow(), getting Gaston/bow lock
Alphonse tells Gaston to bowBack(), and waits for Gaston to complete that action
Gaston tells Alphonse to bowBack(), and waits for Alphonse to complete that action.
thus, both are waiting for the other to finish bowBack() in order to finish their bow(), but cannot start their own bowBack() until they finish bow(). Deadlock.

The reason your solution works is that the bowBack(this) call is outside of the synchronized block.

Smart use of Locks can both demonstrate more clearly why and exactly where deadlock is occurring, and can prevent it. Java's Semaphore class is a nice one to look into.

To actually fix it (prevent deadlock but keep it thread safe), you have to expand on the problem - what should happen when Gaston is waiting for Alphonse to finish bow() to execute his own bow(), but Alphonse needs Gason to execute a bowBack() to finish? One sensible solution is for Gaston to give up on executing bow() when the call to bowBack() comes and just execute bowBack(), but it all depends on what problem you're trying to solve.

There is no static locking here, because the synchronized methods *are not static*. The `Friend.class` lock is never used and there are actually two locks instead of one. Read the [javadoc](http://docs.oracle.com/javase/tutorial/essential/concurrency/syncmeth.html) — kajacx, Aug 15 '14 at 16:28
Interesting. So taking that into account, (and looking debugging thread trace), it seems the deadlock appears elsewhere. I'll edit my answer. — Mshnik, Aug 15 '14 at 17:41

score 1 · Answer 2 · edited May 23 '17 at 11:59

You need to lock on 2 objects at once (Let's call them A and B). here is a nice explaination of what might happen. To fix that, you could try to always lock in same order:

static class Friend {
    private static int idCounter;
    private final int id;
    private final String name;

    public Friend(String name) {
        this.name = name;
        id = idCounter++;
    }
    public String getName() {
        return this.name;
    }

    public void bow(Friend bower) {
        Friend first, second;
        if(bower.id > id) {
            first = this;
            second = bower;
        } else {
            first = bower;
            second = this;
        }
        synchronized(first) {
            synchronized(second) {
                System.out.format("%s: %s has bowed to me!%n", this.name,bower.getName());

                bower.bowBack(this);
            }
        }
    }
    public synchronized void bowBack(Friend bower) {
        System.out.format("%s: %s has bowed back to me!%n", this.name, bower.getName());
    }
}

Here the Friend with lower id gets locked first, so the deadlock now should't happen.

How to fix this deadlock code in java?

2 Answers2