Backport #5757
closed
main threadがreadやselectで待っていると、^C でなかなか死なない
Added by naruse (Yui NARUSE) about 13 years ago.
Updated almost 13 years ago.
Description
FreeBSD 9 にて、 ./ruby と起動して ^C を投げてもなかなか死にません。
./miniruby でも -e'$stdin.read' でも同じです。
仕組みとしては、main thread が read や select で待つ場合、最近は blocking region で
unblock.func に ubf_select を設定するわけですが、この時にシグナルが来ると、
- どこかのスレッドの sighandler が呼ばれて、rb_thread_wakeup_timer_thread() が呼ばれる
- タイマースレッドが起きて、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() が呼ばれる
- タイマースレッドが起きて、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() が呼ばれる
- タイマースレッドが起きて、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
[Bug #5343] (r33307) でのregressionですね(193ブランチだと r33310)。単純にそこを削除すると Bug #5343が復活してしまうので、タイマー貼る必要があるんじゃないですかね。タイマースレッドを起こすためのタイマーって泥縄感が半端ないけど。
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 */
@@ -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;
- 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 nagachika00@gmail.com:
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が呼ばれるようにする
トリックが必要だという認識です。
もうコードがうろ覚えなのでなにか見落としてる可能性もありますが。
- 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 にてコミットしてます。
- 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
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 のチェックを入れればいいのではないかと思います。というわけで追加のパッチを添付します。
- 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.
- Status changed from Closed to Open
- Status changed from Open to Assigned
- Assignee set to kosaki (Motohiro KOSAKI)
- Assignee changed from kosaki (Motohiro KOSAKI) to nagachika (Tomoyuki Chikanaga)
確認ありがとうございます。 r34099 でコミットしました。
- Status changed from Assigned to Closed
- Tracker changed from Bug to Backport
- Project changed from Ruby master to Backport193
- Status changed from Closed to Open
- 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().
Also available in: Atom
PDF
Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0