This repository was archived by the owner on Jul 19, 2025. It is now read-only.

Commit 0982eeb

committed

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

1 parent f528659 commit 0982eebCopy full SHA for 0982eeb

File tree

1 file changed

-17

lines changed

lib/cc/engine/analyzers
- command_line_runner.rb

1 file changed

-17

lines changed

`‎lib/cc/engine/analyzers/command_line_runner.rb‎`

Lines changed: 5 additions & 17 deletions

Original file line number	Diff line number	Diff line change
`@@ -14,23 +14,11 @@ def initialize(command, timeout = DEFAULT_TIMEOUT)`
`14`	`14`
`15`	`15`	`def run(input)`
`16`	`16`	`Timeout.timeout(timeout) do`
`17`		`- Open3.popen3 command, "r+" do \|stdin, stdout, stderr, wait_thr\|`
`18`		`- stdin.puts input`
`19`		`- stdin.close`
`20`		`-`
`21`		`- exit_code = wait_thr.value`
`22`		`-`
`23`		`- output = stdout.gets`
`24`		`- stdout.close`
`25`		`-`
`26`		`- err_output = stderr.gets`
`27`		`- stderr.close`
`28`		`-`
`29`		`- if 0 == exit_code`
`30`		`- yield output`
`31`		`- else`
`32`		`- raise ::CC::Engine::Analyzers::ParserError, "Python parser exited with code #{exit_code}:\n#{err_output}"`
`33`		`- end`
	`17`	`+ out, err, status = Open3.capture3(command, stdin_data: input)`
	`18`	`+ if status.success?`
	`19`	`+ yield out`
	`20`	`+ else`
	`21`	+ raise ::CC::Engine::Analyzers::ParserError, "`#{command}` exited with code #{status.exitstatus}:\n#{err}"
`34`	`22`	`end`
`35`	`23`	`end`
`36`	`24`	`end`

0 commit comments

Comments

(0)

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit 0982eeb

File tree

1 file changed

1 file changed

`‎lib/cc/engine/analyzers/command_line_runner.rb‎`

0 commit comments