io.c: no wait when killed

* io.c (rb_io_s_popen): do not wait the child process during being
  killed.  [ruby-core:70671] [Bug #11510]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@51798 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

5 files changed, 71 insertions, 7 deletions

diff --git a/ChangeLog b/ChangeLog
index 4253384a3c..f25d0f7d74 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+Tue Sep  8 23:09:28 2015  Nobuyoshi Nakada  <nobu@ruby-lang.org>
+
+	* io.c (rb_io_s_popen): do not wait the child process during being
+	  killed.  [ruby-core:70671] [Bug #11510]
+
 Tue Sep  8 22:18:04 2015  NAKAMURA Usaku  <usa@ruby-lang.org>
 
 	* gems/bundled_gems: revert because ruby trunk never be able to install
diff --git a/internal.h b/internal.h
index f352a95c0f..ba655e999e 100644
--- a/internal.h
+++ b/internal.h
@@ -1150,6 +1150,7 @@ VALUE rb_thread_shield_new(void);
 VALUE rb_thread_shield_wait(VALUE self);
 VALUE rb_thread_shield_release(VALUE self);
 VALUE rb_thread_shield_destroy(VALUE self);
+int rb_thread_to_be_killed(VALUE thread);
 void rb_mutex_allow_trap(VALUE self, int val);
 VALUE rb_uninterruptible(VALUE (*b_proc)(ANYARGS), VALUE data);
 VALUE rb_mutex_owned_p(VALUE self);
diff --git a/io.c b/io.c
index 9fa8eaf6cc..4951a09432 100644
--- a/io.c
+++ b/io.c
@@ -4392,8 +4392,8 @@ rb_io_memsize(const rb_io_t *fptr)
     return size;
 }
 
-VALUE
-rb_io_close(VALUE io)
+static rb_io_t *
+io_close_fptr(VALUE io)
 {
     rb_io_t *fptr;
     int fd;
@@ -4409,19 +4409,31 @@ rb_io_close(VALUE io)
     }
 
     fptr = RFILE(io)->fptr;
-    if (!fptr) return Qnil;
-    if (fptr->fd < 0) return Qnil;
+    if (!fptr) return 0;
+    if (fptr->fd < 0) return 0;
 
     fd = fptr->fd;
     rb_thread_fd_close(fd);
     rb_io_fptr_cleanup(fptr, FALSE);
+    return fptr;
+}
 
+static void
+fptr_waitpid(rb_io_t *fptr, int nohang)
+{
+    int status;
     if (fptr->pid) {
-        rb_last_status_clear();
-	rb_syswait(fptr->pid);
+	rb_last_status_clear();
+	rb_waitpid(fptr->pid, &status, nohang ? WNOHANG : 0);
 	fptr->pid = 0;
     }
+}
 
+VALUE
+rb_io_close(VALUE io)
+{
+    rb_io_t *fptr = io_close_fptr(io);
+    if (fptr) fptr_waitpid(fptr, 0);
     return Qnil;
 }
 
@@ -6184,6 +6196,16 @@ pipe_open_s(VALUE prog, const char *modestr, int fmode, convconfig_t *convconfig
     return pipe_open(execarg_obj, modestr, fmode, convconfig);
 }
 
+static VALUE
+pipe_close(VALUE io)
+{
+    rb_io_t *fptr = io_close_fptr(io);
+    if (fptr) {
+	fptr_waitpid(fptr, rb_thread_to_be_killed(rb_thread_current()));
+    }
+    return Qnil;
+}
+
 /*
  *  call-seq:
  *     IO.popen([env,] cmd, mode="r" [, opt])               -> io
@@ -6334,7 +6356,7 @@ rb_io_s_popen(int argc, VALUE *argv, VALUE klass)
     }
     RBASIC_SET_CLASS(port, klass);
     if (rb_block_given_p()) {
-	return rb_ensure(rb_yield, port, io_close, port);
+	return rb_ensure(rb_yield, port, pipe_close, port);
     }
     return port;
 }
diff --git a/test/ruby/test_process.rb b/test/ruby/test_process.rb
index 6828c50bf1..5dbac8e8e0 100644
--- a/test/ruby/test_process.rb
+++ b/test/ruby/test_process.rb
@@ -1715,6 +1715,30 @@ class TestProcess < Test::Unit::TestCase
     }
   end
 
+  def test_popen_exit
+    bug11510 = '[ruby-core:70671] [Bug #11510]'
+    pid = nil
+    opt = {timeout: 10, pgroup: true, stdout_filter: ->(s) {pid = s}}
+    assert_ruby_status(["-", RUBY], <<-'end;', bug11510, **opt)
+      RUBY = ARGV[0]
+      th = Thread.start {
+        Thread.current.abort_on_exception = true
+        IO.popen([RUBY, "-esleep 15", err: [:child, :out]]) {|f|
+          STDOUT.puts f.pid
+          STDOUT.flush
+          sleep(2)
+        }
+      }
+      sleep(0.001) until th.stop?
+    end;
+    assert_match(/\A\d+\Z/, pid)
+  ensure
+    if pid
+      pid = pid.to_i
+      [:TERM, :KILL].each {|sig| Process.kill(sig, pid) rescue break}
+    end
+  end
+
   def test_execopts_new_pgroup
     return unless windows?
 
diff --git a/thread.c b/thread.c
index e92208eedc..1d8ea96e03 100644
--- a/thread.c
+++ b/thread.c
@@ -2229,6 +2229,18 @@ rb_thread_kill(VALUE thread)
     return thread;
 }
 
+int
+rb_thread_to_be_killed(VALUE thread)
+{
+    rb_thread_t *th;
+
+    GetThreadPtr(thread, th);
+
+    if (th->to_kill || th->status == THREAD_KILLED) {
+	return TRUE;
+    }
+    return FALSE;
+}
 
 /*
  *  call-seq: