akr	2014-05-29 23:13:20 +0900 (Thu, 29 May 2014)

  New Revision: 46232

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

  Log:
    * io.c (pipe_open): Close pipes when rb_execarg_fixup() raises
      an exception.
      (rb_execarg_fixup_v): New function.

  Modified files:
    trunk/ChangeLog
    trunk/io.c
    trunk/test/ruby/test_process.rb
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 46231)
+++ ChangeLog	(revision 46232)
@@ -1,3 +1,9 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1
+Thu May 29 23:11:20 2014  Tanaka Akira  <akr@f...>
+
+	* io.c (pipe_open): Close pipes when rb_execarg_fixup() raises
+	  an exception.
+	  (rb_execarg_fixup_v): New function.
+
 Thu May 29 22:18:57 2014  Tanaka Akira  <akr@f...>
 
 	* test/lib/minitest/unit.rb (capture_subprocess_io): Close fds.
Index: io.c
===================================================================
--- io.c	(revision 46231)
+++ io.c	(revision 46232)
@@ -5870,6 +5870,13 @@ popen_exec(void *pp, char *errmsg, size_ https://github.com/ruby/ruby/blob/trunk/io.c#L5870
 #endif
 
 static VALUE
+rb_execarg_fixup_v(VALUE execarg_obj)
+{
+    rb_execarg_fixup(execarg_obj);
+    return Qnil;
+}
+
+static VALUE
 pipe_open(VALUE execarg_obj, const char *modestr, int fmode, convconfig_t *convconfig)
 {
     struct rb_execarg *eargp = NIL_P(execarg_obj) ? NULL : rb_execarg_get(execarg_obj);
@@ -5884,6 +5891,7 @@ pipe_open(VALUE execarg_obj, const char https://github.com/ruby/ruby/blob/trunk/io.c#L5891
     char errmsg[80] = { '\0' };
 #endif
 #if defined(HAVE_FORK) || defined(HAVE_SPAWNV)
+    int state;
     struct popen_arg arg;
     int e = 0;
 #endif
@@ -5964,7 +5972,15 @@ pipe_open(VALUE execarg_obj, const char https://github.com/ruby/ruby/blob/trunk/io.c#L5972
         rb_sys_fail_str(prog);
     }
     if (!NIL_P(execarg_obj)) {
-        rb_execarg_fixup(execarg_obj);
+        rb_protect(rb_execarg_fixup_v, execarg_obj, &state);
+        if (state) {
+            if (0 <= arg.write_pair[0]) close(arg.write_pair[0]);
+            if (0 <= arg.write_pair[1]) close(arg.write_pair[1]);
+            if (0 <= arg.pair[0]) close(arg.pair[0]);
+            if (0 <= arg.pair[1]) close(arg.pair[1]);
+            rb_jump_tag(state);
+        }
+
 # if defined(HAVE_FORK)
 	pid = rb_fork_async_signal_safe(&status, popen_exec, &arg, arg.eargp->redirect_fds, errmsg, sizeof(errmsg));
 # else
Index: test/ruby/test_process.rb
===================================================================
--- test/ruby/test_process.rb	(revision 46231)
+++ test/ruby/test_process.rb	(revision 46232)
@@ -733,11 +733,14 @@ class TestProcess < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_process.rb#L733
     }
     with_pipe {|r, w|
       io = IO.popen([RUBY, "-e", "STDERR.reopen(STDOUT); IO.new(#{w.fileno}, 'w').puts('me')"])
-      w.close
-      errmsg = io.read
-      assert_equal("", r.read)
-      assert_not_equal("", errmsg)
-      Process.wait
+      begin
+        w.close
+        errmsg = io.read
+        assert_equal("", r.read)
+        assert_not_equal("", errmsg)
+      ensure
+        io.close
+      end
     }
     with_pipe {|r, w|
       errmsg = `#{RUBY} -e "STDERR.reopen(STDOUT); IO.new(#{w.fileno}, 'w').puts(123)"`
@@ -785,29 +788,38 @@ class TestProcess < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_process.rb#L788
       }
       with_pipe {|r, w|
         io = IO.popen([RUBY, "-e", "STDERR.reopen(STDOUT); IO.new(#{w.fileno}, 'w').puts('me')", :close_others=>true])
-        w.close
-        errmsg = io.read
-        assert_equal("", r.read)
-        assert_not_equal("", errmsg)
-        Process.wait
+        begin
+          w.close
+          errmsg = io.read
+          assert_equal("", r.read)
+          assert_not_equal("", errmsg)
+        ensure
+          io.close
+        end
       }
       with_pipe {|r, w|
         w.close_on_exec = false
         io = IO.popen([RUBY, "-e", "STDERR.reopen(STDOUT); IO.new(#{w.fileno}, 'w').puts('mo')", :close_others=>false])
-        w.close
-        errmsg = io.read
-        assert_equal("mo\n", r.read)
-        assert_equal("", errmsg)
-        Process.wait
+        begin
+          w.close
+          errmsg = io.read
+          assert_equal("mo\n", r.read)
+          assert_equal("", errmsg)
+        ensure
+          io.close
+        end
       }
       with_pipe {|r, w|
         w.close_on_exec = false
         io = IO.popen([RUBY, "-e", "STDERR.reopen(STDOUT); IO.new(#{w.fileno}, 'w').puts('mo')", :close_others=>nil])
-        w.close
-        errmsg = io.read
-        assert_equal("mo\n", r.read)
-        assert_equal("", errmsg)
-        Process.wait
+        begin
+          w.close
+          errmsg = io.read
+          assert_equal("mo\n", r.read)
+          assert_equal("", errmsg)
+        ensure
+          io.close
+        end
       }
 
     }
@@ -1131,8 +1143,7 @@ class TestProcess < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_process.rb#L1143
       Process.wait spawn([RUBY, "poiu"], "-e", "exit 4")
       assert_equal(4, $?.exitstatus)
 
-      assert_equal("1", IO.popen([[RUBY, "qwerty"], "-e", "print 1"]).read)
-      Process.wait
+      assert_equal("1", IO.popen([[RUBY, "qwerty"], "-e", "print 1"]) {|f| f.read })
 
       write_file("s", <<-"End")
         exec([#{RUBY.dump}, "lkjh"], "-e", "exit 5")

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