git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 1/3] compat/mingw: share file handles created via `CreateFileW()`

From
Patrick Steinhardt <ps@pks.im>
Date
Oct 27, 2024, 15:39 UTC
Message-ID
<63c2cfa87a4a3c44367acdc1f7cdb57289b4b761.1730042775.git.ps@pks.im>
In-Reply-To
<cover.1730042775.git.ps@pks.im>

Unless told otherwise, Windows will keep other processes from reading, writing and deleting files when one has an open handle that was created via `CreateFileW()`. This behaviour can be altered via `FILE_SHARE_*` flags:

  - `FILE_SHARE_READ` allows a concurrent process to open the file for
    reading.
  - `FILE_SHARE_WRITE` allows a concurrent process to open the file for
    writing.
  - `FILE_SHARE_DELETE` allows a concurrent process to delete the file
    or to replace it via an atomic rename.

This sharing mechanism is quite important in the context of Git, as we assume POSIX semantics all over the place. But there are two callsites where we don't pass all three of these flags:

  - We don't set `FILE_SHARE_DELETE` when creating a file for appending
    via `mingw_open_append()`. This makes it impossible to delete the
    file from another process or to replace it via an atomic rename. The
    function was introduced via d641097589 (mingw: enable atomic
    O_APPEND, 2018-08-13) and has been using `FILE_SHARE_READ |
    FILE_SHARE_WRITE` since the inception. There aren't any indicators
    that the omission of `FILE_SHARE_DELETE` was intentional.
  - We don't set any sharing flags in `mingw_utime()`, which changes the
    access and modification of a file. This makes it impossible to
    perform any kind of operation on this file at all from another
    process. While we only open the file for a short amount of time to
    update its timestamps, this still opens us up for a race condition
    with another process.
    `mingw_utime()` was originally implemented via `_wopen()`, which
    doesn't give you full control over the sharing mode. Instead, it
    calls `_wsopen()` with `_SH_DENYNO`, which ultimately translates to
    `FILE_SHARE_READ | FILE_SHARE_WRITE`. It was then refactored via
    090a3085bc (t/helper/test-chmtime: update mingw to support chmtime
    on directories, 2022-03-02) to use `CreateFileW()`, but we stopped
    setting any sharing flags at all, which seems like an unintentional
    side effect. By restoring `FILE_SHARE_READ | FILE_SHARE_WRITE` we
    thus fix this and get back the old behaviour of `_wopen()`.
    The fact that we didn't set the equivalent of `FILE_SHARE_DELETE`
    can be explained, as well: neither `_wopen()` nor `_wsopen()` allow
    you to do so. So overall, it doesn't seem intentional that we didn't
    allow deletions here, either.
Adapt both of these callsites to pass all three sharing flags.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 compat/mingw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 0e851ecae29..e326c6fcd2d 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -502,7 +502,7 @@ static int mingw_open_append(wchar_t const *wfilename, int oflags, ...)
 	 * to append to the file.
 	 */
 	handle = CreateFileW(wfilename, FILE_APPEND_DATA,
-			FILE_SHARE_WRITE | FILE_SHARE_READ,
+			FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE,
 			NULL, create, FILE_ATTRIBUTE_NORMAL, NULL);
 	if (handle == INVALID_HANDLE_VALUE) {
 		DWORD err = GetLastError();
@@ -1006,7 +1006,7 @@ int mingw_utime (const char *file_name, const struct utimbuf *times)
 
 	osfilehandle = CreateFileW(wfilename,
 				   FILE_WRITE_ATTRIBUTES,
-				   0 /*FileShare.None*/,
+				   FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
 				   NULL,
 				   OPEN_EXISTING,
 				   (attrs != INVALID_FILE_ATTRIBUTES &&
-- 
2.47.0.118.gfd3785337b.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 39 of 44 in “compat/mingw: implement POSIX-style atomic renames”
  1. 0/3 compat/mingw: implement POSIX-style atomic renamesPatrick Steinhardt, Oct 23, 2024
  2. 1/3 compat/mingw: share file handles created via `CreateFileW()`Patrick Steinhardt, Oct 23, 2024
  3. Kristoffer HaugsbakkOct 23, 2024
  4. Taylor BlauOct 23, 2024
  5. Taylor BlauOct 23, 2024
  6. Taylor BlauOct 23, 2024
  7. Patrick SteinhardtOct 24, 2024
  8. Johannes SixtOct 27, 2024
  9. Taylor BlauOct 27, 2024
  10. 2/3 compat/mingw: allow deletion of most opened filesPatrick Steinhardt, Oct 23, 2024
  11. Kristoffer HaugsbakkOct 23, 2024
  12. Taylor BlauOct 23, 2024
  13. Patrick SteinhardtOct 24, 2024
  14. Taylor BlauOct 23, 2024
  15. 3/3 compat/mingw: support POSIX semantics for atomic renamesPatrick Steinhardt, Oct 23, 2024
  16. Kristoffer HaugsbakkOct 23, 2024
  17. Patrick SteinhardtOct 24, 2024
  18. Kristoffer HaugsbakkOct 24, 2024
  19. Patrick SteinhardtOct 24, 2024
  20. Taylor BlauOct 23, 2024
  21. Taylor BlauOct 23, 2024
  22. 0/3 compat/mingw: implement POSIX-style atomic renamesPatrick Steinhardt, Oct 24, 2024
  23. 1/3 compat/mingw: share file handles created via `CreateFileW()`Patrick Steinhardt, Oct 24, 2024
  24. 2/3 compat/mingw: allow deletion of most opened filesPatrick Steinhardt, Oct 24, 2024
  25. Johannes SixtOct 27, 2024
  26. Patrick SteinhardtOct 27, 2024
  27. Taylor BlauOct 27, 2024
  28. Taylor BlauOct 27, 2024
  29. 3/3 compat/mingw: support POSIX semantics for atomic renamesPatrick Steinhardt, Oct 24, 2024
  30. Johannes SixtOct 27, 2024
  31. Patrick SteinhardtOct 27, 2024
  32. Johannes SixtOct 27, 2024
  33. Patrick SteinhardtOct 27, 2024
  34. Johannes SixtOct 27, 2024
  35. Taylor BlauOct 27, 2024
  36. Taylor BlauOct 24, 2024
  37. Johannes SixtOct 27, 2024
  38. 0/3 compat/mingw: implement POSIX-style atomic renamesPatrick Steinhardt, Oct 27, 2024
  39. 1/3 compat/mingw: share file handles created via `CreateFileW()`Patrick Steinhardt, Oct 27, 2024
  40. 2/3 compat/mingw: allow deletion of most opened filesPatrick Steinhardt, Oct 27, 2024
  41. 3/3 compat/mingw: support POSIX semantics for atomic renamesPatrick Steinhardt, Oct 27, 2024
  42. Junio C HamanoNov 6, 2024
  43. Johannes SixtNov 6, 2024
  44. Junio C HamanoNov 6, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.