Project

General

Profile

Actions

Backport #5757

closed

main threadがreadやselectで待っていると、^C でなかなか死なない

Added by naruse (Yui NARUSE) over 12 years ago. Updated about 12 years ago.


Description

FreeBSD 9 にて、 ./ruby と起動して ^C を投げてもなかなか死にません。
./miniruby でも -e'$stdin.read' でも同じです。

仕組みとしては、main thread が read や select で待つ場合、最近は blocking region で
unblock.func に ubf_select を設定するわけですが、この時にシグナルが来ると、

  1. どこかのスレッドの sighandler が呼ばれて、rb_thread_wakeup_timer_thread() が呼ばれる
  2. タイマースレッドが起きて、thread_timer() -> timer_thread_function() -> rb_threadptr_check_signal() -> rb_threadptr_interrupt() -> (th->unblock.func)(th->unblock.arg) -> ubf_select() -> rb_thread_wakeup_timer_thread() が呼ばれる
  3. タイマースレッドが起きて、thread_timer() -> timer_thread_function() -> rb_threadptr_check_signal() -> rb_threadptr_interrupt() -> (th->unblock.func)(th->unblock.arg) -> ubf_select() -> rb_thread_wakeup_timer_thread() が呼ばれる
  4. タイマースレッドが起きて、thread_timer() -> timer_thread_function() -> rb_threadptr_check_signal() -> rb_threadptr_interrupt() -> (th->unblock.func)(th->unblock.arg) -> ubf_select() -> rb_thread_wakeup_timer_thread() が呼ばれる
    ...

対策はいくつかあり得ると思うのですが、例えば、ubf_select() から rb_thread_wakeup_timer_thread() を呼ばないようにするとか


Files

bug5757.patch (1.26 KB) bug5757.patch nagachika (Tomoyuki Chikanaga), 12/14/2011 09:45 PM

Related issues 1 (0 open1 closed)

Related to Ruby master - Bug #5343: Unexpected blocking behavior when interrupt Socket#acceptClosedko1 (Koichi Sasada)09/20/2011Actions

Updated by kosaki (Motohiro KOSAKI) over 12 years ago

[Bug #5343] (r33307) でのregressionですね(193ブランチだと r33310)。単純にそこを削除すると Bug #5343が復活してしまうので、タイマー貼る必要があるんじゃないですかね。タイマースレッドを起こすためのタイマーって泥縄感が半端ないけど。

Updated by nagachika (Tomoyuki Chikanaga) over 12 years ago

Bug #5343 を登録したものです。
ちゃんと試さずに書いてしまいますが、 #5343 は別の Thread からの kill や raise での割り込みの場合の問題だったと記憶していますので、ubf_select() での rb_thread_wakeup_timer_thread() の呼び出しはタイマースレッドで実行する場合は呼ばないようにすることで両方大丈夫にならないでしょうか。

diff --git a/thread_pthread.c b/thread_pthread.c
index afef326..16f7674 100644
--- a/thread_pthread.c
+++ b/thread_pthread.c
@@ -924,6 +924,8 @@ native_sleep(rb_thread_t *th, struct timeval *timeout_tv)
thread_debug("native_sleep done\n");
}

