Bug #11060: load(fifo) blocks whole process

• Status: Closed
• Priority: Normal
• Assignee: -
• Target version: -
• ruby -v: ruby 2.3.0dev (2015-04-12 trunk 50257) [x86_64-linux]
• Backport: 2.0.0: WONTFIX, 2.1: DONE, 2.2: DONE

[akr (Akira Tanaka)]

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>'

この挙動をバグと考えるべきかどうかはいまひとつ確信が持てないのですが、
いまのところバグであってもおかしくないと思っています。

[cesario (Franck Verrot)]

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

[kosaki (Motohiro KOSAKI)]

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

[nobu (Nobuyoshi Nakada)]

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

もちろんダメです。

[nobu (Nobuyoshi Nakada)]

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

[ngoto (Naohisa Goto)]

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

[usa (Usaku NAKAMURA)]

• Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN to 2.0.0: WONTFIX, 2.1: REQUIRED, 2.2: REQUIRED

[usa (Usaku NAKAMURA)]

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

[cesario (Franck Verrot)]

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!

[nagachika (Tomoyuki Chikanaga)]

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

[kosaki (Motohiro KOSAKI)]

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

[kosaki (Motohiro KOSAKI)]

• Status changed from Closed to Open

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

[kosaki (Motohiro KOSAKI)]

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

[kosaki (Motohiro KOSAKI)]

修正メモ

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対策。

[nobu (Nobuyoshi Nakada)]

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