Push tags=True blocks on github #145
Description
Activity
I did some digging.
Basically behind the scenes when you call
remote.push(tags=True)the command that is hanging is this guy:proc = Popen( [ 'git', 'push', '--porcelain', '--tags', 'origin', ], cwd='/path/to/head/', stdin=None, stderr=PIPE, stdout=PIPE, )
Basically in our repo if you go into a shell an call that command followed by
proc.wait()it will never finish. However if you drop thePIPEs then it does finish. So the command would look something like this:proc = Popen( [ 'git', 'push', '--porcelain', '--tags', 'origin', ], cwd='/path/to/head/', stdin=None, #stderr=PIPE, #stdout=PIPE, )
A few things to note. I tried running
push(tags=True)from a few different repos to see if it was repo specific.- Repo with no tags - works
- Repo with few tags (less than 10) - works
- Repo with a lot of tags (over 1200) - does not work (hangs)
This only recently started happening. We currently have 1254 tags and it started sometime in the last 10-15ish tags.
It should also be noted that the tags do actually get pushed, however the
push(tags=True)hangs.I think the issue is that with lots of tags, it fills up the buffer, so
waitwill never return. See this SO question for a reference.Here is another relevant solution.
Thanks for the digging.
Looking at the latest 0.3@f573b78 at this spot, it seems the default code path will use proc.communicate(), which in theory will work correctly.
The alternative code path will clearly be prone to blocking if there is a lot of output in stderr.Checking the
communicate()code of python 2.7.6 revealed that different implementations exist for windows and posix, both posix versions use eitherselect()orpoll(), which is assured to be absolutely non-blocking.In this particular issue however, we are looking at git/remote.py obviously first processes stderr output for progress, taking on the actual output later. If stdout fills while progress is being read from stderr, the git process is locking up.
Citing from the comment right above git/remote.py:552:
# read progress information from stderr # we hope stdout can hold all the data, it should ... # read the lines manually as it will use carriage returns between the messages # to override the previous one. This is why we read the bytes manually digest_process_messages(proc.stderr, progress)
That programmer with the multiple personalty issue was me, as a git blame revealed :).
To my mind, the entire push and pull code needs revision for this reason and others. For example, output parsing will fail if there are unexpected tokens.
- added 2 commits that reference this issue
on Aug 13, 2014 I will implement poll()-based parsing of stderr/stdout in case of fetch/pull/push now. The previous fixed was merged into a now obsolete branch, and would cost the real-time progress information that I was keen to have.
The chosen implementation uses
poll()if possible, but resorts to brute-force threads for 'pumping' lines otherwise. The only reason threads are less desirable is that they have more synchronisation overhead in general, and are note reused.
Using a threadpool could help alleviate this overhead.Please also note that to my surprise,
poll()is not available on py2 on OSX either, but is available on py3 on OSX.
On windows, there is nopoll()either, and the existingselect()call only works on sockets. Therefore using threads was the only viable alternate code-path that would work everywhere.
Also note that I managed to have the thread implementation occasionally fail on me, which could hint at some sync issue still present.Did you consider using kqueue on OS X?
Or stepping back, I wonder if you might use an existing event loop such as libev, libuv, etc.?
No, I didn't consider any of these, just because using Threads was 'good enough' as fallback.
If the named libraries wouldn't add another dependency, PR's would be very welcome.Yeah, I kind of figured that threads would be fairly easy and "good enough".
The other things would add dependencies and they're tricky ones as they're C extensions. You could use asyncio in Python 3.4 though that's a bit of work for a pretty small audience. There is a Python 2 port of asyncio called trollius, so I guess maybe it could be used on Python 2, but that's another dependency.
So probably what you have is good.
Let's hope so. After all, I still see these pseudo-random failures on py2, where lines on stderr get gobbled up as if characters are read out of order (which can't be for all I know) ... .
This kind of worries me, as it strongly points to my Thread implementation being somewhat incorrect. But I don't see the issue, maybe you can have a look ?
Interestingly, the issue seems to only show if I runtox.
As of last week, we could push tags using gitpython without any issue. Now it blocks forever. Neither git nor gitpython has been upgraded. This problem is occurring for multiple developers.
git versions: 1.8.5.2, 1.7.0.4