一個排序引發的BUG

why技術發表於2021-06-29

你好呀,我是why。

前兩天在 Git 上閒逛的時候又不知不覺逛到 Dubbo 那裡去了。

看了一下最近一個月的資料,社群活躍度還是很高的:

然後看了一下最新的 issue,大家提問都很積極。

其中看到了這樣的一個 issue,發現有點意思:

https://github.com/apache/dubbo/issues/8055

於是寫下這篇文章給你分享一下這個 BUG 和 BUG 背後的故事。

放心,就算你完全不懂 Dubbo,也不影響你瞭解這個 BUG。

先說一下,下文中提到的 Dubbo 程式碼,沒有特別說明的地方,都是我從 git 上拉下來的 Master 分支上的程式碼

啥 BUG 啊?

先給你描述一個這個 BUG 是啥樣的。

其實就是這個 issue 的作者寫出來的:Dubbo 框架裡面的 Filter 排序過程有問題,即使按照框架要求寫好規則後,最終生成的 Filter 鏈並不是我們想要的。

那麼完全不懂 Dubbo 的朋友可能就遇到了第一個問題:啥是 Filter 呢?

其實就是一個過濾器而已,和 web 服務裡面過濾器在概念上沒啥兩樣。而 Dubbo 有非常多的 Filter,這些 Filter 共同組成了一個 Filter 呼叫鏈。

引用官網上的一個呼叫鏈路圖,在 Filter 的地方我框起來了:

可以看到 Filter 是 Dubbo 框架的一個非常核心的組成部分,很多很多的功能都是從 Filter 擴充套件出來的。

你要是還不明白也沒關係,你只要知道有這樣的一個 Filter 呼叫鏈就行了,鏈上的 Filter 各司其職,各幹各的事兒。

好的,那麼現在需求來了:

我現在要求鏈上的 Filter 的執行順序是我能控制的,即我定義 Filter 的時候你得給我留個地方設定它的優先順序。

聽起來是很簡單的一個需求,對吧?

我直接給你留個口子,讓你輸入 order 引數,不輸入給個預設值,然後組裝 Filter 鏈的時候根據 Order 排個序。

不是我吹牛,十分鐘就能寫完,中間還帶著三分鐘的摸魚。

但是,就這麼個需求出 BUG 了。

具體啥現象呢?

我這裡把專案拉下來,基於官方的測試用例,改巴改巴,給你演示一下這個 BUG 的體現是啥。

在 Dubbo 裡面有這樣的一個註解:

org.apache.dubbo.common.extension.Activate

這裡的 Order 就是做排序用的。簡單演示一下,你看我現在有 5 個 Filter:

排序規則是 Order 越小的越先執行,那麼這個 Filter 鏈的執行順序應該是這樣的:

Filter4 -> Filter3 -> Filter2 -> Filter1 -> Filter5

搞個測試案例,我們驗證一下:

符合預期,沒有任何毛病。

另外說明一下,官方的關於 Filter 的測試用例在這裡,你有興趣,原始碼拉下來就可以看:

org.apache.dubbo.common.extension.support.ActivateComparatorTest#testActivateComparator

不管是官方的案例,還是我自己寫的案例,其中最關鍵的排序功能是這一行程式碼實現的:

Collections.sort(filters, ActivateComparator.COMPARATOR)

而這一行程式碼裡面最關鍵的就是 ActivateComparator.COMPARATOR 這個東西。

這個東西就是 BUG 之源,不慌,等下再說。

那麼為什麼說它有 BUG 呢?

前面演示了正常的情況下,是符合預期的。

但是你看 Activate 註解,裡面還有這樣的兩個東西:

before、after,含義是指定 Filter A 在 Filter B 之前或者之後執行。

但是被打上了 @Deprecated 註解,欄位說明上也備註了:

Deprecated since 2.7.0

2.7.0 之後被廢除。

那麼就有點意思了,為啥被廢除?

來,看個例子,還是剛剛的那個測試用例。

我就稍微的這麼一改:

@Activate(before = "_2")
public class Filter5 implements Filter0{
}

改動點就是在 Filter5 上配置了:

@Activate(before = "_2")

含義就是 Filter5 在 “_2” 之前執行。

“_2” 是啥?

就是 Filter2 的一個對映而已:

那麼問題就來了,作為一個正常的程式猿,自信的對 Filter5 進行了這個改動之後,他內心的想法一定是想要把這樣的 Filter 鏈:

Filter4 -> Filter3 -> Filter2 -> Filter1 -> Filter5

修改為這樣:(Filter5 在 Filter2 之前執行):

Filter4 -> Filter3 -> Filter5 -> Filter2 -> Filter1

那麼實際情況是怎樣的呢?

