Bug #8749: Readline.readline stops STDOUT?

• Status: Closed
• Assignee: kouji (Kouji Takao)
• Target version: -
• ruby -v: ruby 2.1.0dev (2013-08-06 trunk 42402) [x86_64-linux]
• Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN

[no6v (Nobuhiro IMAI)]

=begin
r42402 で、以下のようなスクリプトの挙動が変わっています。
(Enter を押すかどうかは関係なくて、)Readline.readline を実行中に標準出力への出力が出来なくなっているように見えます。

$ cat rl.rb
require "readline"

th = Thread.new do
p Readline.readline("> ")
end

loop do
p :hi
sleep 2
break unless th.alive?
end
$ ruby -v rl.rb
ruby 2.1.0dev (2013-08-06 trunk 42401) [x86_64-linux]
:hi

:hi
:hi
:hi
:hi
# しばらく放置してここで Enter
""
$ ruby -v rl.rb
$ /tmp/ruby/bin/ruby -v /tmp/r.rb
ruby 2.1.0dev (2013-08-06 trunk 42402) [x86_64-linux]
:hi
# しばらく放置してここで Enter
""
$
=end

[akr (Akira Tanaka)]

• File readline-release-gvl.patch readline-release-gvl.patch added

何通か書いたメールが redmine に登録されていないので redmine で書きます。
(メールは (化けてますが) https://www.ruby-forum.com/topic/4416224 で見れます。)

読み込みでブロックしている時に GVL を解放していなくて他のスレッドが動けないようです。

readline_getc は Ruby 1.8 時代の名残りか、
Ruby の IO 読み込み関数 (rb_io_getbyte) を使うことによって
読み込みでブロックしている最中も他のスレッドを動かせるようにしていますが、
それには IO オブジェクトが必要なため、IO オブジェクトがないときは無理だとか
中途半端です。

その判断のところが r42402 で変化してしまって、他のスレッドが動けなくなっていた
のですが、Ruby 1.9 以降では、GVL を外して読み込めばブロック中に
他のスレッドは動けるのでそうするのがいいんじゃないでしょうか。

readline-release-gvl.patch みたいなのはどうですかね。

なお、シグナルがきたときに
rb_thread_call_with_gvl 経由で rb_thread_check_ints を呼んでいますが、
rb_thread_interrupted を使うと設定した trap が即座には起動しなかったためです。
これは私の想定する rb_thread_interrupted の動作と違うんですが、私の理解と
rb_thread_interrupted のどちらが間違っているのかは分かりません。

[kouji (Kouji Takao)]

• Assignee set to kouji (Kouji Takao)

[no6v (Nobuhiro IMAI)]

読み込みでブロックしている時に GVL を解放していなくて他のスレッドが動けないようです。

ありがとうございます。出力じゃなくメインスレッド自体が止まっていたんですね。

その判断のところが r42402 で変化してしまって、他のスレッドが動けなくなっていた
のですが、Ruby 1.9 以降では、GVL を外して読み込めばブロック中に
他のスレッドは動けるのでそうするのがいいんじゃないでしょうか。

readline-release-gvl.patch みたいなのはどうですかね。

今回問題にした挙動は r42525 で一旦直って r42527 でまた壊れはしたものの
r42528 でまた直っているようです。これはたまたまでしょうか。

もちろん readline-release-gvl.patch を当てても動きます。
(コンパイル時に以下の警告が出ました)

compiling ../../../../git/ruby/ext/readline/readline.c
../../../../git/ruby/ext/readline/readline.c: In function ‘check_ints’:
../../../../git/ruby/ext/readline/readline.c:148:1: warning: no return statement in function returning non-void [-Wreturn-type]
}
^

[akr (Akira Tanaka)]

• File readline-release-gvl-2.patch readline-release-gvl-2.patch added

no6v (Nobuhiro IMAI) wrote:

今回問題にした挙動は r42525 で一旦直って r42527 でまた壊れはしたものの
r42528 でまた直っているようです。これはたまたまでしょうか。

readline_getc で、rl_getc を呼び出すか、rb_io_getbyte を呼び出すかという条件判断が問題で、
前者なら GVL を離さないので他のスレッドは動けず、後者なら動けます。

r42525 で直ったのは、ifp->stdio_file に代入したので、readline_getc 内の
if (rl_instream != ifp->stdio_file) return rl_getc(input);
という条件が成立しなくなったためでしょう。
しかし、[ruby-dev:47608] で述べたように、ifp->stdio_file に readline 側で
作った FILE 構造体を代入すると、IO#close で解放されてしまって、ひいては SEGV になる可能性が出てきます。

r42527 は if 文で上記の ifp->stdio_file への代入をガードしていますが、
[ruby-dev:47609] で述べたように、その条件は決して成り立たないので、実質、
r42525 の revert のようなものです。

r42528 は
if (rl_instream != ifp->stdio_file) return rl_getc(input);
という文を削除していて、そのため、rl_getc じゃなくて下のほうに制御が行って、
rb_io_getbyte を呼び出すようになったのでしょう。
ここで rb_io_getbyte は readline_instream という IO オブジェクトから読むので、
それが input という FILE 構造体と対応しているのかという疑問があるわけで、
だからこそ対応していなかったら input から読むという処理が入っているのだと思いますが、
どういう理由かは知りませんが、消してしまっているようです。

もちろん readline-release-gvl.patch を当てても動きます。
(コンパイル時に以下の警告が出ました)

compiling ../../../../git/ruby/ext/readline/readline.c
../../../../git/ruby/ext/readline/readline.c: In function ‘check_ints’:
../../../../git/ruby/ext/readline/readline.c:148:1: warning: no return statement in function returning non-void [-Wreturn-type]
}
^

おっと、return NULL; を忘れていました。
つけたのを readline-release-gvl-2.patch として添付します。

[akr (Akira Tanaka)]

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

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

---

• ext/readline/readline.c: Include ruby/thread.h for
  rb_thread_call_without_gvl2.
  (readline_rl_instream, readline_rl_outstream): Record FILE
  structures allocated by this extension.
  (getc_body): New function extracted from readline_getc.
  (getc_func): New function.
  (readline_getc): Use rb_thread_call_without_gvl2 to invoke getc_func.
  [ruby-dev:47033] [Bug #8749]
  (clear_rl_instream, clear_rl_outstream): Close FILE structure
  allocated by this extention reliably. [ruby-core:57951] [Bug #9040]
  (readline_readline): Use clear_rl_instream and clear_rl_outstream.
  (readline_s_set_input): Set readline_rl_instream.
  (readline_s_set_output): Set readline_rl_outstream.
  (Init_readline): Don't call readline_s_set_input because
  readline_getc doesn't block other threads for any FILE structure now.

  [ruby-dev:47033] [Bug #8749] reported by Nobuhiro IMAI.
  [ruby-core:57951] [Bug #9040] reporeted by Eamonn Webster.

[akr (Akira Tanaka)]

この [ruby-dev:47033] [Bug #8749] と
[ruby-core:57951] [Bug #9040] の件をあわせて修正する変更を入れました。

パッチを出してから 2ヵ月くらい経っていますが反応が無く、
[ruby-core:57951] [Bug #9040] の件も見つかったし、
Ruby 2.1.0 のリリース直前に変更するのもよくないとおもうので。

問題があったら言ってください。

[wanabe (_ wanabe)]

• Related to Bug #12950: irb: 'input-method.rb:151: [BUG] Segmentation fault' / 'malloc(): smallbin double linked list corrupted' added