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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 10, 2018, 18:34 UTC
Message-ID: <xmqqin4i83zg.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <811ded48-6f33-c46e-7bae-b9f7c7e8764c@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 5 quoted lines

> 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...

Just to put the severity of the issue in context, we use O_APPEND in a few codepaths, and the trace thing for debugging is the only thing that could have multiple writers. Other users of O_APPEND are:

 * refs/files-backend.c uses it so that a reflog entry can be
   appended at the end, but because update to each ref is protected
   from racing at a lot higher layer with a lock, no two people
   would try to append to the same reflog file, so atomicity of
   O_APPEND does not matter here.

 * sequencer.c wants to use it when moving one insn from the todo
   list to the 'done' list when it finishes one operation.  If you
   are running two sequences in a single repository, intermixed
   'done' list would be the least of your problem, so presumably we
   are fine here.

It may make sense to allow GIT_TRACE to have a placeholder (e.g. "/tmp/traceout.$$") to help debuggers arrange to give different processes their own trace output file, which perhaps may be a simpler and less impactful to the performance solution than having to make locks at an application layer.