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/