* [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10
@ 2026-04-28 12:52 Johannes Schindelin via GitGitGadget
2026-04-28 12:52 ` [PATCH 1/2] mingw: optionally use legacy (non-POSIX) delete semantics Johannes Schindelin via GitGitGadget
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-04-28 12:52 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin
On Windows, maintenance_task_geometric_repack() opens pack index files via
pack_geometry_init() (which mmap()s the .idx files), then spawns git repack
as a child process without setting child.odb_to_close. The parent's mmap()s
prevent the child from deleting old .idx files.
On Windows 10 builds before the POSIX delete semantics change (between Build
17134.1304 and 18363.657, see https://stackoverflow.com/a/60512798), this
results in Unlink of file '.git/objects/pack/pack-<hash>.idx' failed. Should
I try again? during fetch-triggered auto-maintenance with the geometric
strategy.
The fix adds the missing child.odb_to_close = the_repository->objects line,
matching all other maintenance tasks.
The first commit introduces a GIT_TEST_LEGACY_DELETE environment variable to
simulate legacy (pre-POSIX) delete semantics on modern Windows, so the
regression test can verify the fix even on Windows 11.
This fixes https://github.com/git-for-windows/git/issues/6210.
Johannes Schindelin (2):
mingw: optionally use legacy (non-POSIX) delete semantics
maintenance(geometric): do release the `.idx` files before repacking
builtin/gc.c | 1 +
compat/mingw.c | 47 ++++++++++++++++++++++++++++++++++++++++--
t/t7900-maintenance.sh | 22 +++++++++++++++++++-
3 files changed, 67 insertions(+), 3 deletions(-)
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2103%2Fdscho%2Favoid-deadlocks-in-geometric-repacking-on-windows-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2103/dscho/avoid-deadlocks-in-geometric-repacking-on-windows-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2103
--
gitgitgadget
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] mingw: optionally use legacy (non-POSIX) delete semantics
2026-04-28 12:52 [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10 Johannes Schindelin via GitGitGadget
@ 2026-04-28 12:52 ` Johannes Schindelin via GitGitGadget
2026-05-04 14:12 ` Patrick Steinhardt
2026-04-28 12:52 ` [PATCH 2/2] maintenance(geometric): do release the `.idx` files before repacking Johannes Schindelin via GitGitGadget
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-04-28 12:52 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
At some point between Windows 10 Build 17134.1304 and Build 18363.657,
the default behavior of `DeleteFileW()` was changed to use POSIX
semantics (https://stackoverflow.com/a/60512798). Under those semantics,
a file can be deleted even when another process holds an active
`MapViewOfFile` view on it: the directory entry is removed immediately,
but the underlying data persists until the last handle is closed.
On older Windows versions (and Windows 10 builds before that change),
`DeleteFileW()` uses legacy semantics where deletion fails outright if
any process holds a file mapping.
To allow testing code paths that depend on the legacy behavior, introduce
a `GIT_TEST_LEGACY_DELETE` environment variable. When set, `mingw_unlink()`
uses `SetFileInformationByHandle()` with `FileDispositionInfo` (the
non-POSIX variant) instead of `DeleteFileW()`, forcing legacy delete
semantics regardless of the Windows version.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/mingw.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 2023c16db6..04f9aa3922 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -449,20 +449,63 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)
return wbuf;
}
+/*
+ * Use SetFileInformationByHandle(FileDispositionInfo) to force legacy
+ * (non-POSIX) delete semantics. On Windows 11, DeleteFileW() uses POSIX
+ * delete semantics internally, allowing deletion even with active
+ * MapViewOfFile views. This helper simulates Windows 10 behavior where
+ * deletion fails if a file mapping exists.
+ *
+ * Returns nonzero on success (like DeleteFileW), 0 on failure.
+ */
+static int legacy_delete_file(const wchar_t *wpathname)
+{
+ FILE_DISPOSITION_INFO fdi = { TRUE };
+ DWORD gle;
+ HANDLE h = CreateFileW(wpathname, DELETE,
+ FILE_SHARE_READ | FILE_SHARE_WRITE |
+ FILE_SHARE_DELETE,
+ NULL, OPEN_EXISTING,
+ FILE_FLAG_OPEN_REPARSE_POINT, NULL);
+ if (h == INVALID_HANDLE_VALUE)
+ return 0;
+
+ if (SetFileInformationByHandle(h, FileDispositionInfo,
+ &fdi, sizeof(fdi))) {
+ CloseHandle(h);
+ return 1;
+ }
+ gle = GetLastError();
+ CloseHandle(h);
+ SetLastError(gle);
+ return 0;
+}
+
+static int try_delete_file(const wchar_t *wpathname, int use_legacy)
+{
+ if (use_legacy)
+ return legacy_delete_file(wpathname);
+ return DeleteFileW(wpathname);
+}
+
int mingw_unlink(const char *pathname, int handle_in_use_error)
{
+ static int use_legacy_delete = -1;
int tries = 0;
wchar_t wpathname[MAX_PATH];
if (xutftowcs_path(wpathname, pathname) < 0)
return -1;
- if (DeleteFileW(wpathname))
+ if (use_legacy_delete < 0)
+ use_legacy_delete = !!getenv("GIT_TEST_LEGACY_DELETE");
+
+ if (try_delete_file(wpathname, use_legacy_delete))
return 0;
do {
/* read-only files cannot be removed */
_wchmod(wpathname, 0666);
- if (!_wunlink(wpathname))
+ if (try_delete_file(wpathname, use_legacy_delete))
return 0;
if (!is_file_in_use_error(GetLastError()))
break;
--
gitgitgadget
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] maintenance(geometric): do release the `.idx` files before repacking
2026-04-28 12:52 [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10 Johannes Schindelin via GitGitGadget
2026-04-28 12:52 ` [PATCH 1/2] mingw: optionally use legacy (non-POSIX) delete semantics Johannes Schindelin via GitGitGadget
@ 2026-04-28 12:52 ` Johannes Schindelin via GitGitGadget
2026-04-28 15:01 ` [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10 Derrick Stolee
2026-05-07 12:51 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
3 siblings, 0 replies; 11+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-04-28 12:52 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
As is done for all the other maintenance tasks, let's release the ODB
also before starting the geometric repacking. That way, the `.idx` files
won't be `mmap()`ed when they are to be deleted (which does not work on
Windows because you cannot delete files on that platform as long as they
are kept open by a process).
This regression was introduced by 9bc151850c1c (builtin/maintenance:
introduce "geometric-repack" task, 2025-10-24), but was only noticed
once geometric repacking was made the default in 452b12c2e0fe (builtin/
maintenance: use "geometric" strategy by default, 2026-02-24).
The fix recapitulates my work from df76ee7b77f0 (run-command: offer to
close the object store before running, 2021-09-09) & friends.
To guard against future regressions of this kind, add a check to
`run_and_verify_geometric_pack()` in `t7900` that detects orphaned
`.idx` files left behind after repacking. Contrary to interactive
calls, the `git maintenance` call in that test case would _not_ block on
Windows, asking whether to retry deleting that file, which is the reason
why this bug was not caught earlier.
Furthermore, since the default behavior of `DeleteFileW()` was changed
at some point between Windows 10 Build 17134.1304 and Build 18363.657
to use POSIX semantics (see https://stackoverflow.com/a/60512798),
the added orphaned-`.idx` check would be insufficient to catch this
regression on modern Windows without emulating legacy delete semantics
via `GIT_TEST_LEGACY_DELETE=1`.
This fixes https://github.com/git-for-windows/git/issues/6210.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/gc.c | 1 +
t/t7900-maintenance.sh | 22 +++++++++++++++++++++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 3a71e314c9..84a66d3240 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1590,6 +1590,7 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
pack_geometry_split(&geometry);
child.git_cmd = 1;
+ child.odb_to_close = the_repository->objects;
strvec_pushl(&child.args, "repack", "-d", "-l", NULL);
if (geometry.split < geometry.pack_nr)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 4700beacc1..f497f51b23 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -532,7 +532,16 @@ run_and_verify_geometric_pack () {
# And verify that there are no loose objects anymore.
git count-objects -v >count &&
- test_grep '^count: 0$' count
+ test_grep '^count: 0$' count &&
+
+ # Verify that no orphaned .idx files were left behind. On
+ # Windows, a missing odb_to_close causes the parent to hold
+ # mmap handles on .idx files, silently preventing their
+ # deletion by the child git-repack process.
+ ls .git/objects/pack/pack-*.idx .git/objects/pack/pack-*.pack |
+ sed "s/\.pack$/.idx/" |
+ sort | uniq -u >orphaned-idx &&
+ test_must_be_empty orphaned-idx
}
test_expect_success 'geometric repacking task' '
@@ -580,8 +589,19 @@ test_expect_success 'geometric repacking task' '
# And these two small packs should now be merged via the
# geometric repack. The large packfile should remain intact.
+ cp -R .git/objects .git/objects.save &&
run_and_verify_geometric_pack 2 &&
+ # On Windows, verify the same with legacy delete semantics
+ # that reject deletion of mmap-held .idx files.
+ if test_have_prereq MINGW
+ then
+ rm -rf .git/objects &&
+ mv .git/objects.save .git/objects &&
+ test_env GIT_TEST_LEGACY_DELETE=1 \
+ run_and_verify_geometric_pack 2
+ fi &&
+
# If we now add two more objects and repack twice we should
# then see another all-into-one repack. This time around
# though, as we have unreachable objects, we should also see a
--
gitgitgadget
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10
2026-04-28 12:52 [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10 Johannes Schindelin via GitGitGadget
2026-04-28 12:52 ` [PATCH 1/2] mingw: optionally use legacy (non-POSIX) delete semantics Johannes Schindelin via GitGitGadget
2026-04-28 12:52 ` [PATCH 2/2] maintenance(geometric): do release the `.idx` files before repacking Johannes Schindelin via GitGitGadget
@ 2026-04-28 15:01 ` Derrick Stolee
2026-05-04 14:12 ` Patrick Steinhardt
2026-05-07 12:51 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
3 siblings, 1 reply; 11+ messages in thread
From: Derrick Stolee @ 2026-04-28 15:01 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget, git; +Cc: Johannes Schindelin
On 4/28/2026 8:52 AM, Johannes Schindelin via GitGitGadget wrote:
> On Windows, maintenance_task_geometric_repack() opens pack index files via
> pack_geometry_init() (which mmap()s the .idx files), then spawns git repack
> as a child process without setting child.odb_to_close. The parent's mmap()s
> prevent the child from deleting old .idx files.
>
> On Windows 10 builds before the POSIX delete semantics change (between Build
> 17134.1304 and 18363.657, see https://stackoverflow.com/a/60512798), this
> results in Unlink of file '.git/objects/pack/pack-<hash>.idx' failed. Should
> I try again? during fetch-triggered auto-maintenance with the geometric
> strategy.
>
> The fix adds the missing child.odb_to_close = the_repository->objects line,
> matching all other maintenance tasks.
>
> The first commit introduces a GIT_TEST_LEGACY_DELETE environment variable to
> simulate legacy (pre-POSIX) delete semantics on modern Windows, so the
> regression test can verify the fix even on Windows 11.
>
> This fixes https://github.com/git-for-windows/git/issues/6210.
Thanks for these patches. I reviewed their equivalents in the
git-for-windows/git fork so I'll give my LGTM here, too.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] mingw: optionally use legacy (non-POSIX) delete semantics
2026-04-28 12:52 ` [PATCH 1/2] mingw: optionally use legacy (non-POSIX) delete semantics Johannes Schindelin via GitGitGadget
@ 2026-05-04 14:12 ` Patrick Steinhardt
2026-05-07 12:49 ` Johannes Schindelin
0 siblings, 1 reply; 11+ messages in thread
From: Patrick Steinhardt @ 2026-05-04 14:12 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
On Tue, Apr 28, 2026 at 12:52:47PM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 2023c16db6..04f9aa3922 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -449,20 +449,63 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)
> return wbuf;
> }
>
> +/*
> + * Use SetFileInformationByHandle(FileDispositionInfo) to force legacy
> + * (non-POSIX) delete semantics. On Windows 11, DeleteFileW() uses POSIX
> + * delete semantics internally, allowing deletion even with active
> + * MapViewOfFile views. This helper simulates Windows 10 behavior where
> + * deletion fails if a file mapping exists.
> + *
> + * Returns nonzero on success (like DeleteFileW), 0 on failure.
> + */
> +static int legacy_delete_file(const wchar_t *wpathname)
> +{
> + FILE_DISPOSITION_INFO fdi = { TRUE };
> + DWORD gle;
> + HANDLE h = CreateFileW(wpathname, DELETE,
> + FILE_SHARE_READ | FILE_SHARE_WRITE |
> + FILE_SHARE_DELETE,
> + NULL, OPEN_EXISTING,
> + FILE_FLAG_OPEN_REPARSE_POINT, NULL);
> + if (h == INVALID_HANDLE_VALUE)
> + return 0;
> +
> + if (SetFileInformationByHandle(h, FileDispositionInfo,
> + &fdi, sizeof(fdi))) {
> + CloseHandle(h);
> + return 1;
> + }
> + gle = GetLastError();
> + CloseHandle(h);
> + SetLastError(gle);
> + return 0;
> +}
> +
> +static int try_delete_file(const wchar_t *wpathname, int use_legacy)
> +{
> + if (use_legacy)
> + return legacy_delete_file(wpathname);
> + return DeleteFileW(wpathname);
> +}
> +
> int mingw_unlink(const char *pathname, int handle_in_use_error)
> {
> + static int use_legacy_delete = -1;
> int tries = 0;
> wchar_t wpathname[MAX_PATH];
> if (xutftowcs_path(wpathname, pathname) < 0)
> return -1;
>
> - if (DeleteFileW(wpathname))
> + if (use_legacy_delete < 0)
> + use_legacy_delete = !!getenv("GIT_TEST_LEGACY_DELETE");
Should this use `git_env_bool()`?
Patrick
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10
2026-04-28 15:01 ` [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10 Derrick Stolee
@ 2026-05-04 14:12 ` Patrick Steinhardt
0 siblings, 0 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-05-04 14:12 UTC (permalink / raw)
To: Derrick Stolee
Cc: Johannes Schindelin via GitGitGadget, git, Johannes Schindelin
On Tue, Apr 28, 2026 at 11:01:34AM -0400, Derrick Stolee wrote:
> On 4/28/2026 8:52 AM, Johannes Schindelin via GitGitGadget wrote:
> > On Windows, maintenance_task_geometric_repack() opens pack index files via
> > pack_geometry_init() (which mmap()s the .idx files), then spawns git repack
> > as a child process without setting child.odb_to_close. The parent's mmap()s
> > prevent the child from deleting old .idx files.
> >
> > On Windows 10 builds before the POSIX delete semantics change (between Build
> > 17134.1304 and 18363.657, see https://stackoverflow.com/a/60512798), this
> > results in Unlink of file '.git/objects/pack/pack-<hash>.idx' failed. Should
> > I try again? during fetch-triggered auto-maintenance with the geometric
> > strategy.
> >
> > The fix adds the missing child.odb_to_close = the_repository->objects line,
> > matching all other maintenance tasks.
> >
> > The first commit introduces a GIT_TEST_LEGACY_DELETE environment variable to
> > simulate legacy (pre-POSIX) delete semantics on modern Windows, so the
> > regression test can verify the fix even on Windows 11.
> >
> > This fixes https://github.com/git-for-windows/git/issues/6210.
>
> Thanks for these patches. I reviewed their equivalents in the
> git-for-windows/git fork so I'll give my LGTM here, too.
I've got a single comment on the first patch, but other than that this
series looks good to me. Thanks!
Patrick
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] mingw: optionally use legacy (non-POSIX) delete semantics
2026-05-04 14:12 ` Patrick Steinhardt
@ 2026-05-07 12:49 ` Johannes Schindelin
0 siblings, 0 replies; 11+ messages in thread
From: Johannes Schindelin @ 2026-05-07 12:49 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Johannes Schindelin via GitGitGadget, git
Hi Patrick,
On Thu, 7 May 2026, Patrick Steinhardt wrote:
> On Tue, Apr 28, 2026 at 12:52:47PM +0000, Johannes Schindelin via GitGitGadget wrote:
> >
> > - if (DeleteFileW(wpathname))
> > + if (use_legacy_delete < 0)
> > + use_legacy_delete = !!getenv("GIT_TEST_LEGACY_DELETE");
>
> Should this use `git_env_bool()`?
Yes! Will fix in v2.
Ciao,
Johannes
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 0/2] maintenance(geometric): avoid deadlocks on Windows 10
2026-04-28 12:52 [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10 Johannes Schindelin via GitGitGadget
` (2 preceding siblings ...)
2026-04-28 15:01 ` [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10 Derrick Stolee
@ 2026-05-07 12:51 ` Johannes Schindelin via GitGitGadget
2026-05-07 12:51 ` [PATCH v2 1/2] mingw: optionally use legacy (non-POSIX) delete semantics Johannes Schindelin via GitGitGadget
` (2 more replies)
3 siblings, 3 replies; 11+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-07 12:51 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Patrick Steinhardt, Johannes Schindelin
On Windows, maintenance_task_geometric_repack() opens pack index files via
pack_geometry_init() (which mmap()s the .idx files), then spawns git repack
as a child process without setting child.odb_to_close. The parent's mmap()s
prevent the child from deleting old .idx files.
On Windows 10 builds before the POSIX delete semantics change (between Build
17134.1304 and 18363.657, see https://stackoverflow.com/a/60512798), this
results in Unlink of file '.git/objects/pack/pack-<hash>.idx' failed. Should
I try again? during fetch-triggered auto-maintenance with the geometric
strategy.
The fix adds the missing child.odb_to_close = the_repository->objects line,
matching all other maintenance tasks.
The first commit introduces a GIT_TEST_LEGACY_DELETE environment variable to
simulate legacy (pre-POSIX) delete semantics on modern Windows, so the
regression test can verify the fix even on Windows 11.
This fixes https://github.com/git-for-windows/git/issues/6210.
Changes since v1:
* The code now uses git_env_bool() as appropriate (thanks Patrick!)
Johannes Schindelin (2):
mingw: optionally use legacy (non-POSIX) delete semantics
maintenance(geometric): do release the `.idx` files before repacking
builtin/gc.c | 1 +
compat/mingw.c | 47 ++++++++++++++++++++++++++++++++++++++++--
t/t7900-maintenance.sh | 22 +++++++++++++++++++-
3 files changed, 67 insertions(+), 3 deletions(-)
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2103%2Fdscho%2Favoid-deadlocks-in-geometric-repacking-on-windows-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2103/dscho/avoid-deadlocks-in-geometric-repacking-on-windows-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2103
Range-diff vs v1:
1: 97508e91b6 ! 1: 8ee749dd2f mingw: optionally use legacy (non-POSIX) delete semantics
@@ compat/mingw.c: static wchar_t *normalize_ntpath(wchar_t *wbuf)
- if (DeleteFileW(wpathname))
+ if (use_legacy_delete < 0)
-+ use_legacy_delete = !!getenv("GIT_TEST_LEGACY_DELETE");
++ use_legacy_delete = git_env_bool("GIT_TEST_LEGACY_DELETE", 0);
+
+ if (try_delete_file(wpathname, use_legacy_delete))
return 0;
2: 12ebd5c56f = 2: 66219b79fa maintenance(geometric): do release the `.idx` files before repacking
--
gitgitgadget
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] mingw: optionally use legacy (non-POSIX) delete semantics
2026-05-07 12:51 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
@ 2026-05-07 12:51 ` Johannes Schindelin via GitGitGadget
2026-05-07 12:51 ` [PATCH v2 2/2] maintenance(geometric): do release the `.idx` files before repacking Johannes Schindelin via GitGitGadget
2026-05-08 13:20 ` [PATCH v2 0/2] maintenance(geometric): avoid deadlocks on Windows 10 Patrick Steinhardt
2 siblings, 0 replies; 11+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-07 12:51 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Patrick Steinhardt, Johannes Schindelin,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
At some point between Windows 10 Build 17134.1304 and Build 18363.657,
the default behavior of `DeleteFileW()` was changed to use POSIX
semantics (https://stackoverflow.com/a/60512798). Under those semantics,
a file can be deleted even when another process holds an active
`MapViewOfFile` view on it: the directory entry is removed immediately,
but the underlying data persists until the last handle is closed.
On older Windows versions (and Windows 10 builds before that change),
`DeleteFileW()` uses legacy semantics where deletion fails outright if
any process holds a file mapping.
To allow testing code paths that depend on the legacy behavior, introduce
a `GIT_TEST_LEGACY_DELETE` environment variable. When set, `mingw_unlink()`
uses `SetFileInformationByHandle()` with `FileDispositionInfo` (the
non-POSIX variant) instead of `DeleteFileW()`, forcing legacy delete
semantics regardless of the Windows version.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/mingw.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 2023c16db6..aa7525f419 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -449,20 +449,63 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)
return wbuf;
}
+/*
+ * Use SetFileInformationByHandle(FileDispositionInfo) to force legacy
+ * (non-POSIX) delete semantics. On Windows 11, DeleteFileW() uses POSIX
+ * delete semantics internally, allowing deletion even with active
+ * MapViewOfFile views. This helper simulates Windows 10 behavior where
+ * deletion fails if a file mapping exists.
+ *
+ * Returns nonzero on success (like DeleteFileW), 0 on failure.
+ */
+static int legacy_delete_file(const wchar_t *wpathname)
+{
+ FILE_DISPOSITION_INFO fdi = { TRUE };
+ DWORD gle;
+ HANDLE h = CreateFileW(wpathname, DELETE,
+ FILE_SHARE_READ | FILE_SHARE_WRITE |
+ FILE_SHARE_DELETE,
+ NULL, OPEN_EXISTING,
+ FILE_FLAG_OPEN_REPARSE_POINT, NULL);
+ if (h == INVALID_HANDLE_VALUE)
+ return 0;
+
+ if (SetFileInformationByHandle(h, FileDispositionInfo,
+ &fdi, sizeof(fdi))) {
+ CloseHandle(h);
+ return 1;
+ }
+ gle = GetLastError();
+ CloseHandle(h);
+ SetLastError(gle);
+ return 0;
+}
+
+static int try_delete_file(const wchar_t *wpathname, int use_legacy)
+{
+ if (use_legacy)
+ return legacy_delete_file(wpathname);
+ return DeleteFileW(wpathname);
+}
+
int mingw_unlink(const char *pathname, int handle_in_use_error)
{
+ static int use_legacy_delete = -1;
int tries = 0;
wchar_t wpathname[MAX_PATH];
if (xutftowcs_path(wpathname, pathname) < 0)
return -1;
- if (DeleteFileW(wpathname))
+ if (use_legacy_delete < 0)
+ use_legacy_delete = git_env_bool("GIT_TEST_LEGACY_DELETE", 0);
+
+ if (try_delete_file(wpathname, use_legacy_delete))
return 0;
do {
/* read-only files cannot be removed */
_wchmod(wpathname, 0666);
- if (!_wunlink(wpathname))
+ if (try_delete_file(wpathname, use_legacy_delete))
return 0;
if (!is_file_in_use_error(GetLastError()))
break;
--
gitgitgadget
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] maintenance(geometric): do release the `.idx` files before repacking
2026-05-07 12:51 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2026-05-07 12:51 ` [PATCH v2 1/2] mingw: optionally use legacy (non-POSIX) delete semantics Johannes Schindelin via GitGitGadget
@ 2026-05-07 12:51 ` Johannes Schindelin via GitGitGadget
2026-05-08 13:20 ` [PATCH v2 0/2] maintenance(geometric): avoid deadlocks on Windows 10 Patrick Steinhardt
2 siblings, 0 replies; 11+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-07 12:51 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Patrick Steinhardt, Johannes Schindelin,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
As is done for all the other maintenance tasks, let's release the ODB
also before starting the geometric repacking. That way, the `.idx` files
won't be `mmap()`ed when they are to be deleted (which does not work on
Windows because you cannot delete files on that platform as long as they
are kept open by a process).
This regression was introduced by 9bc151850c1c (builtin/maintenance:
introduce "geometric-repack" task, 2025-10-24), but was only noticed
once geometric repacking was made the default in 452b12c2e0fe (builtin/
maintenance: use "geometric" strategy by default, 2026-02-24).
The fix recapitulates my work from df76ee7b77f0 (run-command: offer to
close the object store before running, 2021-09-09) & friends.
To guard against future regressions of this kind, add a check to
`run_and_verify_geometric_pack()` in `t7900` that detects orphaned
`.idx` files left behind after repacking. Contrary to interactive
calls, the `git maintenance` call in that test case would _not_ block on
Windows, asking whether to retry deleting that file, which is the reason
why this bug was not caught earlier.
Furthermore, since the default behavior of `DeleteFileW()` was changed
at some point between Windows 10 Build 17134.1304 and Build 18363.657
to use POSIX semantics (see https://stackoverflow.com/a/60512798),
the added orphaned-`.idx` check would be insufficient to catch this
regression on modern Windows without emulating legacy delete semantics
via `GIT_TEST_LEGACY_DELETE=1`.
This fixes https://github.com/git-for-windows/git/issues/6210.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/gc.c | 1 +
t/t7900-maintenance.sh | 22 +++++++++++++++++++++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 3a71e314c9..84a66d3240 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1590,6 +1590,7 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
pack_geometry_split(&geometry);
child.git_cmd = 1;
+ child.odb_to_close = the_repository->objects;
strvec_pushl(&child.args, "repack", "-d", "-l", NULL);
if (geometry.split < geometry.pack_nr)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 4700beacc1..f497f51b23 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -532,7 +532,16 @@ run_and_verify_geometric_pack () {
# And verify that there are no loose objects anymore.
git count-objects -v >count &&
- test_grep '^count: 0$' count
+ test_grep '^count: 0$' count &&
+
+ # Verify that no orphaned .idx files were left behind. On
+ # Windows, a missing odb_to_close causes the parent to hold
+ # mmap handles on .idx files, silently preventing their
+ # deletion by the child git-repack process.
+ ls .git/objects/pack/pack-*.idx .git/objects/pack/pack-*.pack |
+ sed "s/\.pack$/.idx/" |
+ sort | uniq -u >orphaned-idx &&
+ test_must_be_empty orphaned-idx
}
test_expect_success 'geometric repacking task' '
@@ -580,8 +589,19 @@ test_expect_success 'geometric repacking task' '
# And these two small packs should now be merged via the
# geometric repack. The large packfile should remain intact.
+ cp -R .git/objects .git/objects.save &&
run_and_verify_geometric_pack 2 &&
+ # On Windows, verify the same with legacy delete semantics
+ # that reject deletion of mmap-held .idx files.
+ if test_have_prereq MINGW
+ then
+ rm -rf .git/objects &&
+ mv .git/objects.save .git/objects &&
+ test_env GIT_TEST_LEGACY_DELETE=1 \
+ run_and_verify_geometric_pack 2
+ fi &&
+
# If we now add two more objects and repack twice we should
# then see another all-into-one repack. This time around
# though, as we have unreachable objects, we should also see a
--
gitgitgadget
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/2] maintenance(geometric): avoid deadlocks on Windows 10
2026-05-07 12:51 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2026-05-07 12:51 ` [PATCH v2 1/2] mingw: optionally use legacy (non-POSIX) delete semantics Johannes Schindelin via GitGitGadget
2026-05-07 12:51 ` [PATCH v2 2/2] maintenance(geometric): do release the `.idx` files before repacking Johannes Schindelin via GitGitGadget
@ 2026-05-08 13:20 ` Patrick Steinhardt
2 siblings, 0 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-05-08 13:20 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Derrick Stolee, Johannes Schindelin
On Thu, May 07, 2026 at 12:51:11PM +0000, Johannes Schindelin via GitGitGadget wrote:
> On Windows, maintenance_task_geometric_repack() opens pack index files via
> pack_geometry_init() (which mmap()s the .idx files), then spawns git repack
> as a child process without setting child.odb_to_close. The parent's mmap()s
> prevent the child from deleting old .idx files.
>
> On Windows 10 builds before the POSIX delete semantics change (between Build
> 17134.1304 and 18363.657, see https://stackoverflow.com/a/60512798), this
> results in Unlink of file '.git/objects/pack/pack-<hash>.idx' failed. Should
> I try again? during fetch-triggered auto-maintenance with the geometric
> strategy.
>
> The fix adds the missing child.odb_to_close = the_repository->objects line,
> matching all other maintenance tasks.
>
> The first commit introduces a GIT_TEST_LEGACY_DELETE environment variable to
> simulate legacy (pre-POSIX) delete semantics on modern Windows, so the
> regression test can verify the fix even on Windows 11.
>
> This fixes https://github.com/git-for-windows/git/issues/6210.
>
> Changes since v1:
>
> * The code now uses git_env_bool() as appropriate (thanks Patrick!)
This version looks good to me, thanks!
Patrick
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-05-08 13:20 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-28 12:52 [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10 Johannes Schindelin via GitGitGadget
2026-04-28 12:52 ` [PATCH 1/2] mingw: optionally use legacy (non-POSIX) delete semantics Johannes Schindelin via GitGitGadget
2026-05-04 14:12 ` Patrick Steinhardt
2026-05-07 12:49 ` Johannes Schindelin
2026-04-28 12:52 ` [PATCH 2/2] maintenance(geometric): do release the `.idx` files before repacking Johannes Schindelin via GitGitGadget
2026-04-28 15:01 ` [PATCH 0/2] maintenance(geometric): avoid deadlocks on Windows 10 Derrick Stolee
2026-05-04 14:12 ` Patrick Steinhardt
2026-05-07 12:51 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2026-05-07 12:51 ` [PATCH v2 1/2] mingw: optionally use legacy (non-POSIX) delete semantics Johannes Schindelin via GitGitGadget
2026-05-07 12:51 ` [PATCH v2 2/2] maintenance(geometric): do release the `.idx` files before repacking Johannes Schindelin via GitGitGadget
2026-05-08 13:20 ` [PATCH v2 0/2] maintenance(geometric): avoid deadlocks on Windows 10 Patrick Steinhardt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.