diff options
author | danielsdeleo <dan@opscode.com> | 2013-09-11 11:50:31 -0700 |
---|---|---|
committer | danielsdeleo <dan@opscode.com> | 2013-09-11 11:50:31 -0700 |
commit | e986fc3ea9d0a6f09c005e7240db60b4db2861a7 (patch) | |
tree | d332fb8ac53c3d14e8e8aae5bcaf115309630f1b | |
parent | fd66077d0368babc5ba57d52f96e1ebc52d70ca1 (diff) | |
parent | ebe7f8e87349655ba2601a3b18c4711fd95c6bf5 (diff) | |
download | mixlib-shellout-e986fc3ea9d0a6f09c005e7240db60b4db2861a7.tar.gz |
Fix MIXLIB-17
-rw-r--r-- | lib/mixlib/shellout/unix.rb | 10 | ||||
-rw-r--r-- | spec/mixlib/shellout_spec.rb | 32 | ||||
-rw-r--r-- | spec/spec_helper.rb | 1 |
3 files changed, 41 insertions, 2 deletions
diff --git a/lib/mixlib/shellout/unix.rb b/lib/mixlib/shellout/unix.rb index 55a0372..d7b8b92 100644 --- a/lib/mixlib/shellout/unix.rb +++ b/lib/mixlib/shellout/unix.rb @@ -82,8 +82,16 @@ module Mixlib end end self + rescue Errno::ENOENT + # When ENOENT happens, we can be reasonably sure that the child process + # is going to exit quickly, so we use the blocking variant of waitpid2 + Process.waitpid2(@child_pid) rescue nil + raise rescue Exception - # do our best to kill zombies + # For exceptions other than ENOENT, such as timeout, we can't be sure + # how long the child process will live, so we use the non-blocking + # variant of waitpid2. This can result in zombie processes when the + # child later dies. See MIXLIB-16 for proposed enhancement. Process.waitpid2(@child_pid, Process::WNOHANG) rescue nil raise ensure diff --git a/spec/mixlib/shellout_spec.rb b/spec/mixlib/shellout_spec.rb index 89f21e7..23d9a13 100644 --- a/spec/mixlib/shellout_spec.rb +++ b/spec/mixlib/shellout_spec.rb @@ -777,6 +777,38 @@ describe Mixlib::ShellOut do end end + context "when running a command that doesn't exist" do + + let(:cmd) { "/bin/this-is-not-a-real-command" } + + def shell_out_cmd + Mixlib::ShellOut.new(cmd) + end + + it "reaps zombie processes after exec fails [OHAI-455]" do + # NOTE: depending on ulimit settings, GC, etc., before the OHAI-455 patch, + # ohai could also exhaust the available file descriptors when creating this + # many zombie processes. A regression _could_ cause Errno::EMFILE but this + # probably won't be consistent on different environments. + created_procs = 0 + 100.times do + begin + shell_out_cmd.run_command + rescue Errno::ENOENT + created_procs += 1 + end + end + created_procs.should == 100 + reaped_procs = 0 + begin + loop { Process.wait(-1); reaped_procs += 1 } + rescue Errno::ECHILD + end + reaped_procs.should == 0 + end + end + + context 'with open files for parent process' do before do @test_file = Tempfile.new('fd_test') diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0eab6ba..8d2b7c9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -6,7 +6,6 @@ require 'tmpdir' require 'tempfile' require 'timeout' -require 'ap' WATCH = lambda { |x| ap x } unless defined?(WATCH) |