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

3 files changed, 36 insertions, 13 deletions

diff --git a/ChangeLog b/ChangeLog
index 16d43145dd..2fb6ec3b65 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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
diff --git a/ruby.c b/ruby.c
index 8678bcce2c..287e22aaaa 100644
--- a/ruby.c
+++ b/ruby.c
@@ -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'