Re: [msysGit] upload-pack timing issue on windows?

[Erik Faye-Lund <kusmabite@gmail.com>]

On Mon, Feb 8, 2010 at 1:18 PM, Erik Faye-Lund <kusmabite@googlemail.com> wrote:
> On Sat, Feb 6, 2010 at 11:18 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>> On Samstag, 6. Februar 2010, Erik Faye-Lund wrote:
>>> However, I have tracked down a bit of what goes on in the client.
>>> There's a call to read_in_full, called from pack-write.c, line 246
>>> that fails in the failure-case, but not in the success-case. This is
>>> where the client expects "pack\tSHA-1" or "keep\tSHA-1". There "fatal:
>>> early EOF"-messages seems to originate from index-pack.c, line 197.
>>> This is the first line of code in parse_pack_header(), it's also
>>> AFAICT the first call-site for any read(0, <...>) (though fill()).
>>
>> This looks like upload-pack died without sending enough to fill a pack header.
>>
>> Try merging this branch:
>>
>>  git://repo.or.cz/git/mingw/j6t.git async-in-thread
>>
>> It contains your changes to start_async plus a refinement of die() when it is
>> called from the async procedure (it passes t5530, for example). It is also
>> converted to pthreads, and therefore also works on Unix. The new
>> implementation of start_async is more careful about the file handles, though
>> not so much on Windows.
>>
>> If there's no change for you, then you could look into implementing
>> fcntl(F_GETFD/SETFD, FD_CLOEXEC), which are currently ignored, on top of this
>> branch, using Get/SetHandleInformation().
>>
>
> Thanks a lot. I tried merging it, but the issue still pops up. I also
> tried to implement fcntl(F_GETFD/SETFD, FD_CLOEXEC), still no dice.
> I'm not entirely sure if I did it correctly, though.
>

More than 6 months later, and I've finally bothered to debug this further:
- The culprit seems to be our poll-emulation. My understanding is that
poll() was called by create_pack_file() in upload-pack.c with nfds=1
(it's 2 until one of the fds are closed) when there's no data
available in the pipe. Since our poll() always returns POLLIN when
nfds=1, the check for xread(...) == 0 further down in
create_pack_file() cause the fd to be closed, leading to an error on
the client-side.
-  Just removing the nfds=1-hack works for me, but I'm suspecting the
nfds=1-hack is there for some socket-reason. So instead I've replaced
our poll-emulation with gnulib's in my branch (with a couple of
patches on top), and it seems to do the trick for me. I still haven't
tested it heavily, though.
- The easiest way I've found to debug the issue, is to use git-fetch
from localhost with a repo with a single commit with a single, empty
file. Doing this triggers the bug every time for me, and doesn't hide
what's going on as much as git-clone does.

The latest version of my branch can be found here:
http://repo.or.cz/w/git/kusma.git/shortlog/refs/heads/work/daemon-win32-process