Project

General

Profile

Actions

Bug #4723

closed

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

check_deadlock_i での transition_for_lockの扱いが thread unsafe

Added by kosaki (Motohiro KOSAKI) over 11 years ago. Updated over 11 years ago.

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

Description

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

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

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

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

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

どうでしょう?


Files

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

Updated by kosaki (Motohiro KOSAKI) over 11 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.


Actions

Also available in: Atom PDF

Like0
Like0Like0