Bug #6911

Sync_m#sync_unlock で ThreadError が発生する場合がある

Added by Kenta Murata over 2 years ago. Updated over 2 years ago.

[ruby-dev:46074]
Status:Closed
Priority:Normal
Assignee:-
ruby -v:ruby 2.0.0dev (2012-08-23 trunk 36795) [x86_64-darwin11.4.0] Backport:

Description

Sync_m#sync_unlock で ThreadError が発生する場合があります。
原因は Sync_m#sync_lock が同一スレッドを複数回 sync_waiting 配列に push してしまうからです。

同一スレッドが多重に sync_waiting 配列に入っていると、sync_unlock の中で何度もそのスレッドに対して Thread#run が呼ばれます。
すると、1回目の Thread#run でそのスレッドが終了した場合、2回目の Thread#run で ThreadError が発生します。

sync_waiting に同一スレッドが複数回 push された状態を再現するコードを gist に置きました。
https://gist.github.com/3434046

この gist に添付されてる test を実行すると、以下のような結果が得られると思います。

[1/1] SyncTest#test_synchronize = 0.51 s
1) Failure:
test_synchronize(SyncTest) [/Users/kenta-murata/work/ruby.git/test/test_sync.rb:57]:
<[#,
#,
#]> expected but was
<[#,
#,
#,
#]>.

修正用のパッチも上記 gist に添付してあります。修正方法がこの通りで良ければコミットしたいので、どなたかレビューをお願いします。


Related issues

Duplicates Ruby trunk - Bug #5355: Sync_mにBug #5195やBug #5258と同様のバグ Closed 09/23/2011

History

#1 Updated by Motohiro KOSAKI over 2 years ago

修正パッチがgistに見つけられないのはぼくだけ?

#2 Updated by Kenta Murata over 2 years ago

わお、ごめんなさい。添付し忘れていたようです。更新しました。

以下にも貼ります。

diff --git a/lib/sync.rb b/lib/sync.rb
index bae05a4..05864c2 100644
--- a/lib/sync.rb
+++ b/lib/sync.rb
@@ -147,7 +147,7 @@ module Sync_m
sync_upgrade_waiting.push [Thread.current, sync_sh_locker[Thread.current]]
sync_sh_locker.delete(Thread.current)
else
- sync_waiting.push Thread.current
+ sync_waiting.push Thread.current unless sync_waiting.include? Thread.current
end
@sync_mutex.sleep
end

#3 Updated by Kenta Murata over 2 years ago

sync_unlock 内で th.wakeup した後に th.run していますが、th.run のときに既にスレッドが死んでいる場合があるようです。
この状況にも対応できるようパッチを更新しました。

#4 Updated by Motohiro KOSAKI over 2 years ago

  • Status changed from Open to Closed

調査した結果 5355と同じだと分かったのであっちのパッチいれます。ごめんね

#5 Updated by Motohiro KOSAKI over 2 years ago

余談なんですけど、なんで Mutex.sleep を thread.run で起こせる仕様なんでしたっけ? Thread.stop, Thread.sleep 以外は起こせないほうが普通の感覚かと思うのですが。
とはいえ、スレッドライブラリはTimeout moduleとかでexeptionが突然飛んできても大丈夫なように書いてあるべきなので、この制限をいれてもSync_mを直さないといけないという結論は変わらないのですが

Also available in: Atom PDF