From 1a6ae81aa9c82ea716795b0949983ceb4c025453 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 Jun 2012 17:00:50 +0200 Subject: [PATCH] Require what to be notified for with block and Subprocess.execute There was an issue before where the stdin buffer would always have space so it would always yield that block and Ruby would spin at 100%. Now we require all callers to say what they want to listen for. This drops CPU down to almost nothing. See GH-832 --- lib/vagrant/driver/virtualbox_base.rb | 3 +++ lib/vagrant/easy/operations.rb | 2 +- lib/vagrant/util/subprocess.rb | 22 ++++++++++++++++++- .../support/isolated_environment.rb | 5 +++-- 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/lib/vagrant/driver/virtualbox_base.rb b/lib/vagrant/driver/virtualbox_base.rb index ceff5d0b7..9e7cd1868 100644 --- a/lib/vagrant/driver/virtualbox_base.rb +++ b/lib/vagrant/driver/virtualbox_base.rb @@ -314,6 +314,9 @@ module Vagrant @logger.info("Interrupted.") end + # Append in the options for subprocess + command << { :notify => [:stdout, :stderr] } + Util::Busy.busy(int_callback) do Subprocess.execute(@vboxmanage_path, *command, &block) end diff --git a/lib/vagrant/easy/operations.rb b/lib/vagrant/easy/operations.rb index 24114942f..932a38daf 100644 --- a/lib/vagrant/easy/operations.rb +++ b/lib/vagrant/easy/operations.rb @@ -63,7 +63,7 @@ module Vagrant end end - Vagrant::Util::Subprocess.execute(command, &block) + Vagrant::Util::Subprocess.execute(command, :notify => [:stdout, :stderr], &block) end # Run a shell command within the VM. The command will run within a diff --git a/lib/vagrant/util/subprocess.rb b/lib/vagrant/util/subprocess.rb index 622baf54c..6f3b70ad6 100644 --- a/lib/vagrant/util/subprocess.rb +++ b/lib/vagrant/util/subprocess.rb @@ -29,8 +29,27 @@ module Vagrant def execute # Get the timeout, if we have one timeout = @options[:timeout] + + # Get the working directory workdir = @options[:workdir] || Dir.pwd + # Get what we're interested in being notified about + notify = @options[:notify] || [] + notify = [notify] if !notify.is_a?(Array) + if notify.empty? && block_given? + # If a block is given, subscribers must be given, otherwise the + # block is never called. This is usually NOT what you want, so this + # is an error. + message = "A list of notify subscriptions must be given if a block is given" + raise ArgumentError, message + end + + # Let's get some more useful booleans that we access a lot so + # we're not constantly calling an `include` check + notify_stderr = notify.include?(:stderr) + notify_stdin = notify.include?(:stdin) + notify_stdout = notify.include?(:stdout) + # Build the ChildProcess @logger.info("Starting process: #{@command.inspect}") process = ChildProcess.build(*@command) @@ -80,7 +99,8 @@ module Vagrant @logger.debug("Selecting on IO") while true - results = IO.select([stdout, stderr], [process.io.stdin], nil, timeout || 5) + writers = notify_stdin ? [process.io.stdin] : [] + results = IO.select([stdout, stderr], writers, nil, timeout || 5) readers, writers = results # Check if we have exceeded our timeout diff --git a/test/acceptance/support/isolated_environment.rb b/test/acceptance/support/isolated_environment.rb index 0ae31965b..995e3e3c1 100644 --- a/test/acceptance/support/isolated_environment.rb +++ b/test/acceptance/support/isolated_environment.rb @@ -51,7 +51,8 @@ module Acceptance options = argN.last.is_a?(Hash) ? argN.pop : {} options = { :workdir => @workdir, - :env => @env + :env => @env, + :notify => [:stdin, :stderr, :stdout] }.merge(options) # Add the options to be passed on @@ -59,7 +60,7 @@ module Acceptance # Execute, logging out the stdout/stderr as we get it @logger.info("Executing: #{[command].concat(argN).inspect}") - Vagrant::Util::Subprocess.execute(command, *argN) do |type, data| + Vagrant::Util::Subprocess.execute(command *argN) do |type, data| @logger.debug("#{type}: #{data}") if type == :stdout || type == :stderr yield type, data if block_given? end