來跑一把:

咋回事?這不是我預期的執行鏈啊?

是的,這就是 BUG 的表現。

咋回事啊?

到底是咋回事呢?

且聽我給你分析一波。

上一小節我說了,問題出在排序演算法上。

org.apache.dubbo.common.extension.support.ActivateComparator

來,一起看一下:

首先標號為 ① 的地方就是把 before、after、order 封裝了一下,然後提供了幾個比較的方法。你知道 ActivateInfo 這個實體裡面有這些東西就行了,後面的程式碼會用到。

然後說說標號為 ② 的地方。

這個地方你別看挺長的,但是其實邏輯特別簡單,當前對比的兩個 filter 中的任何一個配置了 before、after 就會進入到標號為 ② 的部分的邏輯。

然後這裡面的一坨邏輯是的這樣的:

具體邏輯不細說了,等會給你來個直觀的演示。

最後標號為 ③ 這個地方,有點意思,稍微多說幾句。

能走到標號為 ③ 的地方,說明當前對比的兩個 filter 都沒有配置 after、before 這兩個屬性。

直接對比 Order 就行了。

這個地方對 Order 相等的情況還做了一個特殊處理:

o1.getSimpleName().compareTo(o2.getSimpleName()) > 0 ? 1 : -1

如果 Order 相等,再比較類名。

這樣做的原因也是保證排序的穩定性。

舉個例子,比如這兩 Filter,都沒有指定 Order:

那如果我們去掉這個判斷:

程式碼就變成了這樣:

