Fix MIXLIB-17

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)