From 0982eeb89c0cd913e6838ce8b47b5c11eae1578f Mon Sep 17 00:00:00 2001
From: Will Fleming <will@flemi.ng>
Date: Mon, 28 Dec 2015 14:26:33 -0500
Subject: [PATCH] Use .capture3 not .popen3 for external parsers

In my testing, I believe this will fix the frequent timeouts we've seen
happen on specific files.

Like other issues we'd been seeing, these were actually
non-deterministic, they just happened to *almost* always fail. But I did
see them succeed occasionally: this may be partially why I didn't catch
this in testing before the prior release, since I ran it against some of
the same projects we saw failing. I just got lucky the first time
around.

What I saw locally was that sometimes the status thread given by `.open3` would
be dead before it was asked for its value, and in those cases the call
seemed to succeed. If the thread was still alive when asked for its
value, it seemed to then hang indefinitely.

Looking at the code for [`.capture3`][1], the biggest difference seems
to be the use of threads for capturing stdout/stderr. There's even a
warning in [stdlib docs about deadlocking from not doing this][2]! So I
think the previous implementation was effectively equivalent to a poor,
buggy implementation of `.capture3`, and we should just use the real one. It's
unclear to me if the older usage of `IO.popen` also suffered from this
behavior and we just happened to be swallowing it, or if the pipe buffer
behavior is a little different there.

[1]: https://github.com/jruby/jruby/blob/3dbb634f2764b24bca9e92ecd3d52d859cc0e847/lib/ruby/stdlib/open3.rb#L258-L274
[2]: http://ruby-doc.org/stdlib-2.2.3/libdoc/open3/rdoc/Open3.html#method-c-popen3
---
 .../engine/analyzers/command_line_runner.rb   | 22 +++++--------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/lib/cc/engine/analyzers/command_line_runner.rb b/lib/cc/engine/analyzers/command_line_runner.rb
index 320dd728..66e09aae 100644
--- a/lib/cc/engine/analyzers/command_line_runner.rb
+++ b/lib/cc/engine/analyzers/command_line_runner.rb
@@ -14,23 +14,11 @@ def initialize(command, timeout = DEFAULT_TIMEOUT)
 
         def run(input)
           Timeout.timeout(timeout) do
-            Open3.popen3 command, "r+" do |stdin, stdout, stderr, wait_thr|
-              stdin.puts input
-              stdin.close
-
-              exit_code = wait_thr.value
-
-              output = stdout.gets
-              stdout.close
-
-              err_output = stderr.gets
-              stderr.close
-
-              if 0 == exit_code
-                yield output
-              else
-                raise ::CC::Engine::Analyzers::ParserError, "Python parser exited with code #{exit_code}:\n#{err_output}"
-              end
+            out, err, status = Open3.capture3(command, stdin_data: input)
+            if status.success?
+              yield out
+            else
+              raise ::CC::Engine::Analyzers::ParserError, "`#{command}` exited with code #{status.exitstatus}:\n#{err}"
             end
           end
         end