+static pthread_t timer_thread_id;
+
#ifdef USE_SIGNAL_THREAD_LIST
struct signal_thread_list {
rb_thread_t *th;
@@ -1018,7 +1020,8 @@ ubf_select(void *ptr)
{
rb_thread_t *th = (rb_thread_t *)ptr;
add_signal_thread_list(th);

  • rb_thread_wakeup_timer_thread(); /* activate timer thread */
  • if ( pthread_self() != timer_thread_id )
  •   rb_thread_wakeup_timer_thread(); /* activate timer thread */
    
    ubf_select_each(th);
    }

@@ -1047,7 +1050,6 @@ ping_signal_thread_list(void) {
static int ping_signal_thread_list(void) { return 0; }
#endif /* USE_SIGNAL_THREAD_LIST */

-static pthread_t timer_thread_id;
static int timer_thread_pipe[2] = {-1, -1};
static int timer_thread_pipe_owner_process;

Updated by kosaki (Motohiro KOSAKI) over 12 years ago

  • ruby -v changed from ruby 2.0.0dev (2011-12-12 trunk 34015) [x86_64-freebsd9.0] to -

2011年12月13日19:45 Tomoyuki Chikanaga :

Issue #5757 has been updated by Tomoyuki Chikanaga.

Bug #5343 を登録したものです。
ちゃんと試さずに書いてしまいますが、 #5343 は別の Thread からの kill や raise での割り込みの場合の問題だったと記憶していますので、ubf_select() での rb_thread_wakeup_timer_thread() の呼び出しはタイマースレッドで実行する場合は呼ばないようにすることで両方大丈夫にならないでしょうか。

いや、スレッドがGVL競合してなくて、ubf_selectを登録したけどまだselectを読んでないとき
ubf_selectのkill(VTALRM) はスカるので、もう一回ubf_select_eachが呼ばれるようにする
トリックが必要だという認識です。

もうコードがうろ覚えなのでなにか見落としてる可能性もありますが。

Updated by naruse (Yui NARUSE) over 12 years ago

  • ruby -v changed from - to ruby 2.0.0dev (2011-12-12 trunk 34015) [x86_64-freebsd9.0]

レビューありがとうございます。

いや、スレッドがGVL競合してなくて、ubf_selectを登録したけどまだselectを読んでないとき
ubf_selectのkill(VTALRM) はスカるので、もう一回ubf_select_eachが呼ばれるようにする
トリックが必要だという認識です。

そのもう一回も外しちゃう可能性って無いんでしょうか。
どちらかというと、rb_thread_kill 側で確実に止めを刺す用にするべきな気もします。

rb_thread_kill で rb_thread_wakeup_timer_thread() というのも考えていたんですが、
とりあえず [ruby-dev:44992] で動いてるっぽかったので、各プラットフォームでの確認の利便性を鑑み、
r34038 にてコミットしてます。

Updated by ko1 (Koichi Sasada) over 12 years ago

  • ruby -v changed from ruby 2.0.0dev (2011-12-12 trunk 34015) [x86_64-freebsd9.0] to -

(2011/12/14 15:00), Yui NARUSE wrote:

そのもう一回も外しちゃう可能性って無いんでしょうか。
どちらかというと、rb_thread_kill 側で確実に止めを刺す用にするべきな気もします。

 もう一回外す可能性があるため,必要が無くなるまで何度もやります.

--
// SASADA Koichi at atdot dot net

Updated by nagachika (Tomoyuki Chikanaga) over 12 years ago

Motohiro KOSAKI wrote:

いや、スレッドがGVL競合してなくて、ubf_selectを登録したけどまだselectを読んでないとき
ubf_selectのkill(VTALRM) はスカるので、もう一回ubf_select_eachが呼ばれるようにする
トリックが必要だという認識です。
少しゆっくり考えてみましたところ、ubf_select() を呼ぶのがタイマースレッドでなければこれまで通り rb_thread_wakeup_timer_thread() を呼ぶので、 signal_thread_list に入れたものをタイマースレッドが確実に blocking region を抜けるまで面倒をみてくれる(定期的にSIGVTALRM送信)ので安泰です。
しかしタイマースレッドがシグナルを処理させるために rb_threadptr_check_signal() からメインスレッドに割り込みする時の ubf_select() で rb_thread_wakeup_timer_thread() を呼ばなくなるため、シグナル受信処理時に SIGVTALRM の喪失してメインスレッドが止まったままになる可能性があるのではないかと思います。

thread_timer() の ping_signal_thread_list() で signal_thread_list をチェックした(そして空だった)後に timer_thread_function() で新たに signal_thread_list にメインスレッドが追加された場合が問題なわけなので、この後に signal_thread_list のチェックを入れればいいのではないかと思います。というわけで追加のパッチを添付します。

Actions #7

Updated by naruse (Yui NARUSE) over 12 years ago

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

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


  • thread_pthread.c (ubf_select): call rb_thread_wakeup_timer_thread()
    only when it is not timer_thread. [Bug #5757] [ruby-dev:44985]
    patched by Tomoyuki Chikanaga.

Updated by nagachika (Tomoyuki Chikanaga) over 12 years ago

  • Status changed from Closed to Open

[ruby-dev:44999] に書いたように今度はシグナル処理の漏れの懸念があるので再 open させて頂きます。

Updated by kosaki (Motohiro KOSAKI) over 12 years ago

  • Status changed from Open to Assigned
  • Assignee set to kosaki (Motohiro KOSAKI)

すいません。なるべく早く時間取ってレビューします

Updated by kosaki (Motohiro KOSAKI) over 12 years ago

  • Assignee changed from kosaki (Motohiro KOSAKI) to nagachika (Tomoyuki Chikanaga)

しばらく考えた結果、[ruby-dev:44999] のパッチで問題ないという結論に至りました。コミットお願い出来ますでしょうか

Updated by nagachika (Tomoyuki Chikanaga) over 12 years ago

確認ありがとうございます。 r34099 でコミットしました。

Updated by nagachika (Tomoyuki Chikanaga) over 12 years ago

  • Status changed from Assigned to Closed
Actions #13

Updated by naruse (Yui NARUSE) about 12 years ago

  • Tracker changed from Bug to Backport
  • Project changed from Ruby master to Backport193
  • Status changed from Closed to Open
Actions #14

Updated by naruse (Yui NARUSE) about 12 years ago

  • Status changed from Open to Closed

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


merge revision(s) r34038,34099:

* thread_pthread.c (ubf_select): call rb_thread_wakeup_timer_thread()
  only when it is not timer_thread. [Bug #5757] [ruby-dev:44985]
  patched by Tomoyuki Chikanaga.

* thread_pthread.c (ping_signal_thread_list): remove return value.

* thread_pthread.c (check_signal_thread_list): add a new function to
  check if signal thread list is empty.

* thread_pthread.c (thread_timer): check signal thread list after
  timer_thread_function(). main thread might be added into signal thread
  list during timer_thread_function().
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0