Bug #5368

ensure節でsleepするようなThreadがあるとインタプリタが終了しない

Added by Masaki Matsushita over 2 years ago. Updated 3 months ago.

[ruby-dev:44546]
Status:Assigned
Priority:Normal
Assignee:Motohiro KOSAKI
Category:core
Target version:current: 2.2.0
ruby -v:- Backport:

Description

=begin
次のコードを実行するとCPU使用率が跳ね上がった状態になりインタプリタが終了しません。

Thread.new do
begin
sleep
ensure
sleep
end
end

現在のrbthreadterminateallでは最初に1回だけ生きているスレッドに対してterminateiを実行していますが、ensure節でsleepするようなThreadがあると、そのThreadは寝たままになってしまいwhile(!rbthreadalone())が無限ループになってしまいます。

while(!rbthreadalone())の毎回のループでカレントスレッドがメインスレッドであった場合に、生きているスレッドに対してterminate_iを実行するようなpatchを書いたところ、このバグは再現しなくなりました。
patchを添付します。patchの適用後もtest-allをパスします。
=end

patch.diff Magnifier (795 Bytes) Masaki Matsushita, 09/26/2011 01:51 PM


Related issues

Related to ruby-trunk - Feature #1952: cannot stop with Ctrl+C Closed 08/18/2009
Related to ruby-trunk - Bug #7460: メインスレッド終了後のサブスレッド終了待ち処理においてデッドロック検知が動作していない Assigned 11/29/2012

Associated revisions

