Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8187408

AbstractQueuedSynchronizer wait queue corrupted when thread awaits without holding the lock

    XMLWordPrintable

Details

    Backports

      Description

        FULL PRODUCT VERSION :
        java version "1.8.0_112"
        Java(TM) SE Runtime Environment (build 1.8.0_112-b16)
        Java HotSpot(TM) 64-Bit Server VM (build 25.112-b16, mixed mode)

        ADDITIONAL OS VERSION INFORMATION :
        Darwin 192.168.0.104 16.6.0 Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64

        A DESCRIPTION OF THE PROBLEM :
        200 threads invoke AbstractQueuedSynchronizer.ConditionObject#await() concurrently.
        100 threads which hold the exclusive lock succeed while remain ones without holding the lock fail and get IllegalMonitorStateException.
        After all 100 threads release the exclusive lock, the main thread will continuously invoke signalAll() until all waiting threads are waken.

        One possible result is that the main thread will hang which means some wait threads will NEVER be waken.


        STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
        Run the test program with assertion enabled.

        EXPECTED VERSUS ACTUAL BEHAVIOR :
        EXPECTED -
        The given test program should be able to end.
        All waiting threads should be eventually signaled regardless of whether there were failing attempts at invoking awaiting().
        ACTUAL -
        The main thread failed to signal all waiting threads and the program will never end.
        Although all threads without holding the exclusive lock failed and got IllegalMonitorStateException, these failing attempts did make some side effects which will cause the ConditionObject not to work.



        REPRODUCIBILITY :
        This bug can be reproduced often.

        ---------- BEGIN SOURCE ----------
        import java.util.concurrent.locks.Condition;
        import java.util.concurrent.locks.Lock;
        import java.util.concurrent.locks.ReentrantLock;

        public class ConditionObjectTest {

            // to record lock acquire/release
            private static volatile int count = 0;

            public static void main(String[] args) throws InterruptedException {
                Lock lock = new ReentrantLock();
                Condition cond = lock.newCondition();

                int loop = 100;
                for (int i = 0; i < loop; i++) {
                    // thread that can await on the condition
                    new Thread(() -> {
                        lock.lock();
                        try {
                            count++;
                            cond.await();
                        } catch (InterruptedException ignore) {
                        } finally {
                            lock.unlock();
                            count--;
                        }
                    }, "succ-" + i).start();

                    // thread that fails to await on the condition
                    new Thread(() -> {
                        boolean illegalMonitor = false;
                        try {
                            cond.awaitUninterruptibly();
                        } catch (IllegalMonitorStateException ignore) {
                            illegalMonitor = true;
                        } finally {
                            assert illegalMonitor;
                        }
                    }, "fail-" + i).start();

                }

                // make sure that all succ threads got the lock before we try to signal them.
                while (count != loop) {
                    Thread.yield();
                }

                // signal all threads and retry if count > 0
                while (count > 0) {
                    lock.lock();
                    try {
                        cond.signalAll();
                    } finally {
                        lock.unlock();
                    }
                }
            }
        }

        ---------- END SOURCE ----------

        CUSTOMER SUBMITTED WORKAROUND :
        Regardless of the fact that AbstractQueuedSynchronizer.ConditionObject#await can handle the case that the current thread does not hold the exclusive lock and throw IllegalMonitorStateException to respond, it may become unreliable and dangerous.

        So make sure the lock is acquired before invoking ConditionObject#await, otherwise some waiting threads may always wait.

        SUPPORT :
        YES

        Attachments

          Issue Links

            Activity

              People

                martin Martin Buchholz
                webbuggrp Webbug Group
                Votes:
                0 Vote for this issue
                Watchers:
                8 Start watching this issue

                Dates

                  Created:
                  Updated:
                  Resolved: