Skip to content

Commit b0b65ec

Browse files
pks-tttaylorr
authored andcommitted
compat/mingw: share file handles created via CreateFileW()
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 d641097 (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 090a308 (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 <[email protected]> Signed-off-by: Taylor Blau <[email protected]>
1 parent fd37853 commit b0b65ec

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

compat/mingw.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ static int mingw_open_append(wchar_t const *wfilename, int oflags, ...)
502502
* to append to the file.
503503
*/
504504
handle = CreateFileW(wfilename, FILE_APPEND_DATA,
505-
FILE_SHARE_WRITE | FILE_SHARE_READ,
505+
FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE,
506506
NULL, create, FILE_ATTRIBUTE_NORMAL, NULL);
507507
if (handle == INVALID_HANDLE_VALUE) {
508508
DWORD err = GetLastError();
@@ -1006,7 +1006,7 @@ int mingw_utime (const char *file_name, const struct utimbuf *times)
10061006

10071007
osfilehandle = CreateFileW(wfilename,
10081008
FILE_WRITE_ATTRIBUTES,
1009-
0 /*FileShare.None*/,
1009+
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
10101010
NULL,
10111011
OPEN_EXISTING,
10121012
(attrs != INVALID_FILE_ATTRIBUTES &&

0 commit comments

Comments
 (0)