Project

General

Profile

Bug #11060

load(fifo) blocks whole process

Added by akr (Akira Tanaka) about 4 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
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

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

Associated revisions

Revision 0b3ef899
Added by nobu (Nobuyoshi Nakada) about 4 years ago

file.c: open without gvl

  • file.c (rb_file_load_ok): try opening file without gvl not to lock entire process. [Bug #11060]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@50887 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 50887
Added by nobu (Nobuyoshi Nakada) about 4 years ago

file.c: open without gvl

  • file.c (rb_file_load_ok): try opening file without gvl not to lock entire process. [Bug #11060]

Revision 50887
Added by nobu (Nobuyoshi Nakada) about 4 years ago

file.c: open without gvl

  • file.c (rb_file_load_ok): try opening file without gvl not to lock entire process. [Bug #11060]

Revision 50887
Added by nobu (Nobuyoshi Nakada) about 4 years ago

file.c: open without gvl

  • file.c (rb_file_load_ok): try opening file without gvl not to lock entire process. [Bug #11060]

Revision 50887
Added by nobu (Nobuyoshi Nakada) about 4 years ago

file.c: open without gvl

  • file.c (rb_file_load_ok): try opening file without gvl not to lock entire process. [Bug #11060]

Revision 50887
Added by nobu (Nobuyoshi Nakada) about 4 years ago

file.c: open without gvl

  • file.c (rb_file_load_ok): try opening file without gvl not to lock entire process. [Bug #11060]

Revision dffe87c7
Added by usa (Usaku NAKAMURA) almost 4 years ago

merge revision(s) 50887,50896,50902: [Backport #11060]

    * file.c (rb_file_load_ok): try opening file without gvl not to
      lock entire process.  [Bug #11060]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_1@51118 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 51118
Added by usa (Usaku NAKAMURA) almost 4 years ago

merge revision(s) 50887,50896,50902: [Backport #11060]

* file.c (rb_file_load_ok): try opening file without gvl not to
  lock entire process.  [Bug #11060]

Revision b0ed2765
Added by usa (Usaku NAKAMURA) almost 4 years ago

  • test/ruby/test_require.rb (TestRequire#test_loading_fifo_threading): ignore Errno::ENOENT on unlinking. [Bug #11060]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_1@51125 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 51125
Added by usa (Usaku NAKAMURA) almost 4 years ago

  • test/ruby/test_require.rb (TestRequire#test_loading_fifo_threading): ignore Errno::ENOENT on unlinking. [Bug #11060]

Revision 6282b156
Added by usa (Usaku NAKAMURA) almost 4 years ago

  • test/ruby/test_require.rb (TestRequire#test_loading_fifo_threading): fix previous commit. [Bug #11060]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_1@51128 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 51128
Added by usa (Usaku NAKAMURA) almost 4 years ago

  • test/ruby/test_require.rb (TestRequire#test_loading_fifo_threading): fix previous commit. [Bug #11060]

Revision 31539009
Added by nagachika (Tomoyuki Chikanaga) almost 4 years ago

merge revision(s) 50887,50896,50902: [Backport #11060]

    * file.c (rb_file_load_ok): try opening file without gvl not to
      lock entire process.  [Bug #11060]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_2@51143 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 51143
Added by nagachika (Tomoyuki Chikanaga) almost 4 years ago

merge revision(s) 50887,50896,50902: [Backport #11060]

* file.c (rb_file_load_ok): try opening file without gvl not to
  lock entire process.  [Bug #11060]

Revision bc8687ac
Added by kosaki (Motohiro KOSAKI) over 3 years ago

  • 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.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@52151 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 52151
Added by kosaki (Motohiro KOSAKI) over 3 years ago

  • 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.

Revision 52151
Added by kosaki (Motohiro KOSAKI) over 3 years ago

  • 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.

Revision 52151
Added by kosaki (Motohiro KOSAKI) over 3 years ago

  • 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.

Revision 52151
Added by kosaki (Motohiro KOSAKI) over 3 years ago

  • 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.

Revision 52151
Added by kosaki (Motohiro KOSAKI) over 3 years ago

  • 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.

History

Updated by cesario (Franck Verrot) about 4 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) about 4 years ago

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

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

もちろんダメです。

#4

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

Updated by ngoto (Naohisa Goto) about 4 years ago

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

Updated by usa (Usaku NAKAMURA) almost 4 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) almost 4 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) almost 4 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) almost 4 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.

#10

Updated by kosaki (Motohiro KOSAKI) over 3 years ago

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

Updated by kosaki (Motohiro KOSAKI) over 3 years ago

  • Status changed from Closed to Open

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

#12

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

#14

Updated by nobu (Nobuyoshi Nakada) about 1 month ago

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

Also available in: Atom PDF