Bug #595: Fiber ignores ensure clause

• Target version set to 1.9.1 Release Candidate

Help me out--shouldn't this print out only when you call fib.resume twice?

fib = Fiber.new{
  begin
    Fiber.yield :ok
  ensure
    puts "should be print out"
  end
}
p fib.resume
p fib.resume


prints out all right.

• Target version changed from 1.9.1 Release Candidate to 2.0.0

パッチを書くか、もしくはドキュメントにKNOWN BUGとして書く、ということで。

ワナベと申します。

かなり前のチケットですが、題名の件についてパッチを書きました。
もしまだどなたもパッチを書かれていないようならご検討ください。

Index: thread.c
===================================================================
--- thread.c	(リビジョン 23617)
+++ thread.c	(作業コピー)
@@ -293,6 +293,8 @@

 static void rb_mutex_unlock_all(mutex_t *mutex, rb_thread_t *th);

+void rb_fiber_terminate_all(rb_thread_t *th);
+
 void
 rb_thread_terminate_all(void)
 {
@@ -310,6 +312,7 @@

     thread_debug("rb_thread_terminate_all (main thread: %p)\n", (void *)th);
     st_foreach(vm->living_threads, terminate_i, (st_data_t)th);
+    rb_fiber_terminate_all(th);

     while (!rb_thread_alone()) {
 	PUSH_TAG();
@@ -1210,6 +1213,7 @@
 	    thread_debug("rb_thread_execute_interrupts: %ld\n", err);

 	    if (err == eKillSignal || err == eTerminateSignal) {
+		rb_fiber_terminate_all(th);
 		th->errinfo = INT2FIX(TAG_FATAL);
 		TH_JUMP_TAG(th, TAG_FATAL);
 	    }
Index: cont.c
===================================================================
--- cont.c	(リビジョン 23617)
+++ cont.c	(作業コピー)
@@ -534,6 +534,7 @@
       case 0:
 	return Qnil;
       case 1:
+      case -1:
 	return argv[0];
       default:
 	return rb_ary_new4(argc, argv);
@@ -946,6 +947,36 @@
     return fib->status != TERMINATED ? Qtrue : Qfalse;
 }

+static VALUE
+terminate_all_i(VALUE fibval)
+{
+    if (rb_fiber_alive_p(fibval)) {
+	VALUE value = rb_exc_new2(rb_eSystemExit, "terminate");
+	return fiber_switch(fibval, -1, &value, 0);
+    }
+}
+
+void
+rb_fiber_terminate_all(rb_thread_t *th)
+{
+    VALUE fibval;
+    rb_fiber_t *fib, *root_fib;
+    rb_thread_t *_th = GET_THREAD();
+
+    rb_thread_set_current(th);
+    fibval = th->root_fiber;
+    if (!RTEST(fibval)) return;
+    GetFiberPtr(fibval, root_fib);
+
+    fib = root_fib->prev_fiber;
+    while (fib != root_fib) {
+	rb_rescue2(terminate_all_i, fib->cont.self,
+		   0, 0, rb_eSystemExit);
+	fib = fib->prev_fiber;
+    }
+    rb_thread_set_current(_th);
+}
+
 /*
  *  call-seq:
  *     fiber.resume(args, ...) -> obj


-- 
ワナベ

　ささだです．

　返事が随分遅くなってしまってすみません．

wanabe wrote::
> かなり前のチケットですが、題名の件についてパッチを書きました。
> もしまだどなたもパッチを書かれていないようならご検討ください。

　実は，以前似たようなものを作ったのですが，さくっと SEGV の嵐で，面倒く
さいなぁ，と思って放置していたのでした．このパッチですと，たとえば
test-all とかはどうでした？

-- 
// SASADA Koichi at atdot dot net

ワナベです。

2009/06/15 6:33 に SASADA Koichi<[e-mail]> さんは書きました:
> wanabe wrote::
>> かなり前のチケットですが、題名の件についてパッチを書きました。
>> もしまだどなたもパッチを書かれていないようならご検討ください。
>
> 実は，以前似たようなものを作ったのですが，さくっと SEGV の嵐で，面倒く
> さいなぁ，と思って放置していたのでした．このパッチですと，たとえば
> test-all とかはどうでした？

ruby 1.9.2dev (2009-06-14 trunk 23691) [i386-mingw32] では
make test-all で SEGV が出てしまいました。
TestFiber#test_many_fibers_with_threads が E で終わった直後の
TestFiber#test_normal で落ちているようです。
ですがなぜか test_fiber.rb を直接起動するとエラーなしで完走します。

またパッチはLinux環境で書いたのですが、その時のmake test-allでは問題はなく、
今改めて ruby 1.9.2dev (2009-06-15 trunk 23692) [i686-linux] で試してみても
SEGV は発生しませんでした。

どういう事かよく分かりませんが、不安定であることは間違いないので
このパッチは役に立たなさそうです。申し訳ありません。

-- 
ワナベ

　ささだです．

wanabe wrote::
> ruby 1.9.2dev (2009-06-14 trunk 23691) [i386-mingw32] では
> make test-all で SEGV が出てしまいました。
> TestFiber#test_many_fibers_with_threads が E で終わった直後の
> TestFiber#test_normal で落ちているようです。
> ですがなぜか test_fiber.rb を直接起動するとエラーなしで完走します。
> 
> またパッチはLinux環境で書いたのですが、その時のmake test-allでは問題はなく、
> 今改めて ruby 1.9.2dev (2009-06-15 trunk 23692) [i686-linux] で試してみても
> SEGV は発生しませんでした。
> 
> どういう事かよく分かりませんが、不安定であることは間違いないので
> このパッチは役に立たなさそうです。申し訳ありません。

　いえいえ．もうちょっと追跡すればなんとかなると思うのですが，ここはどう
にも難しいですよね．Fiber の集合を保存する，私が最後にあわてて突っ込んだ
部分にバグがあるような気がしています．

　1.9.2 には間に合わせたいところ．

-- 
// SASADA Koichi at atdot dot net

• File ensure_fiber.patch added
• ruby -v set to ruby 1.9.2dev (2010-01-13) [i386-mingw32]

以前このチケットについて書いたパッチが SEGV すると書きましたが、
パッチの問題ではなく Bug #1325 の影響だったようです。
改めて確認したところ make test-all では SEGV しませんでした。

それとは別の問題として、例外でジャンプさせると rescue される恐れがあったので
throw/catch を使うように変更したパッチを添付します。
ご検討くだされば幸いです。

• File deleted (ensure_fiber.patch)

ささださん

このチケットおよびパッチについてコメントいただければ幸いです。
この対処で問題ないかどうか、もし問題ないとしたら 
1.9.2 リリース前に入れるべきかどうかが気になっています。
そもそもアプローチがまずいようでしたら別の方法を考えます。

遠藤です。

2010年4月10日23:20 _ wanabe <[e-mail]>:
> ささださん
>
> このチケットおよびパッチについてコメントいただければ幸いです。
> この対処で問題ないかどうか、もし問題ないとしたら
> 1.9.2 リリース前に入れるべきかどうかが気になっています。
> そもそもアプローチがまずいようでしたら別の方法を考えます。

ささださんではないですが勝手にコメントします。


1) プロセス終了時でなく、Fiber が GC で回収される時に ensure 節が
実行されません。

  1000.times do
    Fiber.new do
      begin
        Fiber.yield
      ensure
        puts "foo!"
      end
    end.resume
    GC.start
  end

  # foo! が 1 個しか出ない (期待は 1000 個)
  $ ./ruby t.rb
  foo!

実行中の Thread と同様に、yield 中の Fiber は GC しないようにする
くらいしか思いつきません。


2) Fiber を作ったスレッドが終了した場合、Fiber の ensure 節が実行
されません。

  Thread.new do
    Fiber.new do
      begin
        Fiber.yield
      ensure
        puts "foo!"
      end
    end.resume
  end
  sleep 1
  puts "end"

  # foo! が出ない
  $ ./ruby t.rb
  end

プロセスの終了時でなく、スレッドの終了時に Fiber を起こさないと
行けない？


どちらも仕様レベルで検討しないといけないことのような気がします。
まだ結構大変そうなので、1.9.2 は見送った方がいいんじゃないかなと
思います。早くコメントしていれば、wanabe さんなら余裕で直せたと
思うので残念ですが。

-- 
Yusuke Endoh <[e-mail]>

(2010/04/21 23:55), Yusuke ENDOH wrote::
> どちらも仕様レベルで検討しないといけないことのような気がします。
> まだ結構大変そうなので、1.9.2 は見送った方がいいんじゃないかなと
> 思います。早くコメントしていれば、wanabe さんなら余裕で直せたと
> 思うので残念ですが。

　返事が出来ていなくてすみません．最近，色々と余裕がなくて，じっくり考え
て返事が出来ず．

-- 
// SASADA Koichi at atdot dot net

• Status changed from Open to Assigned

未だに考え中ですが，これは 1.9.3 には仕様変更になるので入りませんよね？

• ruby -v changed from ruby 1.9.2dev (2010-01-13) [i386-mingw32] to -

まつもと ゆきひろです In message "Re: [ruby-dev:43715] [Ruby 1.9 - Bug #595] Fiber ignores ensure clause" on Sat, 11 Jun 2011 14:52:03 +0900, Koichi Sasada <[e-mail]> writes: |未だに考え中ですが，これは 1.9.3 には仕様変更になるので入りませんよね？ この動作はバグだと考えているので、Yuguiさんが拒絶しない限り、 直せるのであれば1.9.3で直せばよいと思います。間に合わないな らしょうがない。

RubySpec 的にバグ扱いになってますね。 core/fiber/resume_spec.rb

• Target version changed from 2.0.0 to 1.9.3

• Status changed from Assigned to Closed

• Status changed from Closed to Assigned
• Target version changed from 1.9.3 to 3.0