Re: [PATCH] mingw: enable atomic O_APPEND

> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> The Windows CRT implements O_APPEND "manually": on write() calls, the
>> file pointer is set to EOF before the data is written. Clearly, this is
>> not atomic. And in fact, this is the root cause of failures observed in
>> t5552-skipping-fetch-negotiator.sh and t5503-tagfollow.sh, where
>> different processes write to the same trace file simultanously; it also
>> occurred in t5400-send-pack.sh, but there it was worked around in
>> 71406ed4d6 ("t5400: avoid concurrent writes into a trace file",
>> 2017-05-18).
>>
>> Fortunately, Windows does support atomic O_APPEND semantics using the
>> file access mode FILE_APPEND_DATA. Provide an implementation that does.
>>
>> This implementation is minimal in such a way that it only implements
>> the open modes that are actually used in the Git code base. Emulation
>> for other modes can be added as necessary later. To become aware of
>> the necessity early, the unusal error ENOSYS is reported if an
>> unsupported mode is encountered.
>>
>> Diagnosed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>> Helped-by: Jeff Hostetler <git@jeffhostetler.com>
>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>> ---
>>   compat/mingw.c | 41 +++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 39 insertions(+), 2 deletions(-)
> 
> Nice.
> 
> I wonder how much more expensive using this implementation is
> compared with the original "race susceptible" open(), when raciness
> is known not to be an issue (e.g. there is higher level lock that
> protects the appending).

> ...[define race_safe_append_open]... and replace
> the call to open(... O_APPEND ...) in trace.c::get_trace_fd() with a
> call to that wrapper.  That way, other codepaths that use O_APPEND
> (namely, reflog and todo-list writers) can avoid the additional
> cost, if any.
> 
> Some may find it beneficial from code readability POV because that
> approach marks the codepath that needs to have non-racy fd more
> explicitly.