Bug #5993

Thread.new{ Fiber.new { Thread.exit }.resume }.join で例外

Added by Tomoyuki Chikanaga about 3 years ago. Updated about 2 years ago.

[ruby-dev:45218]
Status:Closed
Priority:Normal
Assignee:Tomoyuki Chikanaga
ruby -v:ruby 2.0.0dev (2012-02-09 trunk 34514) [x86_64-darwin10.8.0] Backport:

Description

以下のように Fiber 内で Thread.exit するとメッセージが空の RuntimeError が発生します。

Thread.new{ Fiber.new { Thread.exit }.resume }.join #=> RuntimeError:

rb_fiber_start() で Thread.exit 時の TAG_FATAL での TAG_JUMP を想定していないためだと思います。とりあえず以下のようにすると例外にならなくなります。
あと th->errinfo は空を Qnil としているのに th->thrown_errinfo は 0 (Qfalse)を空であることを示すのに使っているので、その食い違いで thrown_errinfo に Qnil を入れてしまっていた(rb_vm_make_jump_tag_but_local_jump() の結果が Qnil の時)のが原因のようなので、そちらをなんとかすべきかもしれません。全体的に thrown_errinfo も空を意味するために Qnil を使うようにそろえるとか?

--- a/cont.c
+++ b/cont.c
@@ -1152,6 +1152,9 @@ rb_fiber_start(void)
if (state == TAG_RAISE) {
th->thrown_errinfo = th->errinfo;
}
+ else if (state == TAG_FATAL && th->errinfo == INT2FIX(TAG_FATAL)) {
+ /* terminating */
+ }
else {
th->thrown_errinfo =
rb_vm_make_jump_tag_but_local_jump(state, th->errinfo);

thread_exit_in_fiber.patch Magnifier (1.93 KB) Tomoyuki Chikanaga, 12/19/2012 02:31 AM


Related issues

Related to Ruby trunk - Bug #6575: Thread#kill sets rb_errinfo() to Fixnum 8 after rb_protec... Rejected 06/11/2012

Associated revisions

Revision 38414
Added by Tomoyuki Chikanaga about 2 years ago

  • cont.c (rb_fiber_start): don't enqueue Qnil to async_errinfo_queue.
    rb_vm_make_jump_tag_but_local_jump() could return Qnil (ex. when
    finished by Thread.exit). [Bug #5993]

  • test/ruby/test_fiber.rb (test_exit_in_fiber): add test for it.

Revision 38414
Added by Tomoyuki Chikanaga about 2 years ago

  • cont.c (rb_fiber_start): don't enqueue Qnil to async_errinfo_queue.
    rb_vm_make_jump_tag_but_local_jump() could return Qnil (ex. when
    finished by Thread.exit). [Bug #5993]

  • test/ruby/test_fiber.rb (test_exit_in_fiber): add test for it.

Revision 38550
Added by Tomoyuki Chikanaga about 2 years ago

  • cont.c (rb_fiber_start): in case of jump with TAG_FATAL,
    enqueue error into async_errinfo_queue, because you cannot call
    TH_TAG_JUMP() in this function. [Bug #5993]

  • thread.c (rb_threadptr_execute_interrupts): now INT2FIX(TAG_FATAL)
    can be popped from async_errinfo_queue.

  • vm.c (rb_vm_make_jump_tag_but_local_jump): revert r38441.
    rb_vm_make_jump_tag_but_local_jump() shouldn't return exception
    in case of state == TAG_FATAL.

  • test/ruby/test_fiber.rb (test_exit_in_fiber): fix a test to illuminate
    Thread.exit should terminate current Thread.

Revision 38550
Added by Tomoyuki Chikanaga about 2 years ago

  • cont.c (rb_fiber_start): in case of jump with TAG_FATAL,
    enqueue error into async_errinfo_queue, because you cannot call
    TH_TAG_JUMP() in this function. [Bug #5993]

  • thread.c (rb_threadptr_execute_interrupts): now INT2FIX(TAG_FATAL)
    can be popped from async_errinfo_queue.

  • vm.c (rb_vm_make_jump_tag_but_local_jump): revert r38441.
    rb_vm_make_jump_tag_but_local_jump() shouldn't return exception
    in case of state == TAG_FATAL.

  • test/ruby/test_fiber.rb (test_exit_in_fiber): fix a test to illuminate
    Thread.exit should terminate current Thread.

History

#1 Updated by Shyouhei Urabe almost 3 years ago

  • Status changed from Open to Assigned

#2 Updated by Koichi Sasada almost 3 years ago

ちょっと思い出せないんですが(多分、その辺整理すると思う)、
近永さんのことだから信じられると思います。

というわけで、テスト付きでコミットいただければ。

#3 Updated by Tomoyuki Chikanaga almost 3 years ago

長らく放置しててすみません。

先のパッチをテストしていて、これだけでは不十分で確か Fiber 内で fatal() を呼んだりした時の挙動に問題があったためもう少し考えないといけないなぁ、というところで止まってたと思います。
ちょっとどうやら作業していたブランチを消してしまったみたいで、すぐには具体的な内容を思い出せないのですが、少し調べてみてこの修正とは別の問題として切り分けられそうであれば別チケットを作ろうかと思います。

#4 Updated by Tomoyuki Chikanaga about 2 years ago

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

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


  • cont.c (rb_fiber_start): don't enqueue Qnil to async_errinfo_queue.
    rb_vm_make_jump_tag_but_local_jump() could return Qnil (ex. when
    finished by Thread.exit). [Bug #5993]

  • test/ruby/test_fiber.rb (test_exit_in_fiber): add test for it.

#5 Updated by Tomoyuki Chikanaga about 2 years ago

Fiber内で rb_fatal() を呼んだ時の問題は http://bugs.ruby-lang.org/issues/7570 として新しくチケットを作成しました。

#6 Updated by Tomoyuki Chikanaga about 2 years ago

  • Status changed from Closed to Open

すみません、やっぱり r38414 ではちゃんと修正できていませんでした。
例外は発生しなくなりましたが、Thread.exit が Fiber を終了したところで無視されて Thread は動き続けてしまっていました。

Thread.start{
Fiber.new{ Thread.exit }.resume
p :unreachable # <- 実行されてしまう
}.join

async_errinfo_queue に INT2FIX(TAG_FATAL) を入れて伝播させないといけないですかね。

#7 Updated by Tomoyuki Chikanaga about 2 years ago

パッチを作り直しました。

rb_threadptr_execute_interrupts() で async_errinfo_queue から INT2FIX(TAG_FATAL) が取れる可能性を考慮するようにしました。通常は INT2FIX(TAG_FATAL) は rb_threadptr_to_kill() で発生させて同期的例外のように処理するので async_errinfo_queue に入ることはないはずですが、Fiberの終了時は fiber_terminate()して Fiber を切り替えるまで遅延させるために例外的に async_errinfo_queue に入ることがあります。というか入れるようにしました。

そのかわり r38441 で rb_vm_make_jump_tag_but_local_jump() で TAG_FATAL を考慮するようにした変更は revert しています。

手元では make check は通りました。よさそうだったらコミットします。

#8 Updated by Tomoyuki Chikanaga about 2 years ago

  • Status changed from Assigned to Closed

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


  • cont.c (rb_fiber_start): in case of jump with TAG_FATAL,
    enqueue error into async_errinfo_queue, because you cannot call
    TH_TAG_JUMP() in this function. [Bug #5993]

  • thread.c (rb_threadptr_execute_interrupts): now INT2FIX(TAG_FATAL)
    can be popped from async_errinfo_queue.

  • vm.c (rb_vm_make_jump_tag_but_local_jump): revert r38441.
    rb_vm_make_jump_tag_but_local_jump() shouldn't return exception
    in case of state == TAG_FATAL.

  • test/ruby/test_fiber.rb (test_exit_in_fiber): fix a test to illuminate
    Thread.exit should terminate current Thread.

Also available in: Atom PDF