From ceb69e62664b3f63d1d9318ee590ec26e9e4340a Mon Sep 17 00:00:00 2001 From: Jake Teton-Landis Date: Sun, 4 Sep 2016 16:45:38 -0700 Subject: [PATCH 1/2] Allow closing a Vagrant::Util::Subprocess's STDIN Previously, there was no way to close the STDIN stream of a subprocess, so commands that read from stdin in a subprocess would hang forever, such as `/bin/sh -s`. If one tried to close the stdin, the IO.select() call in Subprocess#execute would raise an error for calling select() on a closed IO. Here's a concrete example of a command that needs to close STDIN to work properly: ```ruby script = SOME_VERY_LONG_STRING command = %w(ssh foo.example.com /bin/sh -s foo bar) result = ::Vagrant::Util::Subprocess.execute(*command) do |type, data_or_io| if type == :stdin data_or_io.write(script) data_or_io.write("\n") data_or_io.close next end puts "Remote: #{data_or_io}" end ``` --- lib/vagrant/util/subprocess.rb | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/vagrant/util/subprocess.rb b/lib/vagrant/util/subprocess.rb index d69286b33..75bc101cc 100644 --- a/lib/vagrant/util/subprocess.rb +++ b/lib/vagrant/util/subprocess.rb @@ -144,10 +144,11 @@ module Vagrant # Record the start time for timeout purposes start_time = Time.now.to_i + open_readers = [stdout, stderr] + open_writers = notify_stdin ? [process.io.stdin] : [] @logger.debug("Selecting on IO") while true - writers = notify_stdin ? [process.io.stdin] : [] - results = ::IO.select([stdout, stderr], writers, nil, 0.1) + results = ::IO.select(open_readers, open_writers, nil, 0.1) results ||= [] readers = results[0] writers = results[1] @@ -178,8 +179,14 @@ module Vagrant break if process.exited? # Check the writers to see if they're ready, and notify any listeners - if writers && !writers.empty? - yield :stdin, process.io.stdin if block_given? + if writers && !writers.empty? && block_given? + yield :stdin, process.io.stdin + + # if the callback closed stdin, we should remove it, because + # IO.select() will throw if called with a closed io. + if process.io.stdin.closed? + open_writers = [] + end end end From 51f68f41a23161871c64691ddca366b36abdbc8c Mon Sep 17 00:00:00 2001 From: Jake Teton-Landis Date: Thu, 15 Sep 2016 18:19:01 -0700 Subject: [PATCH 2/2] Unit test Vagrant::Util::Subprocess's STDIN support - create new unit test file for this class, as none existed. - test `Vagrant::Util::Subprocess#execute` behavior relating to STDIN handling. --- test/unit/vagrant/util/subprocess_test.rb | 50 +++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 test/unit/vagrant/util/subprocess_test.rb diff --git a/test/unit/vagrant/util/subprocess_test.rb b/test/unit/vagrant/util/subprocess_test.rb new file mode 100644 index 000000000..3934d1a3c --- /dev/null +++ b/test/unit/vagrant/util/subprocess_test.rb @@ -0,0 +1,50 @@ +require File.expand_path("../../../base", __FILE__) +require "vagrant/util/subprocess" + +describe Vagrant::Util::Subprocess do + describe '#execute' do + before do + # ensure we have `cat` and `echo` in our PATH so that we can run these + # tests successfully. + ['cat', 'echo'].each do |cmd| + if !Vagrant::Util::Which.which(cmd) + pending("cannot run subprocess tests without command #{cmd.inspect}") + end + end + end + + let (:cat) { described_class.new('cat', :notify => [:stdin]) } + + it 'yields the STDIN stream for the process if we set :notify => :stdin' do + echo = described_class.new('echo', 'hello world', :notify => [:stdin]) + echo.execute do |type, data| + expect(type).to eq(:stdin) + expect(data).to be_a(::IO) + end + end + + it 'can close STDIN' do + result = cat.execute do |type, stdin| + # We should be able to close STDIN without raising an exception + stdin.close + end + + # we should exit successfully. + expect(result.exit_code).to eq(0) + end + + it 'can write to STDIN correctly' do + data = "hello world\n" + result = cat.execute do |type, stdin| + stdin.write(data) + stdin.close + end + + # we should exit successfully. + expect(result.exit_code).to eq(0) + + # we should see our data as the output from `cat` + expect(result.stdout).to eq(data) + end + end +end