Skip to content

Commit 8c9b4e9

Browse files
committed
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
1 parent 8028216 commit 8c9b4e9

File tree

2 files changed

+43
-0
lines changed

2 files changed

+43
-0
lines changed

lib/cc/engine/analyzers/command_line_runner.rb

+22
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ def initialize(command, timeout = DEFAULT_TIMEOUT)
1515
def run(input)
1616
Timeout.timeout(timeout) do
1717
out, err, status = Open3.capture3(command, stdin_data: input)
18+
19+
status ||= handle_open3_race_condition(out)
20+
1821
if status.success?
1922
yield out
2023
else
@@ -26,6 +29,25 @@ def run(input)
2629
private
2730

2831
attr_reader :command, :timeout
32+
33+
# Work around a race condition in JRuby's Open3.capture3 that can lead
34+
# to a nil status returned. We'll consider the process successful if it
35+
# produced output that can be parsed as JSON.
36+
#
37+
# https://github.com/jruby/jruby/blob/master/lib/ruby/stdlib/open3.rb#L200-L201
38+
#
39+
def handle_open3_race_condition(out)
40+
JSON.parse(out)
41+
NullStatus.new(true, 0)
42+
rescue JSON::ParserError
43+
NullStatus.new(false, 1)
44+
end
45+
46+
NullStatus = Struct.new(:success, :exitstatus) do
47+
def success?
48+
success
49+
end
50+
end
2951
end
3052
end
3153
end

spec/cc/engine/analyzers/command_line_runner_spec.rb

+21
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,27 @@ module CC::Engine::Analyzers
2626

2727
expect { runner.run("") }.to raise_error(Timeout::Error)
2828
end
29+
30+
context "when Open3 returns a nil status" do
31+
it "accepts it if the output parses as JSON" do
32+
runner = CommandLineRunner.new("")
33+
34+
allow(Open3).to receive(:capture3).and_return(["{\"type\":\"issue\"}", "", nil])
35+
36+
output = runner.run("") { |o| o }
37+
expect(output).to eq "{\"type\":\"issue\"}"
38+
end
39+
40+
it "raises if the output was not valid JSON" do
41+
runner = CommandLineRunner.new("")
42+
43+
allow(Open3).to receive(:capture3).and_return(["", "error output", nil])
44+
45+
expect { runner.run("") }.to raise_error(
46+
ParserError, /code 1:\nerror output/
47+
)
48+
end
49+
end
2950
end
3051
end
3152
end

0 commit comments

Comments
 (0)