Bug #8749

Readline.readline stops STDOUT?

Added by Nobuhiro IMAI 9 months ago. Updated 6 months ago.

[ruby-dev:<unknown>]
Status:Closed
Priority:Normal
Assignee:Kouji Takao
Category:ext
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

Description

=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

readline-release-gvl.patch Magnifier (4.11 KB) Akira Tanaka, 08/12/2013 09:21 PM

readline-release-gvl-2.patch Magnifier (4.12 KB) Akira Tanaka, 08/15/2013 01:42 PM

Associated revisions

Revision 43439
Added by Akira Tanaka 6 months ago

  • ext/readline/readline.c: Include ruby/thread.h for
    rbthreadcallwithoutgvl2.
    (readlinerlinstream, readlinerloutstream): Record FILE
    structures allocated by this extension.
    (getcbody): New function extracted from readlinegetc.
    (getcfunc): New function.
    (readline
    getc): Use rbthreadcallwithoutgvl2 to invoke getcfunc.
    [Bug #8749]
    (clear
    rlinstream, clearrloutstream): Close FILE structure
    allocated by this extention reliably. [Bug #9040]
    (readline
    readline): Use clearrlinstream and clearrloutstream.
    (readlinessetinput): Set readlinerlinstream.
    (readline
    ssetoutput): Set readlinerloutstream.
    (Initreadline): Don't call readlinessetinput because
    readline_getc doesn't block other threads for any FILE structure now.

    [Bug #8749] reported by Nobuhiro IMAI.
    [Bug #9040] reporeted by Eamonn Webster.

History

#1 Updated by Akira Tanaka 8 months ago

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

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

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

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

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

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

#2 Updated by Kouji Takao 8 months ago

  • Assignee set to Kouji Takao

#3 Updated by Nobuhiro IMAI 8 months ago

読み込みでブロックしている時に 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]
}
^

#4 Updated by Akira Tanaka 8 months ago

no6v (Nobuhiro IMAI) wrote:

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

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

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

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

r42528
if (rlinstream != ifp->stdiofile) return rlgetc(input);
という文を削除していて、そのため、rl
getc じゃなくて下のほうに制御が行って、
rbiogetbyte を呼び出すようになったのでしょう。
ここで rbiogetbyte は 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 として添付します。

#5 Updated by Akira Tanaka 6 months ago

  • 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
    rbthreadcallwithoutgvl2.
    (readlinerlinstream, readlinerloutstream): Record FILE
    structures allocated by this extension.
    (getcbody): New function extracted from readlinegetc.
    (getcfunc): New function.
    (readline
    getc): Use rbthreadcallwithoutgvl2 to invoke getcfunc.
    [Bug #8749]
    (clear
    rlinstream, clearrloutstream): Close FILE structure
    allocated by this extention reliably. [Bug #9040]
    (readline
    readline): Use clearrlinstream and clearrloutstream.
    (readlinessetinput): Set readlinerlinstream.
    (readline
    ssetoutput): Set readlinerloutstream.
    (Initreadline): Don't call readlinessetinput because
    readline_getc doesn't block other threads for any FILE structure now.

    [Bug #8749] reported by Nobuhiro IMAI.
    [Bug #9040] reporeted by Eamonn Webster.

#6 Updated by Akira Tanaka 6 months ago

この [Bug #8749] と
[Bug #9040] の件をあわせて修正する変更を入れました。

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

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

Also available in: Atom PDF