nobu	2016-10-10 15:22:30 +0900 (Mon, 10 Oct 2016)

  New Revision: 56388

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=56388

  Log:
    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.

  Modified files:
    trunk/ChangeLog
    trunk/ruby.c
    trunk/test/ruby/test_require.rb
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 56387)
+++ ChangeLog	(revision 56388)
@@ -1,3 +1,8 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1
+Mon Oct 10 15:22:27 2016  Nobuyoshi Nakada  <nobu@r...>
+
+	* 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@r...>
 
 	* ruby.c (open_load_file): compare with EXEEXT instead of hard
Index: test/ruby/test_require.rb
===================================================================
--- test/ruby/test_require.rb	(revision 56387)
+++ test/ruby/test_require.rb	(revision 56388)
@@ -757,6 +757,26 @@ class TestRequire < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_require.rb#L757
     }
   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'
Index: ruby.c
===================================================================
--- ruby.c	(revision 56387)
+++ ruby.c	(revision 56388)
@@ -1916,22 +1916,20 @@ open_load_file(VALUE fname_v, int *xflag https://github.com/ruby/ruby/blob/trunk/ruby.c#L1916
 #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;
 }

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/