if (a1.order > a2.order) {
    return 1 
else {
    return -1 
    
}

簡化一下就是這樣:

return a1.order > a2.order ? 1:-1

那麼這一塊的程式碼,整體就會變成這樣:

這樣仔細一看:咦,好像還能再優化一下。78 行和 80 行是一樣的,所以可以去掉 78 行。

好的,經過這樣的一番改造。

恭喜你,獲得了一個老版本的程式碼:

左邊是之前版本的程式碼,右邊是現在 Master 分支的程式碼:

為什麼會發生變化,必然是有原因的。

看一眼提交記錄:

這次提交指向了編號為 7778 的提交:

https://github.com/apache/dubbo/pull/7778

而這次提交指向了編號為 7757 的 issue:

https://github.com/apache/dubbo/issues/7757

而這個 issue 在前面提到的編號為 8055 的 issue 裡也提到了:

這個 issue 主要就是兩張圖。

第一張圖是這樣的:

在沒有任何自定義 Filter,僅有官方原有的 Filter 的情況下,構建出來的 Filter 鏈,ExecuteLimitFilter 在 MonitorFilter 之前。

第二張圖是這樣的:

在加入了一系列自定義的 Filter(沒有指定 Order)之後,ExecuteLimitFilter 就排在了 MonitorFilter 之後了。

至於這兩個 Filter 排前排後的影響是什麼,和文字關係不大,就不擴充套件了,你有興趣的可以去看看對應的連結。

總之,只有這樣的判斷邏輯是不穩當的:

return a1.order > a2.order ? 1:-1

來個例子演示一下:

左邊是測試用例,右邊是排序規則,下面是輸出結果。

從輸出結果可以看到,最終的 Filter 鏈取決於 list 的新增順序。

這也就是 7757 這個 issues 說的:

list 的遍歷順序會影響到排序的順序。

因此,才會有了這樣的一次提交:

好,現在我們把排序順序改回來,同樣的測試用例再跑一次,就穩定了:

眼睛尖的朋友可能還發現了一個問題。

這個地方還有一次提交:

  • 第一種判斷:return o1.getSimpleName().compareTo(o2.getSimpleName())
  • 第二種判斷:return o1.getSimpleName().compareTo(o2.getSimpleName()) > 0 ? 1 : -1;

你說這是在幹啥?

第一種判斷還疏忽了這樣的一種情況,包名不同但是類名相同的情況:

  • com.why.a.whyFilter
  • com.why.b.whyFilter

這個時候 o1.getSimpleName().compareTo(o2.getSimpleName()) 返回的是 0。

返回 0 會發生啥?

直接吞掉一個 Filter 你信不信?

比如你的集合是 HashSet,或者是 TreeMap。

這就巧了,Dubbo 用的就是 TreeMap。

來個測試用例演示一下。

如果採用第一種判斷,最後 TreeMap 裡面只有一個 Filter 了:

如果採用第二種判斷,最後 TreeMap 裡面會有兩個 Filter :

細節,魔鬼都在細節裡面。

哎呀,真的是防不勝防啊。

好了,比較器我就說完了,但是你發現沒有,我到現在都還沒給你說排序過程不穩定這個 BUG 到底是啥,只是給你引申了一個其他的 BUG 出來。

莫慌,這不是我還沒想好怎麼給你描述嘛。

這個過程其實比較複雜,涉及到 Timsort 排序方法,就這方法就得另起一篇文章才能說清楚。

所以,我換了一個思路,主要給你看比較的過程,至於這個過程背後的原因。

就是 Timsort 在搞鬼,歡迎你自己去探索一番。

那過程是啥呢?

我在比較方法的入口處加上這樣的輸出語句:

五個 Filter 是這樣的:

測試用例是這樣的:

@Test
public void whyTest(){
    List<Class> filters = new ArrayList<>();
    filters.add(Filter4.class);
    filters.add(Filter3.class);
    filters.add(Filter2.class);
    filters.add(Filter1.class);
    filters.add(Filter5.class);
    Collections.sort(filters, ActivateComparator.COMPARATOR);
    StringBuilder builder = new StringBuilder();
    for (int i = 0; i < filters.size(); i++) {
        builder.append(filters.get(i).getSimpleName()).append("->");
    }
    System.out.println(builder.toString());
}

輸出的日誌是這樣的:

發現問題了沒?

首先我很心機的控制了一下 list 的新增順序:

這樣前三次比較就能構建這樣的 Filter 鏈:

Filter4->Filter3->Filter2->Filter1->

然後,Filter5 進來後先和 Filter1 比,發現其 Order 為 0 比 Filter1 的 -1 大,於是比較結束,得到這樣的Filter 鏈:

Filter4->Filter3->Filter2->Filter1->Filter5->

整個過程中,Filter5 與 Filter2 完全沒有發生任何比較的操作,也就更不涉及到 Filter5 裡面的 before 標籤了:

但是當我把 list 的新增順序修改一下:

咦,就正確了,你就說神不神奇?

神奇吧?

為啥呢?

去看看 Timsort 的原理吧。

追根溯源

其實寫到這裡的我產生了一個疑問:

是誰,什麼時候,引入了 after/before 機制?

因為這個機制我個人覺得出發點是挺好的,多一個配置的地方,把選擇權留給使用者。

但是在實際的使用中,卻容易出現比較混亂的情況。

於是我看了一下提交記錄:

這個註解最早是樑飛(就是 Dubbo 專案要的開創者之一)寫出來的,而設計之初沒有 before 和 after,但是有一個 match 和 mismatch。

然後在寫出這個註解一天之後的凌晨 1 點 54 分,提交了一個方法級別的匹配:

這三個方法使用起來甚至比 before/after 更加複雜了。

於是一覺睡醒之後的 12:34 分,樑飛又刪除了這三個配置:

兩個月之後的 2012 年 5 月 8 日,加入了 after 和 before 配置:

然後就一直留在 Dubbo 原始碼裡面,直到 6 年後的 2018 年 8 月 7 日,打上了不建議使用的註解:

並提到了這個 issue:

https://github.com/apache/dubbo/issues/2180

裡面說:Dubbo 原始碼中沒有使用 after 和 before,且排序是存在問題的。

於是這兩個方法,在 2.7.0 版本之後,被標註為不建議使用,宣告了該方法的死亡。

我不知道 2012 年,樑飛為什麼引入了這兩個方法,我也曾想從他的程式碼提交記錄上找到點蛛絲馬跡,可惜沒有。

但是,有了另外的一個想法:

當年樑飛引入這兩個方法後,他寫的比較器,是否考慮到了這樣的情況呢?

於是我馬上又看了比較器的程式碼提交記錄:

org.apache.dubbo.common.extension.support.ActivateComparator

並且把他的程式碼拷貝了出來,用同樣的測試用例跑了一下:

很遺憾,也有一樣的問題。

或許,當年就不應該引入這兩個方法。

大道至簡,學 Spring 的 Order,就只有一個 Order:

然後我又突然想了另外一個框架:SofaRPC。

SofaRPC 和 Dubbo 和 HSF 之間有著千絲萬縷的愛恨情仇,於是我去瞅了一眼 SofaRPC 對應的地方:

com.alipay.sofa.rpc.ext.Extension

用於排序的,也就只是保留了 order。

這樣比較器的程式碼就很簡單了:

com.alipay.sofa.rpc.common.struct.OrderedComparator

另外,我順便對比了一下樑飛最早寫的比較器和現在最新的比較器的程式碼,功能完全一樣,但是程式碼卻差異較大:

不得不說,經過幾次重構之後,最新的比較器的可讀性高了很多。

我追蹤了一下這個類的提交記錄,也就看著這個類的一步步演化,其實算是一個比較好的程式碼重構的例子。

有興趣的自己去翻一翻。

好了,就到這了,打完收工。

相關文章