Revision 37865
Added by Motohiro KOSAKI over 1 year ago

  • thread.c (rbthreadterminateall): use nativesleep() instead of rbthreadschedule(). Otherwise, it consume 100% cpu meaninglessly. [Bug #5368]
  • thread.c (threadstartfunc_2): last sub-thread wakes up main thread.

History

#1 Updated by Koichi Sasada over 2 years ago

  • ruby -v changed from ruby 1.9.4dev (2011-09-26 trunk 33338) [x86_64-linux] to -

(2011/09/25 21:51), Masaki Matsushita wrote:

次のコードを実行するとCPU使用率が跳ね上がった状態になりインタプリタが終了しません。

Thread.new do
begin
sleep
ensure
sleep
end
end

現在のrbthreadterminateallでは最初に1回だけ生きているスレッドに対してterminateiを実行していますが、ensure節でsleepするようなThreadがあると、そのThreadは寝たままになってしまいwhile(!rbthreadalone())が無限ループになってしまいます。

 ここで,バグとは何でしょうか.

(1) CPU 使用率がはねあがる
(2) プロセスが死なない

 (1) が問題というのは理解できます.あまり文句がないんで放置している部分
です.直そうと思えば,正しく他のスレッドを待ち合わせをすればできると思っ
ています.

 (2) については仕様です.ご提案の修正方法では,とにかく生きていたら殺
す,ということをしていますが,例えば他のスレッドで何か時間のかかる終了処
理をしていた場合(例えば,終了時にネットワークで外部に情報を送る,という
ようなことをしている場合),それを強制的に止めてしまうため,まずいことに
なります.

 ということで,これに関しては,現状だと仕様かな,と思いますが,もっと良
い仕様があれば,ご提案頂けと助かります.

--
// SASADA Koichi at atdot dot net

#3 Updated by Anonymous over 2 years ago

高尾と申します。

 (2) については仕様です.ご提案の修正方法では,とにかく生きていたら殺
す,ということをしていますが,例えば他のスレッドで何か時間のかかる終了処
理をしていた場合(例えば,終了時にネットワークで外部に情報を送る,という
ようなことをしている場合),それを強制的に止めてしまうため,まずいことに
なります.

これは「ensure節の実行中に止めるのはNG」ということですよね?
いまの実装 (rev.33339) では、ensure節の実行中であっても、terminateされたら
(外側にさらなるensure節がなければ) ふつうに終了するので、辻褄が合わない気がします。

#4 Updated by Koichi Sasada over 2 years ago

(2011/09/26 7:03), m_takao wrote:

これは「ensure節の実行中に止めるのはNG」ということですよね?
いまの実装 (rev.33339) では、ensure節の実行中であっても、terminateされたら
(外側にさらなるensure節がなければ) ふつうに終了するので、辻褄が合わない気がします。

 ご指摘の通り,「最初の1回だけは必ず ensure 中でも強制的に止めてしま
う」という仕様になっていますね.現状では,ensure 中(後処理中)ではない
ことを期待しています.ファイナライザ実行中でも,強制的にキャンセルされる
ので,何か後処理をしている最中に止められるのは不可避です.

# 今まで文句が無かったというのは,確率的にこういうケースが起きづらい,
# ということでしょうかね.

 真面目にやるなら,すでに何度か提案がある,こういう割り込みを制御するた
めの仕組みを統一的に整備するってことになると思います.うーん,せめてファ
イナライザの処理は main thread のみで行うようにしたほうがいいんだろうか.

 もしくは,こういう処理を行うことがあるのなら,ちゃんと join してから死
にましょう,と開き直って,最初のご提案の通り,確実に殺す,というふうに振
るのかなぁ.それはそれで不便そうに思うのだけど.

 結論も出てませんが,現状はなんとなくうまく動くことが多い,改善するには
しんどそう,ということで,なんとなく現状の仕様になっている,ということか
と思います.

 もしうまい手があれば教えてください.

--
// SASADA Koichi at atdot dot net

#5 Updated by Nobuyoshi Nakada over 2 years ago

=begin
なかだです。

At Mon, 26 Sep 2011 23:52:19 +0900,
SASADA Koichi wrote in :

(2011/09/26 7:03), m_takao wrote:

これは「ensure節の実行中に止めるのはNG」ということですよね?
いまの実装 (rev.33339) では、ensure節の実行中であっても、terminateされたら
(外側にさらなるensure節がなければ) ふつうに終了するので、辻褄が合わない気がします。

 ご指摘の通り,「最初の1回だけは必ず ensure 中でも強制的に止めてしま
う」という仕様になっていますね.現状では,ensure 中(後処理中)ではない
ことを期待しています.ファイナライザ実行中でも,強制的にキャンセルされる
ので,何か後処理をしている最中に止められるのは不可避です.

ensureでsleepされたら止まるのは仕方がないにせよ、そこで割り込みを受けた
らやはり抜けるほうがいいのではないでしょうか。

diff --git i/thread.c w/thread.c
index d9d497a..0e3d096 100644
--- i/thread.c
+++ w/thread.c
@@ -333,6 +333,7 @@ typedef struct rbmutexstruct

static void rbmutexabandonall(rbmutext mutexes);
static const char
rb
mutexunlockth(rbmutext *mutex, rbthreadt volatile *th);
+static int vmlivingthreadnum(rbvm_t *vm);

void
rbthreadptrunlockalllockingmutexes(rbthreadt *th)
@@ -369,14 +370,20 @@ rb
threadterminateall(void)
stforeach(vm->livingthreads, terminatei, (stdatat)th);
vm->inhibit
thread_creation = 1;

  • while (!rbthreadalone()) { +
  • if (th->vm->livingthreads && vmlivingthreadnum(th->vm) > 1) {
  • int state;
  • const double sleep_time = 3600.0; /* should not be too long to
  • * get rid of overflow */ PUSH_TAG();
  • if (EXEC_TAG() == 0) {
  • rbthreadschedule();
  • }
  • else {
  • /* ignore exception */
  • state = EXEC_TAG();
  • while (state == 0 || !rbthreadalone()) {
  • if (state) {
  • stforeach(vm->livingthreads, terminatei, (stdata_t)th);
  • }
  • sleepwaitforinterrupt(th, sleeptime); }
  • /* ignore exception */
    POPTAG();
    }
    }
    @@ -445,6 +452,8 @@ thread
    startfunc2(rbthreadt *th, VALUE *stackstart, VALUE *registerstack_s

    gvl_acquire(th->vm, th);
    {

  • int terminating = 0;
    +
    threaddebug("thread start (get lock): %p\n", (void *)th);
    rb
    threadsetcurrent(th);

    @@ -469,6 +478,7 @@ threadstartfunc2(rbthreadt *th, VALUE *stackstart, VALUE registerstacks
    if (NILP(errinfo)) errinfo = rberrinfo();
    if (state == TAG_FATAL) {
    /
    fatal error within this thread, need to stop whole script */

  •  terminating = (errinfo == eTerminateSignal);
    }
    else if (rb_obj_is_kind_of(errinfo, rb_eSystemExit)) {
    if (th->safe_level >= 4) {
    

    @@ -509,13 +519,23 @@ threadstartfunc2(rbthreadt *th, VALUE *stackstart, VALUE registerstacks
    /
    delete self other than main thread from livingthreads */
    if (th != main
    th) {
    stdeletewrap(th->vm->living_threads, th->self);

  •  if (terminating) {
    
  •  if (vm_living_thread_num(th->vm) == 1) {
    
  •      rb_threadptr_interrupt(main_th);
    
  •  }
    
  •  else {
    
  •      terminating = 0;
    
  •  }
    
  •  }
    

    }

    /* wake up joining threads */
    jointh = th->joinlisthead;
    while (join
    th) {

  •  if (join_th == main_th) errinfo = Qnil;
    
  •  rb_threadptr_interrupt(join_th);
    
  •  if ((join_th != main_th) || (errinfo = Qnil, !terminating)) {
    
  •  /* main thread is already interrupted when terminating */
    
  •  rb_threadptr_interrupt(join_th);
    
  •  }
    switch (join_th->status) {
      case THREAD_STOPPED: case THREAD_STOPPED_FOREVER:
    join_th->status = THREAD_RUNNABLE;
    

    --- 僕の前にBugはない。
    --- 僕の後ろにBugはできる。
    中田 伸悦
    =end

#6 Updated by Motohiro KOSAKI over 2 years ago

2011年9月29日20:20 Nobuyoshi Nakada nobu@ruby-lang.org:

なかだです。

At Mon, 26 Sep 2011 23:52:19 +0900,
SASADA Koichi wrote in :

(2011/09/26 7:03), m_takao wrote:

これは「ensure節の実行中に止めるのはNG」ということですよね?
いまの実装 (rev.33339) では、ensure節の実行中であっても、terminateされたら
(外側にさらなるensure節がなければ) ふつうに終了するので、辻褄が合わない気がします。

  ご指摘の通り,「最初の1回だけは必ず ensure 中でも強制的に止めてしま
う」という仕様になっていますね.現状では,ensure 中(後処理中)ではない
ことを期待しています.ファイナライザ実行中でも,強制的にキャンセルされる
ので,何か後処理をしている最中に止められるのは不可避です.

ensureでsleepされたら止まるのは仕方がないにせよ、そこで割り込みを受けた
らやはり抜けるほうがいいのではないでしょうか。

現在のensure中の非同期例外の扱いは仕様バグであるという認識なので、こういう
ワークアラウンドにはあまりいい印象を持ちません。
実用的にも問題があると思っていて、C-c を日常的に2−3回連打する癖のあるひとは
いるので、そういう人からするとプロセス終了処理がちゃんと行われなくなるregression
に見えるのではないでしょうか。

#7 Updated by Shyouhei Urabe over 2 years ago

提案されている workaround はいまいち、という点には同意します。

ただ、じゃあどうなるべきなの? というところのコンセンサスは足りていないでしょう。

というわけで結論を急がずに少し議論したほうがいいんじゃないでしょうか。

#8 Updated by Koichi Sasada about 2 years ago

  • Assignee set to Koichi Sasada

今日,非同期例外の話が出たので,それと交えて考えます.
多分.

#9 Updated by Shyouhei Urabe about 2 years ago

  • Status changed from Open to Assigned

#10 Updated by Koichi Sasada over 1 year ago

  • Assignee changed from Koichi Sasada to Motohiro KOSAKI
  • Priority changed from Normal to High

ticket の詳細が思い出せないので小崎先生に振ってみます.
control_interrupt じゃ解決しないんだよな,多分.

#11 Updated by Motohiro KOSAKI over 1 year ago

読み返しました。[Feature #1952] とちょっと似た話で終了途中で例外食われてしまったら、という話のようですね。論点をまとめると

  • ensure節の中でsleep等、無限待ちが記述されているとハングしてしまう。これはバグではないか (Glass_saga)
  • この場合止まっているのはサブスレッドなので、Ctrl-cは意味が無い。メインスレッドが食って無視してしまう
  • しかし、タイムアウト等をつくると終了処理に時間のかかるスクリプトが壊れてしまう (ko1)
  • ensure節実行中にterminateされたらensure抜けてしまうので言語仕様としてつじつまがあってない (高尾さん)
  • sleepで寝ていても、もう一度例外が上がってきたら抜けるべきでは? (← これよくわからないが Ctrl-Cうけたらメインスレッドがthread terminateを再送しろということ?)
  • Ctrl-c 二連打で、終了処理がスッポ抜けるのって本当にうれしい?(こさき)
  • 現在、main threadがsub threadの終了を待つロジックがビジーループなのでCPU100%になってしまう。スレッドが終了するまでちゃんと寝るべきではないのか (やや脱線)

ぐらいですかね。

#12 Updated by Motohiro KOSAKI over 1 year ago

なお、#1952 ですでに指摘されているように Ctrl-c が押された時にサブスレッドを待たずに終了してしまうという案はSEGVを引き起こすのでNG。

#13 Updated by Motohiro KOSAKI over 1 year ago

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

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


  • thread.c (rbthreadterminateall): use nativesleep() instead of rbthreadschedule(). Otherwise, it consume 100% cpu meaninglessly. [Bug #5368]
  • thread.c (threadstartfunc_2): last sub-thread wakes up main thread.

#14 Updated by Motohiro KOSAKI over 1 year ago

  • Status changed from Closed to Assigned
  • Assignee changed from Motohiro KOSAKI to Yukihiro Matsumoto
  • Priority changed from High to Normal

現状、ささださんが #1にてCPU使用率が跳ね上がるのだけがバグで、終わらない、かつCtrl-Cも効かなくなるのは仕様という見解を出しているのでそれにそって、r37865でCPU使用率問題を直しました。
さて、このまま閉じてしまっていいのかまったく分からないので、まつもとさん意見をください。

元の起票バグだとensureで無限sleepというちょっとありえなさそうなスクリプトですが、IO.read などでも同様の「終了しない+Ctrl-C効かない」が起こります。これはCtrl-Cが絶対メインスレッドに飛ぶのでサブスレッドが起きれないから。それでも構わないということであれば closeしてください。

だめだと思っている場合は、どのような動作がいいと思っているか教えてください

[Feature #1952] の #12でmameさんが選択肢をいくつか列挙してくれていて

どうしても直したいならば、

  • サブスレッドの終了待ち状態で SIGINT を受け取ったら、
    eTerminateSignal を再送する

    • しつこく Ctrl+C を押していればいつか終了できる、かも
  • eTerminateSignal を捕捉できない例外とする

    • サブスレッドの ensure が実行されない
  • eTerminateSignal を投げて数秒しても終わってくれない場合、
    捕捉できない例外を投げる

    • サブスレッドの ensure が実行されない危険が緩和されるが 本質的に解決はしない。あとダサい

くらいを思いつきましたが、どれも問題がある or 面倒ですね。

とかいうコメントがついています。[Feature #1952]全体を一度読みなおしてからコメントいただけるとなおありがたい

#15 Updated by Motohiro KOSAKI over 1 year ago

1.8 だと、Ctrl-C でsleepを抜けてくれるようです。うーん、この挙動のほうがいいんかなあ。1.8がどういう理屈でこう動いているのかよくわからないんだけど、ようするにInterruptがmain threadではなく、たまたまその時動いていたThreadに飛ぶようなシロモノだったということだろうか(推測)

#16 Updated by Motohiro KOSAKI over 1 year ago

  • % Done changed from 100 to 50

#17 Updated by Masaya Tarui over 1 year ago

1.8の挙動の方がバグっぽいですけれどねぇ
Ctrl-Cでensureの実行が保証されず、終わってしまうのは大きな問題だと思います。
現にTimeoutでensureが飛ばされるので困ってcontrol_interruptをなどを導入しようと
しているのだと理解しています。kill -KILLしないと死なないプロセスは良くある事で、
これはensureを必ず実行しようとする事とのトレードオフだと思います。

デフォルトでの挙動ではCtrl-CでIntrrupt例外を再送するようにして、
ensure節中ではIntrrupt例外に対して:on_blockingにしておいてやると、
ユーザーが望めば:neverにする事が出来て実行が保証でき、かつ普通にensureを書いてる分には
Ctrl-C連打で上記sleepやio待ちもキャンセル出来て良いんじゃないでしょうか?
(:immediateだと設定を変えようとしてる間に例外が飛んで来て飛ばされる可能性があるので)

#18 Updated by Motohiro KOSAKI over 1 year ago

[Feature #1952]の対策として、r37875でCtrl-Cが押されるたびに eTerminateSignalを最送出するようにしたので、現状1.8と同等の動きをするようになってます。

#19 Updated by Motohiro KOSAKI over 1 year ago

1.8と同等と書いたのは

Thread.new do
begin
sleep
ensure
sleep
end
end

というプログラムが一度ensure節のsleepで寝てしまうが、Ctrl-Cにより終わることは出来る。という挙動を指しており、
このスクリプト以外の細かい挙動について、不整合はあると思います。

#20 Updated by Motohiro KOSAKI over 1 year ago

なお、#1952 ですでに指摘されているように Ctrl-c が押された時にサブスレッドを待たずに終了してしまうという案はSEGVを引き起こすのでNG。

もうちょっとまじめに書くと、SEGVを引き起こすのは終了処理でまじめに1つ1つリソース解放処理をしてるからで、いきなりexit()するという選択肢はあるだろう。しかしそれは当然MVMにしたときに問題を引き起こす。
SEGVしないように各所にNULLチェックを入れるという案については、pthreadconddestruct()が別スレッドがリソース使用中に呼び出されるとEBUSYで失敗するためうまくいかない

#21 Updated by Koichi Sasada about 1 year ago

  • Assignee changed from Yukihiro Matsumoto to Motohiro KOSAKI
  • Target version changed from 2.0.0 to 2.1.0

2.1 で結論を付けたいところ。
多分、まつもとさんはこの辺気にしないと思うので、小崎さん、たるいさん(と私かなぁ)で決めると良いと思います。

#22 Updated by Hiroshi SHIBATA 3 months ago

  • Target version changed from 2.1.0 to current: 2.2.0

Also available in: Atom PDF