nobu	2015-09-08 23:09:30 +0900 (Tue, 08 Sep 2015)

  New Revision: 51798

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

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

  Modified files:
    trunk/ChangeLog
    trunk/internal.h
    trunk/io.c
    trunk/test/ruby/test_process.rb
    trunk/thread.c
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 51797)
+++ ChangeLog	(revision 51798)
@@ -1,3 +1,8 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1
+Tue Sep  8 23:09:28 2015  Nobuyoshi Nakada  <nobu@r...>
+
+	* 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@r...>
 
 	* gems/bundled_gems: revert because ruby trunk never be able to install
Index: io.c
===================================================================
--- io.c	(revision 51797)
+++ io.c	(revision 51798)
@@ -4392,8 +4392,8 @@ rb_io_memsize(const rb_io_t *fptr) https://github.com/ruby/ruby/blob/trunk/io.c#L4392
     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) https://github.com/ruby/ruby/blob/trunk/io.c#L4409
     }
 
     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 *mode https://github.com/ruby/ruby/blob/trunk/io.c#L6196
     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, VAL https://github.com/ruby/ruby/blob/trunk/io.c#L6356
     }
     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;
 }
Index: thread.c
===================================================================
--- thread.c	(revision 51797)
+++ thread.c	(revision 51798)
@@ -2229,6 +2229,18 @@ rb_thread_kill(VALUE thread) https://github.com/ruby/ruby/blob/trunk/thread.c#L2229
     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:
Index: internal.h
===================================================================
--- internal.h	(revision 51797)
+++ internal.h	(revision 51798)
@@ -1150,6 +1150,7 @@ VALUE rb_thread_shield_new(void); https://github.com/ruby/ruby/blob/trunk/internal.h#L1150
 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);
Index: test/ruby/test_process.rb
===================================================================
--- test/ruby/test_process.rb	(revision 51797)
+++ test/ruby/test_process.rb	(revision 51798)
@@ -1715,6 +1715,30 @@ class TestProcess < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_process.rb#L1715
     }
   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?
 

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