From b3d617bc20fadaa44fc4e130f478152732d48009 Mon Sep 17 00:00:00 2001 From: Brandon Fish Date: Wed, 16 Jun 2021 17:17:14 -0500 Subject: [PATCH 1/2] Add specs for Process::Status.wait --- spec/ruby/core/process/status/wait_spec.rb | 95 ++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 spec/ruby/core/process/status/wait_spec.rb diff --git a/spec/ruby/core/process/status/wait_spec.rb b/spec/ruby/core/process/status/wait_spec.rb new file mode 100644 index 000000000000..2890f2d18460 --- /dev/null +++ b/spec/ruby/core/process/status/wait_spec.rb @@ -0,0 +1,95 @@ +require_relative '../../../spec_helper' +require_relative '../fixtures/common' + +ruby_version_is "3.0" do + describe "Process::Status.wait" do + ProcessSpecs.use_system_ruby(self) + + before :all do + begin + leaked = Process.waitall + # Ruby-space should not see PIDs used by mjit + raise "subprocesses leaked before wait specs: #{leaked}" unless leaked.empty? + rescue NotImplementedError + end + end + + it "returns a status with pid -1 if there are no child processes" do + Process::Status.wait.pid.should == -1 + end + + platform_is_not :windows do + it "returns a status with its child pid" do + pid = Process.spawn(ruby_cmd('exit')) + status = Process::Status.wait + status.should be_an_instance_of(Process::Status) + status.pid.should == pid + end + + it "should not set $? to a Process::Status" do + pid = Process.spawn(ruby_cmd('exit')) + Process::Status.wait + $?.should be_nil + end + + it "waits for any child process if no pid is given" do + pid = Process.spawn(ruby_cmd('exit')) + Process::Status.wait.pid.should == pid + -> { Process.kill(0, pid) }.should raise_error(Errno::ESRCH) + end + + it "waits for a specific child if a pid is given" do + pid1 = Process.spawn(ruby_cmd('exit')) + pid2 = Process.spawn(ruby_cmd('exit')) + Process::Status.wait(pid2).pid.should == pid2 + Process::Status.wait(pid1).pid.should == pid1 + -> { Process.kill(0, pid1) }.should raise_error(Errno::ESRCH) + -> { Process.kill(0, pid2) }.should raise_error(Errno::ESRCH) + end + + it "coerces the pid to an Integer" do + pid1 = Process.spawn(ruby_cmd('exit')) + Process::Status.wait(mock_int(pid1)).pid.should == pid1 + -> { Process.kill(0, pid1) }.should raise_error(Errno::ESRCH) + end + + # This spec is probably system-dependent. + it "waits for a child whose process group ID is that of the calling process" do + pid1 = Process.spawn(ruby_cmd('exit'), pgroup: true) + pid2 = Process.spawn(ruby_cmd('exit')) + + Process::Status.wait(0).pid.should == pid2 + Process::Status.wait.pid.should == pid1 + end + + # This spec is probably system-dependent. + it "doesn't block if no child is available when WNOHANG is used" do + read, write = IO.pipe + pid = Process.fork do + read.close + Signal.trap("TERM") { Process.exit! } + write << 1 + write.close + sleep + end + + Process::Status.wait(pid, Process::WNOHANG).should be_nil + + # wait for the child to setup its TERM handler + write.close + read.read(1) + read.close + + Process.kill("TERM", pid) + Process::Status.wait.pid.should == pid + end + + it "always accepts flags=0" do + pid = Process.spawn(ruby_cmd('exit')) + Process::Status.wait(-1, 0).pid.should == pid + -> { Process.kill(0, pid) }.should raise_error(Errno::ESRCH) + end + end + end +end + From eaf1a36967e78d2c845b291fe4cf6b24a4b29d9b Mon Sep 17 00:00:00 2001 From: Brandon Fish Date: Thu, 17 Jun 2021 13:05:24 -0500 Subject: [PATCH 2/2] Implement Process::Status.wait --- CHANGELOG.md | 1 + spec/ruby/core/process/status/wait_spec.rb | 12 ++++++-- spec/tags/core/process/status/wait_tags.txt | 1 + spec/truffleruby.mspec | 1 + src/main/ruby/truffleruby/core/process.rb | 29 +++++-------------- .../core/truffle/process_operations.rb | 23 +++++++++++++++ 6 files changed, 43 insertions(+), 24 deletions(-) create mode 100644 spec/tags/core/process/status/wait_tags.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index e9498e501db1..2023f95ce234 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Bug fixes: Compatibility: +* Implement `Process::Status.wait` (#2378). Performance: diff --git a/spec/ruby/core/process/status/wait_spec.rb b/spec/ruby/core/process/status/wait_spec.rb index 2890f2d18460..c65119494ecd 100644 --- a/spec/ruby/core/process/status/wait_spec.rb +++ b/spec/ruby/core/process/status/wait_spec.rb @@ -26,10 +26,18 @@ status.pid.should == pid end - it "should not set $? to a Process::Status" do + it "should not set $? to the Process::Status" do pid = Process.spawn(ruby_cmd('exit')) + status = Process::Status.wait + $?.should_not equal(status) + end + + it "should not change the value of $?" do + pid = Process.spawn(ruby_cmd('exit')) + Process.wait + status = $? Process::Status.wait - $?.should be_nil + status.should equal($?) end it "waits for any child process if no pid is given" do diff --git a/spec/tags/core/process/status/wait_tags.txt b/spec/tags/core/process/status/wait_tags.txt new file mode 100644 index 000000000000..c203a3c96da3 --- /dev/null +++ b/spec/tags/core/process/status/wait_tags.txt @@ -0,0 +1 @@ +fails:Process::Status.wait doesn't block if no child is available when WNOHANG is used diff --git a/spec/truffleruby.mspec b/spec/truffleruby.mspec index 1131370d10ce..53df894e7246 100644 --- a/spec/truffleruby.mspec +++ b/spec/truffleruby.mspec @@ -95,6 +95,7 @@ class MSpecScript set :next, %w[ spec/ruby/core/mutex/owned_spec.rb spec/ruby/core/fiber/raise_spec.rb + spec/ruby/core/process/status/wait_spec.rb ] set :tags_patterns, [ diff --git a/src/main/ruby/truffleruby/core/process.rb b/src/main/ruby/truffleruby/core/process.rb index 14b215f5a60b..4178b8090c54 100644 --- a/src/main/ruby/truffleruby/core/process.rb +++ b/src/main/ruby/truffleruby/core/process.rb @@ -601,26 +601,8 @@ def self.last_status # TODO: Support other options such as WUNTRACED? --rue # def self.wait2(input_pid=-1, flags=nil) - input_pid = Truffle::Type.coerce_to input_pid, Integer, :to_int - flags ||= 0 - - FFI::MemoryPointer.new(:int, 4) do |ptr| - pid = Truffle::POSIX.truffleposix_waitpid(input_pid, flags, ptr) - if pid == 0 - return nil - elsif pid == -1 - Errno.handle "No child process: #{input_pid}" - else - ints = ptr.read_array_of_int(4) - exitcode, termsig, stopsig, = ints.map { |e| e == -1000 ? nil : e } - raw_status = ints.last - - status = Process::Status.new(pid, exitcode, termsig, stopsig, raw_status) - Primitive.thread_set_return_code status - - [pid, status] - end - end + status = Truffle::ProcessOperations.wait(input_pid, flags, true, true) + [status.pid, status] end # @@ -650,8 +632,7 @@ def self.waitall end def self.wait(pid=-1, flags=nil) - pid, _status = Process.wait2(pid, flags) - pid + Truffle::ProcessOperations.wait(pid, flags, true, true)&.pid end class << self @@ -781,6 +762,10 @@ def to_s def inspect "#" end + + def self.wait(pid=-1, flags=nil) + Truffle::ProcessOperations.wait(pid, flags, false, false) + end end module Sys diff --git a/src/main/ruby/truffleruby/core/truffle/process_operations.rb b/src/main/ruby/truffleruby/core/truffle/process_operations.rb index fb00863b72d5..cca0f3f05282 100644 --- a/src/main/ruby/truffleruby/core/truffle/process_operations.rb +++ b/src/main/ruby/truffleruby/core/truffle/process_operations.rb @@ -87,6 +87,29 @@ def self.spawn(*args) end end + def self.wait(input_pid, flags, set_status, raise_on_error) + input_pid = Truffle::Type.coerce_to input_pid, Integer, :to_int + flags ||= 0 + + FFI::MemoryPointer.new(:int, 4) do |ptr| + pid = Truffle::POSIX.truffleposix_waitpid(input_pid, flags, ptr) + if pid == 0 + return nil + elsif raise_on_error && pid == -1 + Errno.handle "No child process: #{input_pid}" + else + ints = ptr.read_array_of_int(4) + exitcode, termsig, stopsig, = ints.map { |e| e == -1000 ? nil : e } + raw_status = ints.last + + status = Process::Status.new(pid, exitcode, termsig, stopsig, raw_status) + Primitive.thread_set_return_code status if set_status + + status + end + end + end + class Execute def initialize @options = {}