Re: [PATCH 0/4] t5552: fix flakiness by introducing proper locking for GIT_TRACE
On 8/10/2018 12:51 PM, Jeff Hostetler wrote:
Show 45 quoted lines
>
>
> On 8/10/2018 12:15 PM, Johannes Sixt wrote:
>> Am 09.08.2018 um 19:35 schrieb Johannes Schindelin via GitGitGadget:
>>> 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/
Show 27 quoted lines
>
> 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;
> }