Am 10.08.2018 um 18:51 schrieb Jeff Hostetler:
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 looked at the CRT code, too, and no, we can't 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.)
As far as I understand, FILE_APPEND_DATA is just a permission flag (I'm
not sure why that would be necessary), but at least the documentation
states that it is still necessary for the caller to set the file pointer.
But I haven't tried your code below, yet. Should it append the line of
text on each invocation? And if so, is it an atomic operation?
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;
}
