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

On Sat, Feb 6, 2010 at 11:18 PM, Johannes Sixt [off-list ref] wrote:

On Samstag, 6. Februar 2010, Erik Faye-Lund wrote:

quoted

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.

Background: On Unix, we need FD_CLOEXEC so that the fds that are meant for the
async thread do not remain open in an unrelated child process; on Windows, we
are just lucky and can get away without FD_CLOEXEC because our pipe()s are
non-inheritable and async only work with pipes. But once we pass other fds to
the async procedure, we need a working FD_CLOEXEC. Perhaps something in this
direction is related to your problem.

You could push out your current state of the git-daemon and a recipe to
reproduce the problem. Perhaps I find some time to look into it.

Sure. You can find my current version at
git://repo.or.cz/git/kusma.git work/daemon-fcntl

This branch includes your branch and my fcntl-attempt, as well as an
almost-fixed-up version of the last daemon-win32 series I sent out
(still lacking critical sections when saving process ids, as you
suggested).

-- 
Erik "kusma" Faye-Lund