git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Johannes Sixt <j6t@kdbg.org>,
	 Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	 Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/2] compat/mingw: fix EACCESS when opening files with `O_CREAT | O_EXCL`
Date: Thu, 20 Mar 2025 11:37:45 +0100	[thread overview]
Message-ID: <20250320-b4-pks-mingw-lockfile-flake-v2-0-a84c90cfc6c2@pks.im> (raw)
In-Reply-To: <20250313-b4-pks-mingw-lockfile-flake-v1-0-bc5d3e70f516@pks.im>

Hi,

I finally found some time to have a look at why t0610 is failing
regularly in MinGW. As it turns out the root cause is our emulation of
open(3p): when trying to open a file with `_wopen(..., O_CREAT|O_EXCL)`
the call fails in case another process has marked the same file for
deletion via `DeleteFileW()`. This gets triggered by t0610 because we
race around locking the reftable stack and thus causes the failure.

The fix is simple: we get `ERROR_ACCESS_DENIED` in this situation, so
instead of translating that error to `EACCESS` we translate it to
`EEXIST`. This fixes the flake on my machine, but as usual when it comes
to Windows I would very much like to ask those in the know to point out
any obvious mistakes I did.

The other patch is a while-at-it patch that I was wondering about while
debugging the issue. It's not needed and I'm happy to drop it if you
don't think we should include it.

Changes in v2:
  - Make the workaround more specific by also paying attention to the
    NtStatus code. Like this, we only translate the error when we see
    that the error code was `STATUS_DELETE_PENDING`, which should rule
    out that the translation triggers in unintended cases.
  - A new patch for Meson that makes us pull in "compat/msvc.c" instead
    of "compat/mingw.c". This is more of a while-at-it fix that I
    spotted while working on this patch series. It doesn't have any
    ramifications for what I'm doing.
  - Drop the patch that makes us handle O_CLOEXEC. It's not needed, and
    I'd rather focus on changes that actually improve the situation.
  - Link to v1: https://lore.kernel.org/r/20250313-b4-pks-mingw-lockfile-flake-v1-0-bc5d3e70f516@pks.im

Thanks!

Patrick

---
Patrick Steinhardt (2):
      meson: fix compat sources when compiling with MSVC
      compat/mingw: fix EACCESS when opening files with `O_CREAT | O_EXCL`

 compat/mingw.c | 20 ++++++++++++++++++++
 meson.build    |  4 +++-
 2 files changed, 23 insertions(+), 1 deletion(-)

Range-diff versus v1:

1:  c2c1f988729 < -:  ----------- compat/mingw: handle O_CLOEXEC in `mingw_open_existing()`
2:  fd698866034 < -:  ----------- compat/mingw: fix EACCESS when opening files with `O_CREAT | O_EXCL`
-:  ----------- > 1:  9a2798b1b63 meson: fix compat sources when compiling with MSVC
-:  ----------- > 2:  ff5bf477747 compat/mingw: fix EACCESS when opening files with `O_CREAT | O_EXCL`

---
base-commit: 4b68faf6b93311254efad80e554780e372deb42f
change-id: 20250313-b4-pks-mingw-lockfile-flake-49dfcce8e7c2


  parent reply	other threads:[~2025-03-20 10:37 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-13 14:17 [PATCH 0/2] compat/mingw: fix EACCESS when opening files with `O_CREAT | O_EXCL` Patrick Steinhardt
2025-03-13 14:17 ` [PATCH 1/2] compat/mingw: handle O_CLOEXEC in `mingw_open_existing()` Patrick Steinhardt
2025-03-13 17:52   ` Junio C Hamano
2025-03-13 14:17 ` [PATCH 2/2] compat/mingw: fix EACCESS when opening files with `O_CREAT | O_EXCL` Patrick Steinhardt
2025-03-13 18:02   ` Junio C Hamano
2025-03-16  0:01   ` Johannes Schindelin
2025-03-17 15:16     ` Patrick Steinhardt
2025-03-20 10:37 ` Patrick Steinhardt [this message]
2025-03-20 10:37   ` [PATCH v2 1/2] meson: fix compat sources when compiling with MSVC Patrick Steinhardt
2025-03-20 10:37   ` [PATCH v2 2/2] compat/mingw: fix EACCESS when opening files with `O_CREAT | O_EXCL` Patrick Steinhardt
2025-03-26 12:20     ` Johannes Schindelin
2025-03-28  9:20       ` Patrick Steinhardt
2025-03-28 15:41         ` Johannes Schindelin
2025-03-31  6:57           ` Patrick Steinhardt

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=20250320-b4-pks-mingw-lockfile-flake-v2-0-a84c90cfc6c2@pks.im \
    --to=ps@pks.im \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    /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).