diff options
| -rw-r--r-- | lib/mixlib/shellout/unix.rb | 22 | ||||
| -rw-r--r-- | spec/mixlib/shellout_spec.rb | 16 |
2 files changed, 35 insertions, 3 deletions
diff --git a/lib/mixlib/shellout/unix.rb b/lib/mixlib/shellout/unix.rb index 776169d..d7082e1 100644 --- a/lib/mixlib/shellout/unix.rb +++ b/lib/mixlib/shellout/unix.rb @@ -178,6 +178,26 @@ module Mixlib STDIN.sync = true if input end + # When a new process is started with chef, it shares the file + # descriptors of the parent. We clean the file descriptors + # coming from the parent to prevent unintended locking if parent + # is killed. + # NOTE: After some discussions we've decided to iterate on file + # descriptors upto 256. We believe this is a reasonable upper + # limit in a chef environment. If we have issues in the future this + # number could be made to be configurable or updated based on + # 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 + end + end + end + def configure_parent_process_file_descriptors # Close the sides of the pipes we don't care about stdin_pipe.first.close @@ -231,6 +251,8 @@ module Mixlib fork do configure_subprocess_file_descriptors + clean_parent_file_descriptors + set_group set_user set_environment diff --git a/spec/mixlib/shellout_spec.rb b/spec/mixlib/shellout_spec.rb index 14af6e2..e4fe504 100644 --- a/spec/mixlib/shellout_spec.rb +++ b/spec/mixlib/shellout_spec.rb @@ -203,7 +203,7 @@ describe Mixlib::ShellOut do context "with options hash" do let(:cmd) { 'brew install couchdb' } - let(:options) { { :cwd => cwd, :user => user, :domain => domain, :password => password, :group => group, + let(:options) { { :cwd => cwd, :user => user, :domain => domain, :password => password, :group => group, :umask => umask, :timeout => timeout, :environment => environment, :returns => valid_exit_codes, :live_stream => stream, :input => input } } @@ -422,12 +422,12 @@ describe Mixlib::ShellOut do let(:user) { 'administrator' } let(:domain) { ENV['COMPUTERNAME'].downcase } let(:options) { { :domain => domain, :user => user, :password => 'vagrant' } } - + it "should run as Administrator" do running_user.should eql("#{domain}\\#{user}") end end - end + end context "with a live stream" do let(:stream) { StringIO.new } @@ -762,6 +762,16 @@ describe Mixlib::ShellOut do end 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" } + + it "should not see file descriptors of the parent" do + test_file = Tempfile.new('fd_test') + stdout.chomp.should eql("3") + test_file.close + end + end + context 'with subprocess that takes longer than timeout' do let(:cmd) { ruby_eval.call('sleep 2') } let(:options) { { :timeout => 0.1 } } |