diff options
| -rw-r--r-- | CHANGELOG.md | 5 | ||||
| -rw-r--r-- | Gemfile | 5 | ||||
| -rw-r--r-- | README.md | 2 | ||||
| -rw-r--r-- | Rakefile | 7 | ||||
| -rw-r--r-- | appveyor.yml | 31 | ||||
| -rw-r--r-- | lib/mixlib/shellout/version.rb | 2 | ||||
| -rw-r--r-- | lib/mixlib/shellout/windows.rb | 8 | ||||
| -rw-r--r-- | mixlib-shellout-windows.gemspec | 8 | ||||
| -rw-r--r-- | mixlib-shellout-x86-mingw32.gemspec | 9 | ||||
| -rw-r--r-- | spec/mixlib/shellout_spec.rb | 225 |
10 files changed, 177 insertions, 125 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index b64b9ea..ca1a924 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +## Release: 2.0.1 + +* add buffering to the child process status pipe to fix chef-client deadlocks +* fix timeouts on Windows + ## Release: 2.0.0 * remove LC_ALL=C default setting, consumers should now set this if they @@ -3,9 +3,10 @@ source 'https://rubygems.org' gemspec :name => "mixlib-shellout" group(:test) do - gem "rspec_junit_formatter" gem 'rake' - end +group(:development) do + gem 'pry' +end @@ -39,7 +39,7 @@ Invoke crontab to edit user cron: ## Windows Impersonation Example Invoke "whoami.exe" to demonstrate running a command as another user: - whomai = Mixlib::ShellOut.new("whoami.exe", :user => "username", :domain => "DOMAIN", :password => "password") + whoami = Mixlib::ShellOut.new("whoami.exe", :user => "username", :domain => "DOMAIN", :password => "password") whoami.run_command ## Platform Support @@ -1,20 +1,19 @@ require 'rspec/core/rake_task' require 'rubygems/package_task' +require 'mixlib/shellout/version' Dir[File.expand_path("../*gemspec", __FILE__)].reverse.each do |gemspec_path| gemspec = eval(IO.read(gemspec_path)) Gem::PackageTask.new(gemspec).define end -require 'mixlib/shellout/version' - desc "Run all specs in spec directory" RSpec::Core::RakeTask.new(:spec) do |t| t.pattern = FileList['spec/**/*_spec.rb'] end desc "Build it and ship it" -task :ship => [:clean, :gem] do +task ship: [:clobber_package, :gem] do sh("git tag #{Mixlib::ShellOut::VERSION}") sh("git push opscode --tags") Dir[File.expand_path("../pkg/*.gem", __FILE__)].reverse.each do |built_gem| @@ -22,4 +21,4 @@ task :ship => [:clean, :gem] do end end -task :default => :spec +task default: :spec diff --git a/appveyor.yml b/appveyor.yml new file mode 100644 index 0000000..de4779c --- /dev/null +++ b/appveyor.yml @@ -0,0 +1,31 @@ +version: "master-{build}" + +os: Windows Server 2012 +platform: + - x64 + +environment: + matrix: + - ruby_version: "200" + - ruby_version: "193" + +clone_folder: c:\projects\mixlib-shellout +clone_depth: 1 +skip_tags: true +branches: + only: + - master + +install: + - SET PATH=C:\Ruby%ruby_version%\bin;%PATH% + - echo %PATH% + - ruby --version + - gem --version + - gem install bundler --quiet --no-ri --no-rdoc + - bundler --version + +build_script: + - bundle install + +test_script: + - bundle exec rspec diff --git a/lib/mixlib/shellout/version.rb b/lib/mixlib/shellout/version.rb index e741dfb..2301857 100644 --- a/lib/mixlib/shellout/version.rb +++ b/lib/mixlib/shellout/version.rb @@ -1,5 +1,5 @@ module Mixlib class ShellOut - VERSION = "2.2.0.dev.0" + VERSION = "2.0.1" end end diff --git a/lib/mixlib/shellout/windows.rb b/lib/mixlib/shellout/windows.rb index 137aaa9..1612b09 100644 --- a/lib/mixlib/shellout/windows.rb +++ b/lib/mixlib/shellout/windows.rb @@ -111,6 +111,12 @@ module Mixlib when WAIT_TIMEOUT # Kill the process if (Time.now - start_wait) > timeout + begin + Process.kill(:KILL, process.process_id) + rescue Errno::EIO + logger.warn("Failed to kill timed out process #{process.process_id}") if logger + end + raise Mixlib::ShellOut::CommandTimeout, "command timed out:\n#{format_for_exception}" end @@ -156,7 +162,7 @@ module Mixlib begin next_chunk = stdout_read.readpartial(READ_SIZE) @stdout << next_chunk - @live_stream << next_chunk if @live_stream + @live_stdout << next_chunk if @live_stdout rescue EOFError stdout_read.close open_streams.delete(stdout_read) diff --git a/mixlib-shellout-windows.gemspec b/mixlib-shellout-windows.gemspec new file mode 100644 index 0000000..20c8164 --- /dev/null +++ b/mixlib-shellout-windows.gemspec @@ -0,0 +1,8 @@ +gemspec = eval(File.read(File.expand_path("../mixlib-shellout.gemspec", __FILE__))) + +gemspec.platform = Gem::Platform.new(["universal", "mingw32"]) + +gemspec.add_dependency "win32-process", "~> 0.7.5" +gemspec.add_dependency "windows-pr", "~> 1.2.4" + +gemspec diff --git a/mixlib-shellout-x86-mingw32.gemspec b/mixlib-shellout-x86-mingw32.gemspec deleted file mode 100644 index 2997a35..0000000 --- a/mixlib-shellout-x86-mingw32.gemspec +++ /dev/null @@ -1,9 +0,0 @@ -# x86-mingw32 Gemspec # -gemspec = eval(IO.read(File.expand_path("../mixlib-shellout.gemspec", __FILE__))) - -gemspec.platform = "x86-mingw32" - -gemspec.add_dependency "win32-process", "~> 0.7.1" -gemspec.add_dependency "windows-pr", "~> 1.2.2" - -gemspec diff --git a/spec/mixlib/shellout_spec.rb b/spec/mixlib/shellout_spec.rb index 3fda419..1765026 100644 --- a/spec/mixlib/shellout_spec.rb +++ b/spec/mixlib/shellout_spec.rb @@ -1,5 +1,6 @@ require 'spec_helper' require 'logger' +require 'timeout' describe Mixlib::ShellOut do let(:shell_cmd) { options ? shell_cmd_with_options : shell_cmd_without_options } @@ -538,19 +539,14 @@ describe Mixlib::ShellOut do let(:locale) { nil } context 'when running under Unix', :unix_only do - let(:parent_locale) { ENV['LC_ALL'].to_s.strip } - - it "should unset the parent process's locale" do + it "should unset the process's locale" do should eql("") end end context 'when running under Windows', :windows_only do - # On windows, if an environmental variable is not set, it returns the key - let(:parent_locale) { (ENV['LC_ALL'] || '%LC_ALL%').to_s.strip } - - it "should use the parent process's locale" do - should eql(parent_locale) + it "should unset process's locale" do + should eql('%LC_ALL%') end end end @@ -1032,142 +1028,157 @@ describe Mixlib::ShellOut do end - context 'with subprocess that takes longer than timeout', :unix_only do - def ruby_wo_shell(code) - parts = %w[ruby] - parts << "--disable-gems" if ruby_19? - parts << "-e" - parts << code - end - - let(:cmd) do - ruby_wo_shell(<<-CODE) - STDOUT.sync = true - trap(:TERM) { puts "got term"; exit!(123) } - sleep 10 - CODE - end + context 'with subprocess that takes longer than timeout' do let(:options) { { :timeout => 1 } } - it "should raise CommandTimeout" do - lambda { executed_cmd }.should raise_error(Mixlib::ShellOut::CommandTimeout) - end + context 'on windows', :windows_only do + let(:cmd) do + 'powershell -c "sleep 10"' + end - it "should ask the process nicely to exit" do - # note: let blocks don't correctly memoize if an exception is raised, - # so can't use executed_cmd - lambda { shell_cmd.run_command }.should raise_error(Mixlib::ShellOut::CommandTimeout) - shell_cmd.stdout.should include("got term") - shell_cmd.exitstatus.should == 123 + it "should raise CommandTimeout" do + Timeout::timeout(5) do + lambda { executed_cmd }.should raise_error(Mixlib::ShellOut::CommandTimeout) + end + end end - context "and the child is unresponsive" do + context 'on unix', :unix_only do + def ruby_wo_shell(code) + parts = %w[ruby] + parts << "--disable-gems" if ruby_19? + parts << "-e" + parts << code + end + let(:cmd) do ruby_wo_shell(<<-CODE) STDOUT.sync = true - trap(:TERM) { puts "nanana cant hear you" } + trap(:TERM) { puts "got term"; exit!(123) } sleep 10 CODE end - it "should KILL the wayward child" do + it "should raise CommandTimeout" do + lambda { executed_cmd }.should raise_error(Mixlib::ShellOut::CommandTimeout) + end + + it "should ask the process nicely to exit" do # note: let blocks don't correctly memoize if an exception is raised, # so can't use executed_cmd - lambda { shell_cmd.run_command}.should raise_error(Mixlib::ShellOut::CommandTimeout) - shell_cmd.stdout.should include("nanana cant hear you") - shell_cmd.status.termsig.should == 9 + lambda { shell_cmd.run_command }.should raise_error(Mixlib::ShellOut::CommandTimeout) + shell_cmd.stdout.should include("got term") + shell_cmd.exitstatus.should == 123 end - context "and a logger is configured" do - let(:log_output) { StringIO.new } - let(:logger) { Logger.new(log_output) } - let(:options) { {:timeout => 1, :logger => logger} } + context "and the child is unresponsive" do + let(:cmd) do + ruby_wo_shell(<<-CODE) + STDOUT.sync = true + trap(:TERM) { puts "nanana cant hear you" } + sleep 10 + CODE + end - it "should log messages about killing the child process" do + it "should KILL the wayward child" do # note: let blocks don't correctly memoize if an exception is raised, # so can't use executed_cmd lambda { shell_cmd.run_command}.should raise_error(Mixlib::ShellOut::CommandTimeout) shell_cmd.stdout.should include("nanana cant hear you") shell_cmd.status.termsig.should == 9 - - log_output.string.should include("Command exceeded allowed execution time, sending TERM") - log_output.string.should include("Command exceeded allowed execution time, sending KILL") end - end - end + context "and a logger is configured" do + let(:log_output) { StringIO.new } + let(:logger) { Logger.new(log_output) } + let(:options) { {:timeout => 1, :logger => logger} } - context "and the child process forks grandchildren" do - let(:cmd) do - ruby_wo_shell(<<-CODE) - STDOUT.sync = true - trap(:TERM) { print "got term in child\n"; exit!(123) } - fork do - trap(:TERM) { print "got term in grandchild\n"; exit!(142) } - sleep 10 + it "should log messages about killing the child process" do + # note: let blocks don't correctly memoize if an exception is raised, + # so can't use executed_cmd + lambda { shell_cmd.run_command}.should raise_error(Mixlib::ShellOut::CommandTimeout) + shell_cmd.stdout.should include("nanana cant hear you") + shell_cmd.status.termsig.should == 9 + + log_output.string.should include("Command exceeded allowed execution time, sending TERM") + log_output.string.should include("Command exceeded allowed execution time, sending KILL") end - sleep 10 - CODE - end - it "should TERM the wayward child and grandchild" do - # note: let blocks don't correctly memoize if an exception is raised, - # so can't use executed_cmd - lambda { shell_cmd.run_command}.should raise_error(Mixlib::ShellOut::CommandTimeout) - shell_cmd.stdout.should include("got term in child") - shell_cmd.stdout.should include("got term in grandchild") + end end - end - context "and the child process forks grandchildren that don't respond to TERM" do - let(:cmd) do - ruby_wo_shell(<<-CODE) - STDOUT.sync = true - - trap(:TERM) { print "got term in child\n"; exit!(123) } - fork do - trap(:TERM) { print "got term in grandchild\n" } + context "and the child process forks grandchildren" do + let(:cmd) do + ruby_wo_shell(<<-CODE) + STDOUT.sync = true + trap(:TERM) { print "got term in child\n"; exit!(123) } + fork do + trap(:TERM) { print "got term in grandchild\n"; exit!(142) } + sleep 10 + end sleep 10 - end - sleep 10 - CODE - end - - it "should TERM the wayward child and grandchild, then KILL whoever is left" do - # note: let blocks don't correctly memoize if an exception is raised, - # so can't use executed_cmd - lambda { shell_cmd.run_command}.should raise_error(Mixlib::ShellOut::CommandTimeout) - - begin - - # A little janky. We get the process group id out of the command - # object, then try to kill a process in it to make sure none - # exists. Trusting the system under test like this isn't great but - # it's difficult to test otherwise. - child_pgid = shell_cmd.send(:child_pgid) - initial_process_listing = `ps -j` + CODE + end + it "should TERM the wayward child and grandchild" do + # note: let blocks don't correctly memoize if an exception is raised, + # so can't use executed_cmd + lambda { shell_cmd.run_command}.should raise_error(Mixlib::ShellOut::CommandTimeout) shell_cmd.stdout.should include("got term in child") shell_cmd.stdout.should include("got term in grandchild") + end - kill_return_val = Process.kill(:INT, child_pgid) # should raise ESRCH - # AIX - kill returns code > 0 for error, where as other platforms return -1. Ruby code signal.c treats < 0 as error and raises exception and hence fails on AIX. So we check the return code for assertions since ruby wont raise an error here. - - if(kill_return_val == 0) - # Debug the failure: - puts "child pgid=#{child_pgid.inspect}" - Process.wait - puts "collected process: #{$?.inspect}" - puts "initial process listing:\n#{initial_process_listing}" - puts "current process listing:" - puts `ps -j` - raise "Failed to kill all expected processes" + end + context "and the child process forks grandchildren that don't respond to TERM" do + let(:cmd) do + ruby_wo_shell(<<-CODE) + STDOUT.sync = true + + trap(:TERM) { print "got term in child\n"; exit!(123) } + fork do + trap(:TERM) { print "got term in grandchild\n" } + sleep 10 + end + sleep 10 + CODE + end + + it "should TERM the wayward child and grandchild, then KILL whoever is left" do + # note: let blocks don't correctly memoize if an exception is raised, + # so can't use executed_cmd + lambda { shell_cmd.run_command}.should raise_error(Mixlib::ShellOut::CommandTimeout) + + begin + + # A little janky. We get the process group id out of the command + # object, then try to kill a process in it to make sure none + # exists. Trusting the system under test like this isn't great but + # it's difficult to test otherwise. + child_pgid = shell_cmd.send(:child_pgid) + initial_process_listing = `ps -j` + + shell_cmd.stdout.should include("got term in child") + shell_cmd.stdout.should include("got term in grandchild") + + kill_return_val = Process.kill(:INT, child_pgid) # should raise ESRCH + # AIX - kill returns code > 0 for error, where as other platforms return -1. Ruby code signal.c treats < 0 as error and raises exception and hence fails on AIX. So we check the return code for assertions since ruby wont raise an error here. + + if(kill_return_val == 0) + # Debug the failure: + puts "child pgid=#{child_pgid.inspect}" + Process.wait + puts "collected process: #{$?.inspect}" + puts "initial process listing:\n#{initial_process_listing}" + puts "current process listing:" + puts `ps -j` + raise "Failed to kill all expected processes" + end + rescue Errno::ESRCH + # this is what we want end - rescue Errno::ESRCH - # this is what we want end - end + end end end |