javamultithreadingjunitdeadlocksynchronized

Testing a potential deadlock without sleep in JUnit


In classical Deadlock example, there are two paths in the code acquiring the same two synchronized locks, but in different order like here:

// Production code
public class Deadlock {
    final private Object monitor1;
    final private Object monitor2;

    public Deadlock(Object monitor1, Object monitor2) {
        this.monitor1 = monitor1;
        this.monitor2 = monitor2;
    }

    public void method1() {
        synchronized (monitor1) {
            tryToSleep(1000);
            synchronized (monitor2) {
                tryToSleep(1000);
            }
        }
    }

    public void method2() {
        synchronized (monitor2) {
            tryToSleep(1000);
            synchronized (monitor1) {
                tryToSleep(1000);
            }
        }
    }

    public static void tryToSleep(long millis) {
        try {
            Thread.sleep(millis);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}

This could potentially result in a deadlock. To increase the chance that it actually deadlocks, I am adding those tryToSleep(1000);, just to ensure that method1 will acquire a lock on monitor1, and method2 will acquire the lock on monitor2, before even trying to acquire next lock. So using the sleep this Deadlock simulates "unlucky" timing. Say, there is a strange requirement, that our code should have the potential to result in a deadlock, and for that reason, we want to test it:

   // Test
   @Test
    void callingBothMethodsWillDeadlock() {
        var deadlock = new Deadlock(Integer.class, String.class);

        var t1 = new Thread(() -> {
            deadlock.method1(); // Executes for at least 1000ms
        });
        t1.start();

        var t2 = new Thread(() -> {
            deadlock.method2(); // Executes for at least 1000ms
        });
        t2.start();

        Deadlock.tryToSleep(5000); // We need to wait for 2s + 2s + some more to be sure...

        assertEquals(Thread.State.BLOCKED, t1.getState());
        assertTrue(t1.isAlive());

        assertEquals(Thread.State.BLOCKED, t2.getState());
        assertTrue(t2.isAlive());
    }

This passes, which is good. Which is not good is that I had to add sleep into the Deadlock class itself, and also in its test. I had to do this just in order to make the test consistently pass. Even though if I remove sleep from everywhere this code could sometimes produce a deadlock, but then there is no guarantee that it happens during the test. Now say having sleep is unacceptable here, then the question is: How can I reliably test that this code, has a potential to cause a deadlock without any sleep neither in the test and in the actual code itself?

edit: I just wanted to emphasize that I am asking for the class to have a potential for a deadlock, only in some "unlucky" timing (when two threads are calling method1() and method2() at the same time) this deadlock should happen. And in my test, I want to demonstrate deadlock on every run. I want to remove sleep calls from the production code (hopefully from the test also). Maybe there is a way to use mocks instead of the injected monitors, so we could orchestrate them acquiring locks in a specific order during the test?


Solution

  • Essentially, you need Thread (t1) executing method1 to wait inside synchronized (monitor1) but outside ofsynchronized (monitor2) until another Thread (t2) executing method2 goes inside synchronized (monitor2) and releases t1 and both threads try to proceed.

    Or vice versa, where t2 waits until t1 comes and releases

    You can code this scenario yourself. But since you are focusing just on Deadlock testing, you can use a java.util.concurrent.CyclicBarrier between 2 parties to orchestrate this, where parties indicate the number of threads that must invoke CyclicBarrier.await() before the barrier is tripped (in other words, all the threads previous awaiting proceeds).

    class Deadlock {
        final private CyclicBarrier cyclicBarrier = new CyclicBarrier(2);
        final private Object monitor1;
        final private Object monitor2;
    
        public Deadlock(Object monitor1, Object monitor2) {
            this.monitor1 = monitor1;
            this.monitor2 = monitor2;
        }
    
        public void method1() throws BrokenBarrierException, InterruptedException {
            synchronized (monitor1) {
                cyclicBarrier.await();
                synchronized (monitor2) {
                }
            }
        }
    
        public void method2() throws BrokenBarrierException, InterruptedException {
            synchronized (monitor2) {
                cyclicBarrier.await();
                synchronized (monitor1) {
                }
            }
        }
    
        public static void tryToSleep(long millis) {
            try {
                Thread.sleep(millis);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
    

    You will have to handle the checked Exceptions threw by cyclicBarrier.await()

            Thread t1 = new Thread(() -> {
                try {
                    deadlock.method1(); 
                } catch (BrokenBarrierException | InterruptedException e) {
                    e.printStackTrace();
                }
            });
            t1.start();
    
            Thread t2 = new Thread(() -> {
                try {
                    deadlock.method2();
                } catch (BrokenBarrierException | InterruptedException e) {
                    e.printStackTrace();
                }
            });
            t2.start();
    
            deadlock.tryToSleep(5000); // Wait till all threads have a chance to become alive
    
            assertEquals(Thread.State.BLOCKED, t1.getState());
            assertTrue(t1.isAlive());
    
            assertEquals(Thread.State.BLOCKED, t2.getState());
            assertTrue(t2.isAlive());