Merge pull request #70 from opscode/ryan/issue_2062_12

Add buffering to the process status pipe (CHEF 2062)

2 files changed, 19 insertions, 9 deletions

diff --git a/lib/mixlib/shellout.rb b/lib/mixlib/shellout.rb
index a379ee8..2b53d02 100644
--- a/lib/mixlib/shellout.rb
+++ b/lib/mixlib/shellout.rb
@@ -144,7 +144,7 @@ module Mixlib
     #   cmd = Mixlib::ShellOut.new("apachectl", "start", :user => 'www', :env => nil, :cwd => '/tmp')
     #   cmd.run_command # etc.
     def initialize(*command_args)
-      @stdout, @stderr = '', ''
+      @stdout, @stderr, @process_status = '', '', ''
       @live_stdout = @live_stderr = nil
       @input = nil
       @log_level = :debug
diff --git a/lib/mixlib/shellout/unix.rb b/lib/mixlib/shellout/unix.rb
index 2bde64a..40d7efa 100644
--- a/lib/mixlib/shellout/unix.rb
+++ b/lib/mixlib/shellout/unix.rb
@@ -229,7 +229,7 @@ module Mixlib
       # Some patch levels of ruby in wide use (in particular the ruby 1.8.6 on OSX)
       # segfault when you IO.select a pipe that's reached eof. Weak sauce.
       def open_pipes
-        @open_pipes ||= [child_stdout, child_stderr]
+        @open_pipes ||= [child_stdout, child_stderr, child_process_status]
       end
 
       # Keep this unbuffered for now
@@ -241,11 +241,10 @@ module Mixlib
 
       def attempt_buffer_read
         ready = IO.select(open_pipes, nil, nil, READ_WAIT_TIME)
-        if ready && ready.first.include?(child_stdout)
-          read_stdout_to_buffer
-        end
-        if ready && ready.first.include?(child_stderr)
-          read_stderr_to_buffer
+        if ready
+          read_stdout_to_buffer if ready.first.include?(child_stdout)
+          read_stderr_to_buffer if ready.first.include?(child_stderr)
+          read_process_status_to_buffer if ready.first.include?(child_process_status)
         end
         ready
       end
@@ -270,6 +269,15 @@ module Mixlib
         open_pipes.delete(child_stderr)
       end
 
+      def read_process_status_to_buffer
+        while chunk = child_process_status.read_nonblock(READ_SIZE)
+          @process_status << chunk
+        end
+      rescue Errno::EAGAIN
+      rescue EOFError
+        open_pipes.delete(child_process_status)
+      end
+
       def fork_subprocess
         initialize_ipc
 
@@ -311,12 +319,14 @@ module Mixlib
       # assume everything went well.
       def propagate_pre_exec_failure
         begin
-          e = Marshal.load child_process_status
+          attempt_buffer_read until child_process_status.eof?
+          e = Marshal.load(@process_status)
           raise(Exception === e ? e : "unknown failure: #{e.inspect}")
-        rescue EOFError # If we get an EOF error, then the exec was successful
+        rescue ArgumentError # If we get an ArgumentError error, then the exec was successful
           true
         ensure
           child_process_status.close
+          open_pipes.delete(child_process_status)
         end
       end