From: Johannes Sixt <j6t@kdbg.org>
To: Jeff King <peff@peff.net>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: t5400 failure on Windows
Date: Tue, 16 May 2017 20:35:46 +0200	[thread overview]
Message-ID: <afca6bf5-472e-dda4-2c16-a2256080ac51@kdbg.org> (raw)
In-Reply-To: <20170515222406.hxab2wrapv75ybmj@sigill.intra.peff.net>
Am 16.05.2017 um 00:24 schrieb Jeff King:
>    4. There is something racy and unportable about both programs writing
>       to the same trace file. It's opened with O_APPEND, which means that
>       each write should atomically position the pointer at the end of the
>       file. Is it possible there's a problem with that in the way
>       O_APPEND works on Windows?
> 
>       If that was the case, though, I'd generally expect a sheared write
>       or a partial overwrite. The two origin/HEAD lines from the two
>       programs are the exact same length, but I'd find it more likely for
>       the overwrite to happen with one of the follow-on lines.
Good guesswork! O_APPEND is not atomic on Windows, in particular, it is 
emulated as lseek(SEEK_END) followed by write().
I ran the test a few more times, and it fails in different ways 
(different missing lines) and also succeeds in a minority of the cases.
Windows is capable of writing atomically in the way O_APPEND requires 
(keyword: FILE_APPEND_DATA), but I do not see a way to wedge this into 
the emulation layer. For the time being, I think I have to skip the test 
case.
The question remains how endangered our uses of O_APPEND are when the 
POSIX semantics are not obeyed precisely:
* trace.c: It is about debugging. Not critical.
* sequencer.c: It writes the "done" file. No concurrent accesses are 
expected: Not critial.
* refs/files-backend.c: There are uses in functions 
open_or_create_logfile() and log_ref_setup(). Sounds like it is in 
reflogs. Sounds like this is about reflogs. If there are concurrent 
accesses, there is a danger that a reflog is not recorded correctly. 
Then reflogs may be missing and objects may be pruned earlier than 
expected. That's something to worry about, but I would not count it as 
mission critical.
-- Hannes
next prev parent reply	other threads:[~2017-05-16 18:35 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-15 20:05 t5400 failure on Windows Johannes Sixt
2017-05-15 22:24 ` Jeff King
2017-05-16 18:35   ` Johannes Sixt [this message]
2017-05-17  5:44     ` Jeff King
2017-05-17 18:41       ` Johannes Sixt
2017-05-18  5:02         ` [PATCH] t5400: avoid concurrent writes into a trace file Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox
  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):
  git send-email \
    --in-reply-to=afca6bf5-472e-dda4-2c16-a2256080ac51@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY
  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
  Be sure your reply has a Subject: header at the top and a blank line
  before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).