[WIP] brew search freezes on some GitHub API responses
#2466
Conversation
We also had this issue, which was caused by However, I cannot reproduce this. |
|
@reitermarkus It may work for most users today but it’s bound to break for everyone eventually. I have isolated a small test case that can be run from Terminal without Homebrew. If you run the following command line from Bash (doesn’t matter which version of Ruby/MRI you have installed), does it return after 2 seconds, or does it freeze for you?
|
|
@claui I get only |
|
freezes saying Reading stdout |
|
@vitorgalvao @monouser7dig Thank you both. This is basically what the method |
|
I have added a spec which now fails reliably. It triggers the bug in I expect this test to fail on every system, including Travis. |
|
Wow, great work writing a test case for this and your general debugging skills @claui! I need to look at this in more detail but just wanted to give a 👍 before I do so. Thanks again! |
|
Thanks a bunch @MikeMcQuaid! Happy you find it helpful 😊 |
|
(to clarify by "fixed" I mean "fixed for |
|
@MikeMcQuaid 👍 for #2540! That said, the |
Agreed.
The most important thing to me it that Homebrew just works – no matter if involuntarily or with a proper (by whose definition?) fix. That said, |
|
@claui Seems reasonable to me 👍 Will leave this open for you to be able to work on it further. Thanks again! |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
I’m actively working on unit tests. While doing so, I have discovered (and smoothed out) a couple of edge cases in the fix. What do I do to make that bot go away? |
|
@claui Your posting a comment was enough. We'd love to see more, smaller PRs if that helps this progress at all. Thanks! |
|
@MikeMcQuaid I have opened a smaller PR as per your suggestion. That makes it easier indeed! |
|
I have now updated this PR with the completed fix. It builds upon #2762, which is currently under review. To see the relevant diff, you can just zoom in on the individual commit. |
This commit sets the stage for an upcoming fix for #2466. In a nutshell, this is what it does: - In `cask/system_command.rb`, factor out existing code (“IO selection”) that we’re going to need later in order to fix PR #2466; - move said code into its own class `Utils::IOSelector`; - split the class into smaller methods to make the code more expressive than before; and - add unit tests for `Utils::IOSelector` (they’re a bit bloated because edge cases.)
This commit adds a couple of tests for the utility methods in `utils/curl.rb`, including a few common and edge cases. One of the edge cases with large stdout/stderr outputs should fail due to an issue in the `curl_output` method. Note that this commit also introduces a mock for the `/usr/bin/curl` binary, which gets injected into the methods under test via the `HOMEBREW_CURL` environment variable. References: - #2466 - https://github.com/Homebrew/brew/blob/d84655efb9dba5840d75e08da5b69f5fd0c56744/Library/Homebrew/utils/curl.rb#L40
This addition to `io_selector.rb` prevents the `curl_output` method from blocking indefinitely for certain outputs of the curl binary. This also fixes the failing test in `curl_test.rb`. Reference: - #2466
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
brew testswith your changes locally?Note: work in progress – DO NOT MERGE
Update: Fixed the
curlsize report.Update 2: Updated next steps; also, added instructions to use
HOMEBREW_NO_GITHUB_APIto temporarily work around the issue.Update 3: Clarified description; added two tasks (reuse parts of PR #21665; review commits)
The issue
Since yesterday (7th of April 2017),
brew searchfreezes indefinitely on my system:The issue is reproducible on my system and happens 100 % of the time. It also affects
brew installwhen I misspell a formula, which also triggers a search.The freeze seems to be unrecoverable; for example, I’ve left it for over three hours as of right now, with no noticeable progress.
There is some evidence that the freeze specifically occurs while
brew searchqueries the GitHub API to enumerate all the formulae and casks. Notably, whenever it gets to thecaskroom/casktap, the size of the response grows to more than 1 MiB:My environment
brew tap
The output of
brew tapis:brew doctor
This is what
brew doctorsays:brew config
The output of
brew configis:Other environment variables
Lastly, here are some of the environment variables my shell exports to Homebrew:
Analysis
The title bar of my Terminal window shows that the freeze always occurs while
curlis the active process.However, while e. g.
brew search hellois stuck, runningpgrep -afl brew(in another Terminal window) reveals that the freeze always happens during the one specific GitHub API query which asks for the file tree of thecaskroom/casktap.This is what the output of
pgrep -afl brewsays:My network connection seems fine;
curlwill freeze no matter whether I’m at work or at home.According to
stat, the temporary file which is supposed to receive the API response headers seems stuck at 0 bytes size, and timestamps matching the spawn time of thecurlprocess:However, whenever I do the same
curlcommand manually, it works perfectly fine:Suspected cause
There’s something fishy about the response I get from the above
curlcommand when I count the lines:which returns:
You’ll notice that the length of the API response is just barely over 1 MiB, a number that has a suspicious smell to me. A hint of an I/O-related deadlock?
Last year, we actually had an issue in Homebrew-Cask with similar symptoms. Turned out that the
Hbc::SystemCommandclass would not handle stdout/stderr streams properly, leading to I/O starvation and eventually to a freeze (which happened as soon as the starved stream grew big enough to fill Ruby’s internal I/O buffers, I suppose).The issue has since been fixed in
Hbc::SystemCommand; however, something similar might contribute to the issue at hand. Who knows.Known workarounds
HOMEBREW_NO_GITHUB_API=1 brew searchinstead ofbrew search. This temporarily keeps Homebrew from fetching the API response.Next steps
I plan to continue working on this right away; that’s why I made it a PR, not an issue.
Anyone is free to give it a shot though!
@Homebrew/maintainers Feel free to assign core, do not merge and in progress labels if you want.
Figure out where in the code the specific
curlcall happensFigure out whether the code handles I/O streams properly (see examples for safe I/O handling in Ruby) Update: looks like it doesn’t.
Write tests to reproduce the behavior, possibly similar to our existing tests for the
Hbc::SystemCommandclassApply a fix similar to the one inUpdate: Factor out a piece of code from PR #21665 first; that code has been in production for a while so it might make sense to reuse itApply fix to
curl_output, reusing the factored-out codeReview commits