lib/mini_magick/shell.rb in mini_magick-4.9.0 vs lib/mini_magick/shell.rb in mini_magick-4.9.1

@@ -36,44 +36,38 @@
     private
 
     def execute_open3(command, options = {})
       require "open3"
 
-      in_w, out_r, err_r, subprocess_thread = Open3.popen3(*command)
+      # We would ideally use Open3.capture3, but it wouldn't allow us to
+      # terminate the command after timing out.
+      Open3.popen3(*command) do |in_w, out_r, err_r, thread|
+        [in_w, out_r, err_r].each(&:binmode)
+        stdout_reader = Thread.new { out_r.read }
+        stderr_reader = Thread.new { err_r.read }
+        begin
+          in_w.write options[:stdin].to_s
+        rescue Errno::EPIPE
+        end
+        in_w.close
 
-      capture_command(in_w, out_r, err_r, subprocess_thread, options)
+        begin
+          Timeout.timeout(MiniMagick.timeout, nil, "MiniMagick command timed out: #{command}") { thread.join }
+        ensure
+          Process.kill("TERM", thread.pid) rescue nil
+          Process.waitpid(thread.pid)      rescue nil
+        end
+
+        [stdout_reader.value, stderr_reader.value, thread.value]
+      end
     end
 
     def execute_posix_spawn(command, options = {})
       require "posix-spawn"
       child = POSIX::Spawn::Child.new(*command, input: options[:stdin].to_s, timeout: MiniMagick.timeout)
       [child.out, child.err, child.status]
     rescue POSIX::Spawn::TimeoutExceeded
-      raise Timeout::Error
-    end
-
-    def capture_command(in_w, out_r, err_r, subprocess_thread, options)
-      [in_w, out_r, err_r].each(&:binmode)
-      stdout_reader = Thread.new { out_r.read }
-      stderr_reader = Thread.new { err_r.read }
-      begin
-        in_w.write options[:stdin].to_s
-      rescue Errno::EPIPE
-      end
-      in_w.close
-
-      Timeout.timeout(MiniMagick.timeout) { subprocess_thread.join }
-
-      [stdout_reader.value, stderr_reader.value, subprocess_thread.value]
-    rescue Timeout::Error => error
-      begin
-        Process.kill("TERM", subprocess_thread.pid)
-      rescue Errno::ESRCH
-        # ignore if the PID doesn't exist
-      end
-      raise error
-    ensure
-      [out_r, err_r].each(&:close)
+      raise Timeout::Error, "MiniMagick command timed out: #{command}"
     end
 
     def log(command, &block)
       value = nil
       duration = Benchmark.realtime { value = block.call }