Bug #11060
closedload(fifo) blocks whole process
Description
fifo を load しようとすると、プロセス全体がブロックします。
以下では、0.1 秒毎に表示を行うスレッドを作っていますが、
0.5 秒後に load が呼ばれると表示が途切れます。
% mkfifo fifo.rb
% ls -l fifo.rb
prw-r--r-- 1 akr akr 0 Apr 12 17:13 fifo.rb
% ./ruby -ve 'Thread.new { 0.step {|i| p i; sleep 0.1 } }; sleep 0.5; load "fifo.rb"'
ruby 2.3.0dev (2015-04-12 trunk 50257) [x86_64-linux]
0
1
2
3
4
^C5
-e:1:in `new': Interrupt
from -e:1:in `load'
from -e:1:in `<main>'
当然、timeout も効きません。
% ./ruby -rtimeout -ve 'Thread.new { 0.step {|i| p i; sleep 0.1 } }; sleep 0.5; timeout(1) { load "fifo.rb" }'
ruby 2.3.0dev (2015-04-12 trunk 50257) [x86_64-linux]
0
1
2
3
4
^C5
-e:1:in `new': Interrupt
from -e:1:in `load'
from -e:1:in `block in <main>'
from /home/ruby/tst1/lib/ruby/2.3.0/timeout.rb:89:in `block in timeout'
from /home/ruby/tst1/lib/ruby/2.3.0/timeout.rb:34:in `block in catch'
from /home/ruby/tst1/lib/ruby/2.3.0/timeout.rb:34:in `catch'
from /home/ruby/tst1/lib/ruby/2.3.0/timeout.rb:34:in `catch'
from /home/ruby/tst1/lib/ruby/2.3.0/timeout.rb:104:in `timeout'
from /home/ruby/tst1/lib/ruby/2.3.0/timeout.rb:125:in `timeout'
from -e:1:in `<main>'
この挙動をバグと考えるべきかどうかはいまひとつ確信が持てないのですが、
いまのところバグであってもおかしくないと思っています。
Files
Updated by cesario (Franck Verrot) over 9 years ago
- File 0001-file.c-load-now-supports-reading-from-a-FIFO-file.patch 0001-file.c-load-now-supports-reading-from-a-FIFO-file.patch added
Akira Tanaka wrote:
fifo を load しようとすると、プロセス全体がブロックします。
I can't reproduce the same errors without correcting the typo (or I'm getting a NameError: undefined local variable or method ```pi' for main:Object
in the threads).
Once running, it seems that loading from a FIFO isn't supported (it's expecting a regular file). Here's a patch for making load
work with FIFO files.
Updated by kosaki (Motohiro KOSAKI) over 9 years ago
本筋ではないんですが、fstatのエラーを無視するように変えてしまっているのはいいんでしょうか
Updated by nobu (Nobuyoshi Nakada) over 9 years ago
Franck Verrot wrote:
I can't reproduce the same errors without correcting the typo (or I'm getting a
NameError: undefined local variable or method ```pi' for main:Object
in the threads).
It's your typo, not pi
but p i
.
Once running, it seems that loading from a FIFO isn't supported (it's expecting a regular file). Here's a patch for making
load
work with FIFO files.
It's a different story, please file a new ticket if you think it useful.
Motohiro KOSAKI wrote:
本筋ではないんですが、fstatのエラーを無視するように変えてしまっているのはいいんでしょうか
もちろんダメです。
Updated by nobu (Nobuyoshi Nakada) over 9 years ago
- Status changed from Open to Closed
Applied in changeset r50887.
file.c: open without gvl
- file.c (rb_file_load_ok): try opening file without gvl not to
lock entire process. [Bug #11060]
Updated by ngoto (Naohisa Goto) over 9 years ago
- Related to Bug #11277: "code converter not found" error with multi-thread (high occurrence rate since r50887) added
Updated by usa (Usaku NAKAMURA) over 9 years ago
- Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN to 2.0.0: WONTFIX, 2.1: REQUIRED, 2.2: REQUIRED
Updated by usa (Usaku NAKAMURA) over 9 years ago
- Backport changed from 2.0.0: WONTFIX, 2.1: REQUIRED, 2.2: REQUIRED to 2.0.0: WONTFIX, 2.1: DONE, 2.2: REQUIRED
ruby_2_1 r51118 merged revision(s) 50887,50896,50902.
Updated by cesario (Franck Verrot) over 9 years ago
Nobuyoshi Nakada wrote:
Franck Verrot wrote:
I can't reproduce the same errors without correcting the typo (or I'm getting a
NameError: undefined local variable or method ```pi' for main:Object
in the threads).It's your typo, not
pi
butp i
.
My automatic translator tricked me :-)
Once running, it seems that loading from a FIFO isn't supported (it's expecting a regular file). Here's a patch for making
load
work with FIFO files.It's a different story, please file a new ticket if you think it useful.
Yes it is, so here's the proposal: https://bugs.ruby-lang.org/issues/11273 Thanks!
Updated by nagachika (Tomoyuki Chikanaga) over 9 years ago
- Backport changed from 2.0.0: WONTFIX, 2.1: DONE, 2.2: REQUIRED to 2.0.0: WONTFIX, 2.1: DONE, 2.2: DONE
Backported into ruby_2_2
branch at r51143.
Updated by kosaki (Motohiro KOSAKI) about 9 years ago
- Related to Bug #11559: ビジーループの thread と YAML.parse を組み合わせたときの実行時間が 2.2.3 で遅くなっている added
Updated by kosaki (Motohiro KOSAKI) about 9 years ago
- Status changed from Closed to Open
[Bug #11559] の原因になっているようなので、reopenします。
Updated by kosaki (Motohiro KOSAKI) about 9 years ago
- Status changed from Open to Closed
Applied in changeset r52151.
- ruby.c (open_load_file): reset O_NONBLOCK after open.
Even if S_ISREG() is true, the file may be file on FUSE filesystem
or something. We can't assume O_NONBLOCK is safe.
Moreover, we should wait if the path is point to FIFO. That's
FIFO semantics. GVL should be transparent from ruby script.
Thus, just reopen without O_NONBLOCK for filling the requirements.
[Bug #11060][Bug #11559] - ruby.c (loadopen_func): new for the above.
- file.c (ruby_is_fd_loadable): new. for checks loadable file type
of not. - file.c (rb_file_load_ok): use ruby_is_fd_loadble()
- internal.h: add ruby_is_fd_loadble()
- common.mk: now, ruby.o depend on thread.h.
- test/ruby/test_require.rb
(TestRequire#test_loading_fifo_threading_success): new test.
This test successful case that loading from FIFO. - test/ruby/test_require.rb
(TestRequire#test_loading_fifo_threading_raise): rename from
test_loading_fifo_threading. You souldn't rescue an exception
if you test raise or not.
Moreover, this case should be caught IOError because load(FIFO)
should be blocked until given any input.
Updated by kosaki (Motohiro KOSAKI) about 9 years ago
修正メモ
r50887 はakrパッチにあったS_ISFIFO()のチェックが入っていないため、FIFOがemptyじゃなくなるまで待つが、待った後エラーになってしまいロードできていませんでした。
FIFOから正常にロードできるテストが存在しないのがよくないので、当該テスト足しました。
また、rb_file_load_ok()を修正するのは論理的におかしくて、rb_file_load_ok()でロードできるかどうか一度チェックしてから一旦クローズ、
再度 load_file_internal()で開き直す処理のため、load_file_internal()を手当しないとrace conditionがありました。
r52139 は単に、load時のすべてのopenをO_NONBLOCK付きに変えていますが、挙動が変わってしまうためNGだと考えています。よって、一旦O_NONBLOCKで開いてからfcntl()で
O_NONBLOCK取り消し、かつFIFOのときのみopenしなおしという論理にしました。開く前にstatせずに、一旦oepn(O_NONBLOCK)で開いてからfstatするのはrace対策。
Updated by nobu (Nobuyoshi Nakada) over 5 years ago
- Related to Bug #15787: LoadError by EPERM on read-only volume added