CI框架(3 or 4) session鎖問題引發的討論
阿新 • • 發佈：2018-12-09
角色介紹
sskaje:提起疑問者,疑似國人
narfbg:CI作者之一
sskaje:在CI3和CI4中,用來實現session的redis驅動與memcached驅動關於session key鎖的設計是不對的.
估計是sskaje一針見血的表達導致了作者的第一次回覆會帶有不滿情緒..
CI4中,程式碼老套,雖然更加相容但是仍然有bugCI4中關於session鎖的迴圈中的虛擬碼如下:
if redis::ttl(lock_key) > 0 then sleep 1s continue loop redis::set(lock_key, ttl)
這裡sskaje拿出了CI4中的程式碼,估計是當時CI4中程式碼還沒更新,作者也提到在後來的版本中升級了這段程式碼.但是作者的方法就是接下來的這段程式碼,直接kill掉了其他的併發請求.
沒錯,兩個併發請求會同時通過ttl(有效時間)>0的檢查,並且同時上鎖.併發的請求對session執行寫操作可能會導致資料汙染,然而1.併發的session讀操作2.一個請求執行session寫操作,多個併發請求執行session讀操作都能正常執行在CI3的某個提交中,引進了如下邏輯(虛擬碼):
if redis::ttl(lock_key) > 0 then sleep 1s continue loop if redis::ttl(lock_key) == key-not-exists then redis::setNx(lock_key, ttl) else redis::set(lock_key, ttl)
然而並沒有什麼卵用反而錯的更離譜了與CI4中的情況一樣,都是併發請求可以同時越過ttl>0並且匹配到ttl===-2的條件,因為此時還不存在key.請求1在執行redis::setNx後返回成功,請求2因為已經存在了key而失敗了.因此請求2將無法獲取到session資料
redis::setNx與redis::set不同點在於只對不存在的key生效,若key已存在則會返回false,然後sskaje給出了簡單的解決方案:
redis:
protected function _get_lock($session_id) { // PHP 7 reuses the SessionHandler object on regeneration, // so we need to check here if the lock key is for the // correct session ID. if ($this->_lock_key === $this->_key_prefix.$session_id.':lock') { log_message('error', 'SESSION Redis _get_lock'); return $this->_redis->setTimeout($this->_lock_key, 300); } // 30 attempts to obtain a lock, in case another request already has it $lock_key = $this->_key_prefix.$session_id.':lock'; $attempt = 0; do { $result = $this->_redis->set($lock_key, time(), array('nx', 'ex'=>300)); if (! $result) { sleep(1); continue; } $this->_lock_key = $lock_key; break; } while (++$attempt < 30); if ($attempt === 30) { // in case someone added a key without expiration $ttl = $this->_redis->ttl($lock_key); if ($ttl === -1 || $ttl > 300) { $this->_redis->setTimeout($lock_key, 300); } log_message('error', 'Session: Unable to obtain lock for '.$this->_key_prefix.$session_id.' after 30 attempts, aborting.'); return FALSE; } $this->_lock = TRUE; return TRUE; }
Memcached:
protected function _get_lock($session_id)
{
// PHP 7 reuses the SessionHandler object on regeneration,
// so we need to check here if the lock key is for the
// correct session ID.
if ($this->_lock_key === $this->_key_prefix.$session_id.':lock')
{
if ( ! $this->_memcached->replace($this->_lock_key, time(), 300))
{
return ($this->_memcached->getResultCode() === Memcached::RES_NOTFOUND)
? $this->_memcached->add($this->_lock_key, time(), 300)
: FALSE;
}
return TRUE;
}
// 30 attempts to obtain a lock, in case another request already has it
$lock_key = $this->_key_prefix.$session_id.':lock';
$attempt = 0;
do
{
if ($this->_memcached->add($lock_key, time(), 300))
{
sleep(1);
continue;
}
$this->_lock_key = $lock_key;
break;
}
while (++$attempt < 30);
if ($attempt === 30)
{
// if someone manually adds a key with a very long ttl, this session data would be not accessible by clients
log_message('error', 'Session: Unable to obtain lock for '.$this->_key_prefix.$session_id.' after 30 attempts, aborting.');
return FALSE;
}
$this->_lock = TRUE;
return TRUE;
}
然後給出了2點關於memcahed的建議,是因為memcahed沒有提供讀取key的ttl的api的問題,因為我用的是redis就略過啦~然後是作者不滿的回覆,因為你一來就直接說我程式碼寫的不好有bug.
narfbg(作者):這是非常,非常不可理喻的討論首先,你在CI3的倉庫中討論CI4的程式碼沒有任何意義,只會讓其他人感到疑惑,無法理解其次,你沒有給出關於錯誤所在與預期結果的問題陳訴,我只覺得你在說一些關於TTL和返回值的事(當然了,這些是與問題有關係的,但都是實現細節的事情)最後,你提供了一些簡短的虛擬碼之後就貼了一大段方法的程式碼上來.是想讓其他人直接複製貼上到自己專案中去而不展示一下跟之前程式碼的不同之處嗎?請詳細的解釋一下,僅僅針對CI3的工作環境下從在什麼場景下你期望得到什麼結果開始解釋吧.
作者表示不懂sskaje在說什麼,其實只是來自程式設計師的報復吧,我覺得說的很明白了
sskaje:親愛的narfbg,只是討論一下,CI3與CI4的session處理程式幾乎是一樣的.所以我就沒有在CI4的倉庫裡再提PR了.在討論清楚後我會在兩個專案下都提PR的.
接著sskaje貼了一大段程式碼表示CI4與3的session處理程式幾乎一致,然後被作者diss了..
現在,我應該表達清楚了我們只需要在CI3的專案中討論這個問題了在新程式碼中,2個新的請求,A和B,擁有相同的session_id S,同時執行到這段程式碼:
if (($ttl = $this->_redis->ttl($lock_key)) > 0)
{
sleep(1);
continue;
}
同時檢查了lock_key S的ttl並且返回-2,因為此時還未上鎖,接下來
$result = ($ttl === -2)
? $this->_redis->set($lock_key, time(), array('nx', 'ex' => 300))
: $this->_redis->setex($lock_key, 300, time());
A與B都返回了-2,所以他們同時來到了這裡 setNx $this->_redis->set($lock_key, time(), array('nx', 'ex' => 300))如果A先執行,因為設定了nx的原因,B就會失敗,反之亦然
if ( ! $result)
{
log_message('error', 'Session: Error while trying to obtain lock for '.$this->_key_prefix.$session_id);
return FALSE;
}
B沒有辦法拿到鎖,生成了如下錯誤日誌:
ERROR - 2018-01-17 17:06:31 --> Session: Error while trying to obtain lock for nbss:11075jfvblitou09ic772dm3e8gp11ni
ERROR - 2018-01-17 17:06:31 --> Severity: Warning --> session_start(): Failed to read session data: user (path: tcp://redis.xxx.xxx.xxx:6379?prefix=nbss:&weight=1) ........../CodeIgniter/system/libraries/Session/Session.php 127
看下我的虛擬碼,我定位的問題如下:1.檢查並且執行是錯的,如果不存在則加上是對的.2.redis與memcached的實現面臨著同樣的問題:如果有人錯誤的將lock key的ttl設定了很長的時間或者永不過期,那麼這個session id將永遠無法訪問如果不存在則新增 redsi:
$result = $this->_redis->set($lock_key, time(), array('nx', 'ex'=>300));
在redis中回收錯誤的lock key:
if ($attempt === 30) {
// in case someone added a key without expiration
$ttl = $this->_redis->ttl($lock_key);
if ($ttl === -1 || $ttl > 300) {
$this->_redis->setTimeout($lock_key, 300);
}
log_message('error', 'Session: Unable to obtain lock for '.$this->_key_prefix.$session_id.' after 30 attempts, aborting.');
return FALSE;
}
memcached:沒有完美的方法,可以這樣寫
if ($attempt === 30)
{
// if someone manually adds a key with a very long ttl, this session data would be not accessible by clients
$lock_data = $this->_memcached->get($lock_key);
if ($lock_data < time() - 300) {
$this->_memcached->delete($lock_key);
}
log_message('error', 'Session: Unable to obtain lock for '.$this->_key_prefix.$session_id.' after 30 attempts, aborting.');
return FALSE;
}
我不明白為什麼之前的session鎖的寫法,也不明白為什麼會被修復成這樣.
說的很明白了~小題外話,目前版本,也就是sskaje現在質疑的這段程式碼,是一位叫做tianhe1986的中國程式設計師提交的,在2017年5月份提出的pr並且在作者的稽核下合併了程式碼.
然後呢,作者引用了sskaje的一些話來回復
narfbg(作者):現在,我應該表達清楚了我們只需要在CI3的專案中討論這個問題了我希望看到的討論就是針對CI3的實現.你寫了這麼多,有一大半的內容就是貼出了幾個版本中不一樣的4行程式碼..是我寫的基礎演算法,我當然知道它們很相似,並且這種高級別的解決方案應該被普及.我們現在就3.1分支中最新程式碼的redis驅動來討論,只要這個明白了剩下的就好說了.
因為sskaje貼了很多程式碼來說在這個倉庫裡討論就行了,搞的作者很不爽
"A與B都返回了-2,所以他們同時來到了這裡 setNx $this->_redis->set($lock_key, time(), array('nx', 'ex' => 300))如果A先執行,因為設定了nx的原因,B就會失敗,反之亦然"據我瞭解,你希望B請求除了超時是不會失敗的.並且你認為導致他失敗的原因是資源競爭.而#5170解決的是完全不一樣的問題-在同樣是資源競爭的情況下,2個請求都成功會有潛在的資料汙染的風險.而這才是#5170所解決的如果我理解的沒問題(再一次的),你建議的更改如下:(-表示刪除 +表示增加
@@ -335,22 +335,12 @@ class CI_Session_redis_driver extends CI_Session_driver implements SessionHandle
$attempt = 0;
do
{
- if (($ttl = $this->_redis->ttl($lock_key)) > 0)
+ if ( ! $this->_redis->set($lock_key, time(), array('nx', 'ex' => '300')))
{
sleep(1);
continue;
}
- $result = ($ttl === -2)
- ? $this->_redis->set($lock_key, time(), array('nx', 'ex' => 300))
- : $this->_redis->setex($lock_key, 300, time());
-
- if ( ! $result)
- {
- log_message('error', 'Session: Error while trying to obtain lock for '.$this->_key_prefix.$session_id);
- return FALSE;
- }
-
$this->_lock_key = $lock_key;
break;
}
這樣的目的是為了消除資源競爭的情況,讓B請求永不失敗,對嗎?再一次的,我們需要專注於核心問題,何況目前的討論還是忽略了memcached與不存在過期時間的key的情況下-這又是另一個問題了
這裡的核心問題指的是資源競爭應該如何處理的問題,而不是應該直接忽略它的存在.
- 計算機執行過程中，併發、無序、大量的程序在使用有限、獨佔、不可搶佔的資源，由於程序無限，資源有限，產生矛盾，這種矛盾稱為競爭（Race）。
- 由於兩個或者多個程序競爭使用不能被同時訪問的資源，使得這些程序有可能因為時間上推進的先後原因而出現問題，這叫做競爭條件（Race Condition）。
sskaje:作者同學:#5170是通過殺掉2個併發請求的其中一個來解決資源競爭問題的,副作用是導致了1個失敗請求.更明智的做法是等到超時或者其他請求釋放了鎖或者到達了最大嘗試次數而不是直接殺掉請求.所以沒錯,這個簡化了的程式碼是我建議的一部分.長時間/永久有效期的key問題也因為這個簡化而暴露出問題.另外,我曾為前公司設計框架時,引進了一套只讀模式session處理程式,因為我的大多數api都需要驗證登入狀態但都是隻讀的.如果開啟了只讀模式,該模式下一旦儲存了過期時間後session處理程式將會是無鎖的並且不會執行寫操作,這在高負載網站中是非常有效的.我在想哪個能成為預設的session鎖模式.殺掉程序太暴力,這樣是bug而不是解決方法;阻塞等待其他請求,如果其他請求慢也不好;優化成只讀模式太複雜;無鎖你怎麼看?
長時間/永久有效期的key表示的是開發人員用了與這個鎖的key一樣的key,並且設定了長時間有效期,或者設定成了永久有效的.
殺掉請求並不是真正的殺掉,只是在session的read或者write方法重寫裡返回了false,原文是killin就翻成殺掉了.
narfbg(作者):#5170是通過殺掉2個併發請求的其中一個來解決資源競爭問題的,副作用是導致了1個失敗請求.沒錯更明智的做法是等到超時或者其他請求釋放了鎖或者到達了最大嘗試次數而不是直接殺掉請求.所以沒錯,這個簡化了的程式碼是我建議的一部分我也同意我現在明白你為什麼要一直提到#5170了,但要知道-之前只用了setex(),引起了資料汙染.最重要的是當時#5170解決了這個問題,我想當時沒人想到你的解決方法.就這樣,沒必要再討論#5170了.時間/永久有效期的key問題也因為這個簡化而暴露出問題現在來討論當有人設定鎖大於300s或者永久有效的情況吧問題是,這是很難發生的,並且沒有好的解決方法:* 忽略並且當做獲得了key - 可能導致資料汙染* 忽略並且當做失敗 - 可能永遠無法獲取到key* 刪除/重寫 - 操作了其他開發人員創造的資料另一方面,我不覺得這是由資源競爭的任何一個解決方案引起的問題.老實說,為啥會有人故意去設定一個這麼特別的key(一個高熵隨機key),那這個人的程式碼出問題是很正常的了..另外,我曾為前公司設計框架時,引進了一套只讀模式session處理程式,因為我的大多數api都需要驗證登入狀態但都是隻讀的酷,但說句題外話:我希望你說的不是REST API,因為那應該是無狀態的並且不應該有session這個概念 :)如果開啟了只讀模式,該模式下一旦儲存了過期時間後session處理程式將會是無鎖的並且不會執行寫操作,這在高負載網站中是非常有效的.不能是無鎖的一個共享的讀鎖可以同時被多個請求獲取,但它始終是個鎖-寫操作應該在讀的期間被阻塞.在CI框架中會存在這些問題:* 在session開啟前,你需要提前申明只讀模式(我大概是唯一一個會手動載入session而不用自動載入的人了)* 請求越多,越有可能有寫請求加入佇列中.其實從鎖設計中受益的正是我們的一部分使用者群,他們寫了存在很多ajax請求的應用,或者一些長輪詢 - 這些正是導致問題的理想環境.我們正是為此而模擬了鎖,可能在作業系統級別上能解決這個問題吧* 有一點你說錯了...不是高負荷網站 - 高負荷其實是由多個不同的使用者引起的,他們都有著不同的sessionId,都擁有各自的鎖不是說沒有用,只是不適合CI我在想哪個能成為預設的session鎖模式預設一詞意味著更多的配置選項 - 所以對不起.只能有一種模式殺掉程序太暴力,這樣是bug而不是解決方法;你不能將你認為是次優的解分類成bug..bug是非預期的負面影響所以這不是bug.話雖這麼說,歡迎更好的解決方案.阻塞等待其他請求,如果其他請求慢也不好;所以請優化你的app :)優化成只讀模式太複雜;同意,如上所述無鎖你怎麼看?不行.絕對不行!這曾是我們的噩夢,回顧一下CI2中與session相關的問題,90%都是因為無鎖而導致的.引進鎖並且強制開啟是CI3中重寫session類的主要原因,我寧願去掉整個session類也不願意讓它無鎖.我在使用者手冊中的一整章中都在告誡人們不要試圖去掉鎖.所以絕不會發生.
作者的態度緩和了很多,然後有理有據讓人信服,觀點是一定要存在鎖,鎖在人在,同時有表明了確實在CI中沒有更好的解決方案.
之後呢,過了12天,sskaje大概忙去了沒有回覆.說句題外話:我翻了下sskaje的個人網站,發現有很多文章裡有中文..
narfbg(作者):我已經修復了這個問題,但你說你想在自己PR前討論一下,要我等你還是我自己來了?
其實還是保持了次優解,我看了CI4中的程式碼,也與一開始提出來的程式碼是一致的.
sskaje:作者同學:不好意思上個禮拜有些忙我剛剛re-forked了CI3推送了一些程式碼https://github.com/sskaje/CodeIgniter/commit/c8e321c488a662ad2f5f191d4642eb4bb0dff919有些程式碼風格的問題,會稍後修改我讓鎖模式可配置了,因為在我的場景下真的不需要這麼多鎖並且不能阻塞這些請求但我還在猶豫讓鎖等待是否是一個最好的預設方案請reviwe,如果我覺得沒問題的話我會在CI3與4中建立PR
可惜的是,這個請求被sskaje刪除了,因為作者拒絕了這端程式碼.