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都不會玩。