Re: [PATCH 0/4] t5552: fix flakiness by introducing proper locking for GIT_TRACE

On 8/10/2018 12:51 PM, Jeff Hostetler wrote:

On 8/10/2018 12:15 PM, Johannes Sixt wrote:

quoted

Am 09.08.2018 um 19:35 schrieb Johannes Schindelin via GitGitGadget:

quoted

I reported a couple of times that t5552 is not passing reliably. It 
has now
reached next, and will no doubt infect master soon.

Turns out that it is not a Windows-specific issue, even if it occurs 
a lot
more often on Windows than elsewhere.

The culprit is that two processes try simultaneously to write to the 
same
file specified via GIT_TRACE_PACKET, and it is not well defined how that
should work, even on Linux.

Thanks for digging down to the root cause. As has been said, the 
behavior of O_APPEND is well-defined under POSIX, but last time I 
looked for equivalent feature on Windows, I did not find any.

Last time was when I worked around the same failure in 
t5503-tagfollow.sh in my private builds:

https://github.com/j6t/git/commit/9a447a6844b50b43746d9765b3ac809e2793d742 


It is basically the same as Peff suggests: log only one side of the 
fetch.

As this buglet looks like a recurring theme, and a proper fix is 
preferable over repeated work-arounds. To me it looks like we need 
some sort of locking on Windows. Unless your friends at Microsoft have 
an ace in their sleeves that lets us have atomic O_APPEND the POSIX 
way...

The official Windows CRT for open() does document an O_APPEND, but after
looking at the distributed source, I'm not sure I trust it.

I have not looked at the MSYS version of the CRT yet so I cannot comment
on it.

I did find that the following code does what we want.  Notice the use of
FILE_APPEND_DATA in place of the usual GENERIC_READ.  (And yes, the docs
are very vague here.)

d'oh

s/GENERIC_READ/GENERIC_WRITE/

Before we try a locking solution, perhaps we should tweak mingw_open()
to use something like this to get a proper HANDLE directly and then use
_open_osfhandle() to get a "fd" for it.

Jeff

---------------------------------------------------------------------

#include <Windows.h>

int main()
{
     HANDLE h = CreateFileW(L"foo.txt",
         FILE_APPEND_DATA,
         FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
         NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);

     const char * buf = "this is a test\n";
     DWORD len = strlen(buf);
     DWORD nbw;

     WriteFile(h, buf, len, &nbw, NULL);
     CloseHandle(h);

     return 0;
}