diff options
author | nobu <nobu@b2dd03c8-39d4-4d8f-98ff-823fe69b080e> | 2016-10-10 06:22:30 +0000 |
---|---|---|
committer | nobu <nobu@b2dd03c8-39d4-4d8f-98ff-823fe69b080e> | 2016-10-10 06:22:30 +0000 |
commit | 8c3af8ecc078704128cda961b5316162506e1eb1 (patch) | |
tree | 4f9388417afcdfe816be26a1dafa354128865606 | |
parent | a28d3d591dc6a37cb2544d529a888a99c63e4328 (diff) | |
download | ruby-8c3af8ecc078704128cda961b5316162506e1eb1.tar.gz |
ruby.c: bind fd before waiting
* ruby.c (open_load_file): bind the open fd to an IO instance
before waiting FIFO, not to leak the fd if interrupted.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@56388 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
-rw-r--r-- | ChangeLog | 5 | ||||
-rw-r--r-- | ruby.c | 24 | ||||
-rw-r--r-- | test/ruby/test_require.rb | 20 |
3 files changed, 36 insertions, 13 deletions
@@ -1,3 +1,8 @@ +Mon Oct 10 15:22:27 2016 Nobuyoshi Nakada <nobu@ruby-lang.org> + + * ruby.c (open_load_file): bind the open fd to an IO instance + before waiting FIFO, not to leak the fd if interrupted. + Mon Oct 10 12:40:54 2016 Nobuyoshi Nakada <nobu@ruby-lang.org> * ruby.c (open_load_file): compare with EXEEXT instead of hard @@ -1916,22 +1916,20 @@ open_load_file(VALUE fname_v, int *xflag) #endif e = ruby_is_fd_loadable(fd); - if (e <= 0) { - if (!e) { - e = errno; - (void)close(fd); - rb_load_fail(fname_v, strerror(e)); - } - else { - /* - We need to wait if FIFO is empty. It's FIFO's semantics. - rb_thread_wait_fd() release GVL. So, it's safe. - */ - rb_thread_wait_fd(fd); - } + if (!e) { + e = errno; + (void)close(fd); + rb_load_fail(fname_v, strerror(e)); } f = rb_io_fdopen(fd, mode, fname); + if (e < 0) { + /* + We need to wait if FIFO is empty. It's FIFO's semantics. + rb_thread_wait_fd() release GVL. So, it's safe. + */ + rb_thread_wait_fd(fd); + } } return f; } diff --git a/test/ruby/test_require.rb b/test/ruby/test_require.rb index b5802e71c4..5c2221c77b 100644 --- a/test/ruby/test_require.rb +++ b/test/ruby/test_require.rb @@ -757,6 +757,26 @@ class TestRequire < Test::Unit::TestCase } end if File.respond_to?(:mkfifo) + def test_loading_fifo_fd_leak + Tempfile.create(%w'fifo .rb') {|f| + f.close + File.unlink(f.path) + File.mkfifo(f.path) + assert_separately(["-", f.path], "#{<<-"begin;"}\n#{<<-"end;"}", timeout: 3) + begin; + Process.setrlimit(Process::RLIMIT_NOFILE, 50) + th = Thread.current + 100.times do |i| + Thread.start {begin sleep(0.001) end until th.stop?; th.raise(IOError)} + assert_raise(IOError, "\#{i} time") do + tap {tap {tap {load(ARGV[0])}}} + end + GC.start + end + end; + } + end if File.respond_to?(:mkfifo) and defined?(Process::RLIMIT_NOFILE) + def test_throw_while_loading Tempfile.create(%w'bug-11404 .rb') do |f| f.puts 'sleep' |