[前][次][番号順一覧][スレッド一覧]

ruby-changes:51555

From: normal <ko1@a...>
Date: Thu, 28 Jun 2018 06:45:01 +0900 (JST)
Subject: [ruby-changes:51555] normal:r63764 (trunk): process.c (waitpid_wait): do not set ECHILD prematurely

normal	2018-06-27 19:09:33 +0900 (Wed, 27 Jun 2018)

  New Revision: 63764

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

  Log:
    process.c (waitpid_wait): do not set ECHILD prematurely
    
    It is possible to have both MJIT and normal child processes
    alive, so we cannot set ECHILD based on such a guess.  We can
    still elide waitpid(PID <= 0) calls if we have callers in
    vm->waiting_pids, however.
    
    For specs, ensure Process.waitall does not leak MJIT
    PIDs to Rubyspace.

  Modified files:
    trunk/process.c
    trunk/spec/mspec/lib/mspec/guards/feature.rb
    trunk/spec/ruby/core/process/wait2_spec.rb
    trunk/spec/ruby/core/process/wait_spec.rb
Index: spec/mspec/lib/mspec/guards/feature.rb
===================================================================
--- spec/mspec/lib/mspec/guards/feature.rb	(revision 63763)
+++ spec/mspec/lib/mspec/guards/feature.rb	(revision 63764)
@@ -39,3 +39,9 @@ end https://github.com/ruby/ruby/blob/trunk/spec/mspec/lib/mspec/guards/feature.rb#L39
 def with_feature(*features, &block)
   FeatureGuard.new(*features).run_if(:with_feature, &block)
 end
+
+MSpecEnv.class_eval do
+  def without_feature(*features, &block)
+    FeatureGuard.new(*features).run_unless(:without_feature, &block)
+  end
+end
Index: spec/ruby/core/process/wait_spec.rb
===================================================================
--- spec/ruby/core/process/wait_spec.rb	(revision 63763)
+++ spec/ruby/core/process/wait_spec.rb	(revision 63764)
@@ -8,6 +8,10 @@ describe "Process.wait" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/process/wait_spec.rb#L8
     begin
       leaked = Process.waitall
       puts "leaked before wait specs: #{leaked}" unless leaked.empty?
+      with_feature :mjit do
+        # Ruby-space should not see PIDs used by mjit
+        leaked.should be_empty
+      end
     rescue NotImplementedError
     end
   end
Index: spec/ruby/core/process/wait2_spec.rb
===================================================================
--- spec/ruby/core/process/wait2_spec.rb	(revision 63763)
+++ spec/ruby/core/process/wait2_spec.rb	(revision 63764)
@@ -4,11 +4,19 @@ describe "Process.wait2" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/process/wait2_spec.rb#L4
   before :all do
     # HACK: this kludge is temporarily necessary because some
     # misbehaving spec somewhere else does not clear processes
+    # Note: background processes are unavoidable with MJIT,
+    # but we shouldn't reap them from Ruby-space
     begin
       Process.wait(-1, Process::WNOHANG)
-      $stderr.puts "Leaked process before wait2 specs! Waiting for it"
+      without_feature :mjit do
+        $stderr.puts "Leaked process before wait2 specs! Waiting for it"
+      end
       leaked = Process.waitall
-      $stderr.puts "leaked before wait2 specs: #{leaked}"
+      $stderr.puts "leaked before wait2 specs: #{leaked}" unless leaked.empty?
+      with_feature :mjit do
+        # Ruby-space should not see PIDs used by mjit
+        leaked.should be_empty
+      end
     rescue Errno::ECHILD # No child processes
     rescue NotImplementedError
     end
Index: process.c
===================================================================
--- process.c	(revision 63763)
+++ process.c	(revision 63764)
@@ -1074,12 +1074,6 @@ waitpid_wait(struct waitpid_state *w) https://github.com/ruby/ruby/blob/trunk/process.c#L1074
     }
     else if (w->options & WNOHANG) {
         w->cond = 0;
-
-        /* MJIT must be waiting, but don't tell Ruby callers about it */
-        if (w->pid < 0 && !list_empty(&vm->waiting_pids)) {
-            w->ret = -1;
-            w->errnum = ECHILD;
-        }
     }
     else {
         w->cond = rb_sleep_cond_get(w->ec);

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

[前][次][番号順一覧][スレッド一覧]