Avoid deadlocks in spawning commands

Author: ianfixes

CHANGELOG.md

@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Removed
 
 ### Fixed
+- Replaced pipes with `Open3.capture3` to avoid deadlocks when commands have too much output
 
 ### Security
 

lib/arduino_ci/arduino_cmd.rb

@@ -114,12 +114,18 @@ def set_pref(key, value)
     end
 
     # run the arduino command
-    def _run(*args, **kwargs)
+    # @return [bool] whether the command succeeded
+    def _run_and_output(*args, **kwargs)
       raise "Ian needs to implement this in a subclass #{args} #{kwargs}"
     end
 
-    # build and run the arduino command
-    def run(*args, **kwargs)
+    # run the arduino command
+    # @return [Hash] keys for :success, :out, and :err
+    def _run_and_capture(*args, **kwargs)
+      raise "Ian needs to implement this in a subclass #{args} #{kwargs}"
+    end
+
+    def _wrap_run(work_fn, *args, **kwargs)
       # do some work to extract & merge environment variables if they exist
       has_env = !args.empty? && args[0].class == Hash
       env_vars = has_env ? args[0] : {}
@@ -129,26 +135,21 @@ def run(*args, **kwargs)
 
       shell_vars = env_vars.map { |k, v| "#{k}=#{v}" }.join(" ")
       @last_msg = " $ #{shell_vars} #{full_args.join(' ')}"
-      _run(*full_cmd, **kwargs)
+      work_fn.call(*full_cmd, **kwargs)
+    end
+
+    # build and run the arduino command
+    def run_and_output(*args, **kwargs)
+      _wrap_run((proc { |*a, **k| _run_and_output(*a, **k) }), *args, **kwargs)
     end
 
     # run a command and capture its output
     # @return [Hash] {:out => String, :err => String, :success => bool}
     def run_and_capture(*args, **kwargs)
-      pipe_out, pipe_out_wr = IO.pipe
-      pipe_err, pipe_err_wr = IO.pipe
-      our_kwargs = { out: pipe_out_wr, err: pipe_err_wr }
-      eventual_kwargs = our_kwargs.merge(kwargs)
-      success = run(*args, **eventual_kwargs)
-      pipe_out_wr.close
-      pipe_err_wr.close
-      str_out = pipe_out.read
-      str_err = pipe_err.read
-      pipe_out.close
-      pipe_err.close
-      @last_err = str_err
-      @last_out = str_out
-      { out: str_out, err: str_err, success: success }
+      ret = _wrap_run((proc { |*a, **k| _run_and_capture(*a, **k) }), *args, **kwargs)
+      @last_err = ret[:err]
+      @last_out = ret[:out]
+      ret
     end
 
     # run a command and don't capture its output, but use the same signature

lib/arduino_ci/arduino_cmd_linux.rb

@@ -39,8 +39,15 @@ def _lib_dir
     end
 
     # run the arduino command
-    def _run(*args, **kwargs)
-      @display_mgr.run(*args, **kwargs)
+    # @return [bool] whether the command succeeded
+    def _run_and_output(*args, **kwargs)
+      @display_mgr.run_and_output(*args, **kwargs)
+    end
+
+    # run the arduino command
+    # @return [Hash] keys for :success, :out, and :err
+    def _run_and_capture(*args, **kwargs)
+      @display_mgr.run_and_capture(*args, **kwargs)
     end
 
     def run_with_gui_guess(message, *args, **kwargs)

lib/arduino_ci/arduino_cmd_linux_builder.rb

@@ -21,10 +21,15 @@ def _lib_dir
     end
 
     # run the arduino command
-    # @param [Array<String>] Arguments for the run command
-    # @return [bool] Whether the command succeeded
-    def _run(*args, **kwargs)
-      Host.run(*args, **kwargs)
+    # @return [bool] whether the command succeeded
+    def _run_and_output(*args, **kwargs)
+      Host.run_and_output(*args, **kwargs)
+    end
+
+    # run the arduino command
+    # @return [Hash] keys for :success, :out, and :err
+    def _run_and_capture(*args, **kwargs)
+      Host.run_and_capture(*args, **kwargs)
     end
 
   end

