git_wrapper.run deadlocks with lots of stderr about pygitup HOT 7 CLOSED

My suggested fix is missing a "if tostdout:" before printing to stdout.

        if tostdout:
          sys.stdout.write(stdout.decode('utf-8'))
          sys.stdout.flush()

from pygitup.

Hey, thanks for reporting! That's an interesting edge case. I'll need a couple more details to look into this. Which version of Python are you using? Also, how exactly does the deadlock play out? Does PyGitUp just hang forever? Is there any output before the deadlock happens?

from pygitup.

from pygitup.

So I've thought a little about how we can solve this. It appears to me we have the following options:

• Increase the buffer size. This is only a band-aid really, as we don't know beforehand how much stderr output there will be.
• The solution you proposed seems to just wait for the subprocess to finish and then process stdout and stderr. This avoids the buffer problem but it also means we print the contents stdout after the subprocess finished instead of printing it live. Right now, we only need stdout printing for git fetch. So we could probably have a special implementation that does live output the way we do it now. This implementation then would only be used for git fetch where we probably won't run into buffering problems.
• A more general solution would be to move stderr processing off to a separate thread (using something like this: http://stackoverflow.com/a/31867499/997063). This would be the most clean solution but would introduce multithreading as a potential source of problems. And I guess it may be just plain overkill.

What are your thoughts on this?

from pygitup.

from pygitup.

I've implemented a fix for the deadlock problem following option 2 I outlined earlier. In theory it may still deadlock when running git fetch but I doubt this command will be able to fill the buffer (default is 8 KB) with error messages.

Do you still have the repository that used to cause the deadlock? If so, could you verify the fix works? Of course you'd have to install PyGitUp from the latest master first.

from pygitup.

Closing due to inactivity, feel free to re-open if this is still an issue.

from pygitup.