More robust pipe checks and tests.

2 files changed, 17 insertions, 9 deletions

diff --git a/lib/mixlib/shellout/unix.rb b/lib/mixlib/shellout/unix.rb
index d7082e1..bd20ffb 100644
--- a/lib/mixlib/shellout/unix.rb
+++ b/lib/mixlib/shellout/unix.rb
@@ -189,11 +189,13 @@ module Mixlib
       # the ulimit based on platform.
       def clean_parent_file_descriptors
         # Don't clean $stdin, $stdout, $stderr, process_status_pipe.
-        # Also 3 & 4 is reserved by RubyVM
-        5.upto(256) do |n|
-          fd = File.for_fd(n) rescue nil
-          if fd && process_status_pipe.last.to_i != n
-            fd.close
+        3.upto(256) do |n|
+          # We are checking the fd for error pipe before attempting to
+          # create a file because error pipe will auto close when we
+          # try to create a file since it's set to CLOEXEC.
+          if n != @process_status_pipe.last.to_i
+            fd = File.for_fd(n) rescue nil
+            fd.close if fd
           end
         end
       end
diff --git a/spec/mixlib/shellout_spec.rb b/spec/mixlib/shellout_spec.rb
index e4fe504..2020dda 100644
--- a/spec/mixlib/shellout_spec.rb
+++ b/spec/mixlib/shellout_spec.rb
@@ -763,12 +763,18 @@ describe Mixlib::ShellOut do
       end
 
       context 'with open files for parent process' do
-        let(:ruby_code) { "count = 0; 0.upto(256) do |n| fd = File.for_fd(n) rescue nil; count += 1 if fd end; puts count" }
+        before do
+          @test_file = Tempfile.new('fd_test')
+        end
+
+        after do
+          @test_file.close if @test_file
+        end
+
+        let(:ruby_code) { "fd = File.for_fd(#{@test_file.to_i}) rescue nil; puts fd.nil?" }
 
         it "should not see file descriptors of the parent" do
-          test_file = Tempfile.new('fd_test')
-          stdout.chomp.should eql("3")
-          test_file.close
+          stdout.chomp.should eql("true")
         end
       end