Git development
 help / color / mirror / Atom feed
* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox