Bug #4723

Bug #4696: thread.c#lock_func() が spurious wakeup unsafe

check_deadlock_i での transition_for_lockの扱いが thread unsafe

Added by Motohiro KOSAKI almost 3 years ago. Updated almost 3 years ago.

[ruby-dev:43563]
Status:Closed
Priority:Normal
Assignee:Motohiro KOSAKI
Category:core
Target version:1.9.3
ruby -v:ruby 1.9.3dev (2011-05-13 trunk 31548) [x86_64-linux] Backport:

Description

kosakiです。別のチケット切ります。

あと、transitionforlock に volatile を付けるのもお願いします

これは明らかに悪い効果を及ぼしようがないので、先にコミットしました。
しかし、ご存じの通り、volatileをつけることでスレッド安全になるのは
マシンがUP(マシンにCPUが1つしかない)環境に限られており、最近は
デスクトップでもその仮定はあやしい。

というわけで、transitionforlockを思い切って削除してしまうパッチを
作りました。設計ポイントは

・lockfunc()を出るときにtransitionforlockは元々なんの意味も無かった。lockfunc出口で
GVL取れずに滞留しているときは、mutex->th や RUBYVMINTERRUPTED(th) を見ることで
checkdeadlocki()から状態変化が可視であった。
・元のデザインは GVLとmutex->lockを同時にとっては危ないという考えてそれを回避すべく
頑張っていたが、ソースコード全体を調査したところ、そんな事ないことが分かった
・よって、最初にGVLとmutex->lockを両方とも持っている瞬間をつくって、そこでvm->sleeper
をインクリメントすることにより、vm->sleeperはインクリメントされたが、mutex->condwaiting
がまだインクリメントされてない瞬間が check
deadlock_i() から見える事はなくなる

どうでしょう?

0001-remove-transition_for_lock.patch Magnifier (3.04 KB) Motohiro KOSAKI, 05/17/2011 10:48 PM

Associated revisions

Revision 32022
Added by Motohiro KOSAKI almost 3 years ago

  • thread.c: remove th->transitionforlock. It's thread unsafe. [Bug #4723]

History

#1 Updated by Motohiro KOSAKI almost 3 years ago

deadlock checkを入れるときの議論のURLを貼ります

http://markmail.org/message/xxlb4hnai54etixu#query:+page:1+mid:4ekc6xwdubrjno5r+state:results

#2 Updated by Motohiro KOSAKI almost 3 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r32022.
Motohiro, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • thread.c: remove th->transitionforlock. It's thread unsafe. [Bug #4723]

Also available in: Atom PDF