Project

General

Profile

Actions

Bug #11060

closed

load(fifo) blocks whole process

Added by akr (Akira Tanaka) almost 9 years ago. Updated over 8 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 2.3.0dev (2015-04-12 trunk 50257) [x86_64-linux]
[ruby-dev:<unknown>]

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


Related issues 3 (0 open3 closed)

Related to Ruby master - Bug #11277: "code converter not found" error with multi-thread (high occurrence rate since r50887)ClosedActions
Related to Ruby master - Bug #11559: ビジーループの thread と YAML.parse を組み合わせたときの実行時間が 2.2.3 で遅くなっているClosednobu (Nobuyoshi Nakada)Actions
Related to Ruby master - Bug #15787: LoadError by EPERM on read-only volumeFeedbackActions

Updated by cesario (Franck Verrot) almost 9 years ago

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) almost 9 years ago

本筋ではないんですが、fstatのエラーを無視するように変えてしまっているのはいいんでしょうか

Updated by nobu (Nobuyoshi Nakada) almost 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のエラーを無視するように変えてしまっているのはいいんでしょうか

もちろんダメです。

Actions #4

Updated by nobu (Nobuyoshi Nakada) almost 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]
Actions #5

Updated by ngoto (Naohisa Goto) almost 9 years ago

  • Related to Bug #11277: "code converter not found" error with multi-thread (high occurrence rate since r50887) added
Actions #6

Updated by usa (Usaku NAKAMURA) over 8 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 8 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 8 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 but p 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 8 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.

Actions #10

Updated by kosaki (Motohiro KOSAKI) over 8 years ago

  • Related to Bug #11559: ビジーループの thread と YAML.parse を組み合わせたときの実行時間が 2.2.3 で遅くなっている added
Actions #11

Updated by kosaki (Motohiro KOSAKI) over 8 years ago

  • Status changed from Closed to Open

[Bug #11559] の原因になっているようなので、reopenします。

Actions #12

Updated by kosaki (Motohiro KOSAKI) over 8 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) over 8 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対策。

Actions #14

Updated by nobu (Nobuyoshi Nakada) almost 5 years ago

  • Related to Bug #15787: LoadError by EPERM on read-only volume added
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0