給JDK提的一個bug(關於AbstractQueuedSynchronizer.ConditionObject)

時間 2019-12-13

標籤 jdk 一個 bug 關於 abstractqueuedsynchronizer.conditionobject abstractqueuedsynchronizer conditionobject 欄目 Java 简体版

原文原文鏈接

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。html

這顯然是有問題的，ConditionObject中的firstWaiter/lastWaiter都是沒有被volatile修飾的，它們的可見性是經過鎖的獲取和釋放來保證的。若是有線程由於某些狀況實際上沒有持有互斥鎖，可是調用了ConditionObject的await，儘管可能會由於fullyRelease方法的調用發現未持有互斥鎖而拋出IllegalMonitorStateException，但此時可能已經對ConditionObject的內部數據結果形成了永久性破壞。好比可能有些其餘正常經過持有鎖來await的線程，不再能被喚醒了。java

2. 提bug

不得不說，這個bug是看源碼看出的bug。JDK或者咱們平常不會遇到的緣由是由於咱們一直在正確地使用Condition。保證await前要先持鎖，保證signal前也要先持鎖。可是做爲JDK，必須保證庫的魯棒性，也就是在乎外狀況下一樣可以處理，而且不會崩。數據結構

AQS的Condition和JVM提供的wait/notify的native實現，效果是很相似的。做爲對比使用wait/notify。即使在有線程未持鎖的狀況下調用wait，會拋出IllegalMonitorStateException,可是原先wait的線程仍然最終能夠被喚醒。可是AQS的Condition因爲上述邏輯上的一些疏忽，會致使錯誤的調用拋出指望的異常，但對內部數據結構形成破壞，致使不可靠。oop

因而，上了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同樣，在方法一開始就先去檢查是否持有鎖就好了。設計

還有人稱不明白爲何測試用例要打開斷言。其實個人測試用例裏的斷言只是爲了證實那些非法請求確實都被拋出異常了。code

不事後麪人以爲確實是bug，還有個哥們稱他發現這個bug貌似早就被引入了。而後翻出了Doug Lea老爺子很早之前有一次改代碼的記錄,參考連接。

htm

這算是上古時期AQS的代碼了，能夠看到Doug Lea老爺子當時把checkConditionAccess方法改成了isHeldExclusively。checkConditionAccess原來是會在每一個ConditionObject方法(await/signal那些)都被調用的，可是isHeldExclusively卻不會在await方法中調用，await方法經過release來判斷鎖。

因此這就是這個bug的根源，從邏輯上來講確實release裏面也包含了判斷是否持有互斥鎖的邏輯，但實現語義以及魯棒性卻由於這個改動被弱化了不少。