lib/arduino_ci/arduino_cmd_osx.rb

@@ -14,8 +14,15 @@ class ArduinoCmdOSX < ArduinoCmd
     flag :verify,          "--verify"
 
     # run the arduino command
-    def _run(*args, **kwargs)
-      Host.run(*args, **kwargs)
+    # @return [bool] whether the command succeeded
+    def _run_and_output(*args, **kwargs)
+      Host.run_and_output(*args, **kwargs)
+    end
+
+    # run the arduino command
+    # @return [Hash] keys for :success, :out, and :err
+    def _run_and_capture(*args, **kwargs)
+      Host.run_and_capture(*args, **kwargs)
     end
 
     def _lib_dir

lib/arduino_ci/cpp_library.rb

@@ -108,23 +108,13 @@ def header_dirs
 
     # wrapper for the GCC command
     def run_gcc(*args, **kwargs)
-      pipe_out, pipe_out_wr = IO.pipe
-      pipe_err, pipe_err_wr = IO.pipe
       full_args = ["g++"] + args
       @last_cmd = " $ #{full_args.join(' ')}"
-      our_kwargs = { out: pipe_out_wr, err: pipe_err_wr }
-      eventual_kwargs = our_kwargs.merge(kwargs)
-      success = Host.run(*full_args, **eventual_kwargs)
 
-      pipe_out_wr.close
-      pipe_err_wr.close
-      str_out = pipe_out.read
-      str_err = pipe_err.read
-      pipe_out.close
-      pipe_err.close
-      @last_err = str_err
-      @last_out = str_out
-      success
+      ret = Host.run_and_capture(*full_args, **kwargs)
+      @last_err = ret[:err]
+      @last_out = ret[:out]
+      ret[:success]
     end
 
     # Return the GCC version
@@ -214,7 +204,7 @@ def run_test_file(executable)
       @last_cmd = executable
       @last_out = ""
       @last_err = ""
-      Host.run(executable)
+      Host.run_and_output(executable)
     end
 
   end

lib/arduino_ci/display_manager.rb

@@ -150,26 +150,30 @@ def with_display
       end
     end
 
-    # run a command in a display
-    # @return [bool]
-    def run(*args, **kwargs)
-      ret = false
+    def wrap_run(work_fn, *args, **kwargs)
+      ret = nil
       # do some work to extract & merge environment variables if they exist
       has_env = !args.empty? && args[0].class == Hash
       with_display do |env_vars|
         env_vars = {} if env_vars.nil?
         env_vars.merge!(args[0]) if has_env
         actual_args = has_env ? args[1..-1] : args  # need to shift over if we extracted args
         full_cmd = env_vars.empty? ? actual_args : [env_vars] + actual_args
-        ret = Host.run(*full_cmd, **kwargs)
+        ret = work_fn.call(*full_cmd, **kwargs)
       end
       ret
     end
 
-    # run a command in a display with no output
+    # run a command in a display, outputting to stdout
+    # @return [bool]
+    def run_and_output(*args, **kwargs)
+      wrap_run((proc { |*a, **k| Host.run_and_output(*a, **k) }), *args, **kwargs)
+    end
+
+    # run a command in a display, capturing output
     # @return [bool]
-    def run_silent(*args)
-      run(*args, out: File::NULL, err: File::NULL)
+    def run_and_capture(*args, **kwargs)
+      wrap_run((proc { |*a, **k| Host.run_and_capture(*a, **k) }), *args, **kwargs)
     end
 
     # @return [Hash] the environment variables for the display

lib/arduino_ci/host.rb

@@ -1,4 +1,5 @@
 require 'os'
+require 'open3'
 
 module ArduinoCI
 
@@ -20,8 +21,12 @@ def self.which(cmd)
       nil
     end
 
-    # run a command in a display
-    def self.run(*args, **kwargs)
+    def self.run_and_capture(*args, **kwargs)
+      stdout, stderr, status = Open3.capture3(*args, **kwargs)
+      { out: stdout, err: stderr, success: status.exitstatus.zero? }
+    end
+
+    def self.run_and_output(*args, **kwargs)
       system(*args, **kwargs)
     end