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

Author: pbrisbin

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

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