From 8028216fa1df436e23ace97169743dda265bbcd1 Mon Sep 17 00:00:00 2001
From: patrick brisbin <pbrisbin@gmail.com>
Date: Mon, 1 Feb 2016 12:56:40 -0500
Subject: [PATCH 1/2] Add spec coverage on CommandLineRunner

---
 .../analyzers/command_line_runner_spec.rb     | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 spec/cc/engine/analyzers/command_line_runner_spec.rb

diff --git a/spec/cc/engine/analyzers/command_line_runner_spec.rb b/spec/cc/engine/analyzers/command_line_runner_spec.rb
new file mode 100644
index 00000000..0206d7be
--- /dev/null
+++ b/spec/cc/engine/analyzers/command_line_runner_spec.rb
@@ -0,0 +1,31 @@
+require "spec_helper"
+require "cc/engine/duplication"
+
+module CC::Engine::Analyzers
+  RSpec.describe CommandLineRunner do
+    describe "#run" do
+      it "runs the command on the input and yields the output" do
+        runner = CommandLineRunner.new("cat; echo hi")
+
+        output = runner.run("oh ") { |o| o }
+
+        expect(output).to eq "oh hi\n"
+      end
+
+
+      it "raises on errors" do
+        runner = CommandLineRunner.new("echo error output >&2; false")
+
+        expect { runner.run("") }.to raise_error(
+          ParserError, /code 1:\nerror output/
+        )
+      end
+
+      it "times out commands" do
+        runner = CommandLineRunner.new("sleep 3", 0.01)
+
+        expect { runner.run("") }.to raise_error(Timeout::Error)
+      end
+    end
+  end
+end

From 8c9b4e9297167bef1edef84a4aefd84dece96283 Mon Sep 17 00:00:00 2001
From: patrick brisbin <pbrisbin@gmail.com>
Date: Mon, 1 Feb 2016 12:57:54 -0500
Subject: [PATCH 2/2] Handle race condition in JRuby's capture3

capture3 on JRuby will assign a pid variable as the result of spawn[1].
It then passes that to Process.detach[2]. If the process in question has
exited between these two statements, Process.detach will return nil.

It can't be determined then if the process had succeeded or not. Rather
than always assuming an error, we use a heuristic: if the output
produced parses as valid JSON, we should consider it successful.

[1]: https://github.com/jruby/jruby/blob/master/lib/ruby/stdlib/open3.rb#L200
[2]: https://github.com/jruby/jruby/blob/master/lib/ruby/stdlib/open3.rb#L201
---
 .../engine/analyzers/command_line_runner.rb   | 22 +++++++++++++++++++
 .../analyzers/command_line_runner_spec.rb     | 21 ++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/lib/cc/engine/analyzers/command_line_runner.rb b/lib/cc/engine/analyzers/command_line_runner.rb
index 66e09aae..59d0a698 100644
--- a/lib/cc/engine/analyzers/command_line_runner.rb
+++ b/lib/cc/engine/analyzers/command_line_runner.rb
@@ -15,6 +15,9 @@ def initialize(command, timeout = DEFAULT_TIMEOUT)
         def run(input)
           Timeout.timeout(timeout) do
             out, err, status = Open3.capture3(command, stdin_data: input)
+
+            status ||= handle_open3_race_condition(out)
+
             if status.success?
               yield out
             else
@@ -26,6 +29,25 @@ def run(input)
         private
 
         attr_reader :command, :timeout
+
+        # Work around a race condition in JRuby's Open3.capture3 that can lead
+        # to a nil status returned. We'll consider the process successful if it
+        # produced output that can be parsed as JSON.
+        #
+        # https://github.com/jruby/jruby/blob/master/lib/ruby/stdlib/open3.rb#L200-L201
+        #
+        def handle_open3_race_condition(out)
+          JSON.parse(out)
+          NullStatus.new(true, 0)
+        rescue JSON::ParserError
+          NullStatus.new(false, 1)
+        end
+
+        NullStatus = Struct.new(:success, :exitstatus) do
+          def success?
+            success
+          end
+        end
       end
     end
   end
diff --git a/spec/cc/engine/analyzers/command_line_runner_spec.rb b/spec/cc/engine/analyzers/command_line_runner_spec.rb
index 0206d7be..5de8e5b1 100644
--- a/spec/cc/engine/analyzers/command_line_runner_spec.rb
+++ b/spec/cc/engine/analyzers/command_line_runner_spec.rb
@@ -26,6 +26,27 @@ module CC::Engine::Analyzers
 
         expect { runner.run("") }.to raise_error(Timeout::Error)
       end
+
+      context "when Open3 returns a nil status" do
+        it "accepts it if the output parses as JSON" do
+          runner = CommandLineRunner.new("")
+
+          allow(Open3).to receive(:capture3).and_return(["{\"type\":\"issue\"}", "", nil])
+
+          output = runner.run("") { |o| o }
+          expect(output).to eq "{\"type\":\"issue\"}"
+        end
+
+        it "raises if the output was not valid JSON" do
+          runner = CommandLineRunner.new("")
+
+          allow(Open3).to receive(:capture3).and_return(["", "error output", nil])
+
+          expect { runner.run("") }.to raise_error(
+            ParserError, /code 1:\nerror output/
+          )
+        end
+      end
     end
   end
 end