给JDK提的一个bug(关于AbstractQueuedSynchronizer.ConditionObject)

1. 背景

以前读JUC的AQS源码，读到Condition部分，我当时也写了一篇源码阅读文章--(AbstractQueuedSynchronizer源码解读--续篇之Condition)[http://www.cnblogs.com/micrari/p/7219751.html]。Doug Lea大师的代码写的很好，整个设计与编码都很优秀。可是我也在最后的思考与总结中指出了Condition有一个缺陷，在于await/awaitNanos/awaitUntil那些方法，在JavaDoc中写了应该要在持有互斥锁的状况下调用，不然一般会抛出IllegalMonitorStateException。确实在AQS的Condition的实现中会抛出异常，但异常是在fullyRelease->release->tryRelease这样的一条方法调用链中被抛出的。而在fullyRelease以前，会作一件事情就是addConditionWaiter。

这显然是有问题的，ConditionObject中的firstWaiter/lastWaiter都是没有被volatile修饰的，它们的可见性是经过锁的获取和释放来保证的。若是有线程由于某些状况实际上没有持有互斥锁，可是调用了ConditionObject的await，尽管可能会由于fullyRelease方法的调用发现未持有互斥锁而抛出IllegalMonitorStateException，但此时可能已经对ConditionObject的内部数据结果形成了永久性破坏。好比可能有些其余正常经过持有锁来await的线程，不再能被唤醒了。

2. 提bug

不得不说，这个bug是看源码看出的bug。JDK或者咱们平常不会遇到的缘由是由于咱们一直在正确地使用Condition。保证await前要先持锁，保证signal前也要先持锁。可是做为JDK，必须保证库的鲁棒性，也就是在乎外状况下一样可以处理，而且不会崩。

AQS的Condition和JVM提供的wait/notify的native实现，效果是很相似的。做为对比使用wait/notify。即使在有线程未持锁的状况下调用wait，会抛出IllegalMonitorStateException,可是原先wait的线程仍然最终能够被唤醒。可是AQS的Condition因为上述逻辑上的一些疏忽，会致使错误的调用抛出指望的异常，但对内部数据结构形成破坏，致使不可靠。

因而，上了Oracle的bugs.java.com。给Oracle提了一个bug，流程不复杂也不简单，仍是要填很多bug相关信息，而且要给出测试用例。我写的用例是这样的：

import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

public class ConditionObjectTest {

    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++) {
            new Thread(() -> {
                lock.lock();
                try {
                    count++;
                    cond.await();
                } catch (InterruptedException ignore) {
                } finally {
                    lock.unlock();
                    count--;
                }
            }, "succ-" + i).start();

            new Thread(() -> {
                boolean illegalMonitor = false;
                try {
                    cond.awaitUninterruptibly();
                } catch (IllegalMonitorStateException ignore) {
                    illegalMonitor = true;
                } finally {
                    assert illegalMonitor;
                }
            }, "fail-" + i).start();

        }

        while (count != loop) {
            Thread.yield();
        }

        while (count > 0) {
            lock.lock();
            try {
                cond.signalAll();
            } finally {
                lock.unlock();
            }
        }
    }
}

其实代码很简单，要作的事情就是证实若是在有线程不持有锁调用Condition#await的状况下，最终这些非法调用都会抛出异常。可是那些正常await的线程，却会出现有的怎么也没办法被唤醒，整个程序hang住。

提完以后，Oracle的bus.java.com会显示已经提交到内部了，等内部审核经过后会分配一个bugID。过了没几天我收到邮件称bug被审核过了，分配的ID是JDK-8187408。

3.结果

接下来20多天，Oracle内部人员貌似对我提的这个bug讨论还挺多的。刚开始有人认为这不是bug，Java Doc写了须要持有锁的状况下调用await。我以为做为JDK，代码实现不能停留在经过Doc来约束的程度啊，做为库来讲鲁棒性很是重要，应该能hold住错误的请求而且还屹立不倒。并且我认为这个bug改起来很容易，和signal同样，在方法一开始就先去检查是否持有锁就好了。

还有人称不明白为何测试用例要打开断言。其实个人测试用例里的断言只是为了证实那些非法请求确实都被抛出异常了。

不事后面人以为确实是bug，还有个哥们称他发现这个bug貌似早就被引入了。而后翻出了Doug Lea老爷子很早之前有一次改代码的记录,参考连接。

这算是上古时期AQS的代码了，能够看到Doug Lea老爷子当时把checkConditionAccess方法改成了isHeldExclusively。checkConditionAccess原来是会在每一个ConditionObject方法(await/signal那些)都被调用的，可是isHeldExclusively却不会在await方法中调用，await方法经过release来判断锁。

因此这就是这个bug的根源，从逻辑上来讲确实release里面也包含了判断是否持有互斥锁的逻辑，但实现语义以及鲁棒性却由于这个改动被弱化了不少。

Doug Lea竟然改了这个bug

没想到Doug Lea老爷子最后改了这个bug。

我以为Doug Lea老爷子这明显仍是改的复杂了，这个条件有那么复杂么。不过这个代码确实很Doug Lea(一个if里干了一大堆的事情)。

不过最终JSR166小组仍是简化了if

和我本身预期的修复方法同样，不就这么一句话就搞定的事情么。
另外他们也修了一下isHeldExclusively的JavaDoc注释，指出isHeldExclusively会被全部ConditionObject方法调用到。

最后，他们还修了一下AQS的使用样例，参考连接，这里就不贴图了。

最终，此bug的修复被合并到JDK10b26中。
JDK10原来Oracle已经开始在启动开发了呀。我连Java9都不会玩。