[PATCH 0/1] pack-write, pack-bitmap-write: register tmp pack files for cleanup

[PATCH 0/1] pack-write, pack-bitmap-write: register tmp pack files for cleanup

[Royce Remer]

This patch registers the temporary pack files created by git-gc(1) and
git-maintenance(1) with the tempfile subsystem so they are removed when
the process exits gracefully.

Background
----------

The motivation came from diagnosing disk space in Kubernetes pods
running Gitea as git mirrors.  If a pod was killed mid-gc, the
pack-writing code left orphaned temp files in objects/pack/.
On restart, git gc would start fresh and write new temp files
alongside the existing ones.  Over many restarts these,
accumulated until the underlying volume was exhausted:

  Two repositories examined on a single pod:
    objects/pack/tmp_pack_* -- 27 GiB, 21 GiB, 11 GiB, ... (17 files, ~60 GiB total)
    objects/pack/.tmp-*-pack-*.{pack,rev} -- ~118 GiB across 6 killed repacks

  A second pod had a different repository where two killed repacks left:
    objects/pack/.tmp-*-pack-*.{pack,rev} -- ~39 GiB across 2 killed repacks

  Deleting those files and running git-prune-packed(1) to remove loose
  objects already represented in pack files recovered ~224 GiB on that
  second pod alone.

Obviously, this is dependent on repository sizes and number of
failures and such, but I thought I'd share my extreme example.

Reviewing the gc and maintenance code, I don't see any attempts to
resume or reuse temp files left by a previous invocation; each run
calls odb_mkstemp() unconditionally to create a fresh file.  Any
surviving temp file should be safe to remove.

The tmp_idx, tmp_pack and tmp_bitmap sites predate the tempfile
subsystem (1a9d15db25, 2015-08-10) and so had no mechanism to
register when introduced.  The tmp_rev and tmp_mtimes sites
were added afterward but did not use it either.

Note that git-repack(1) already handles this correctly: it calls
register_tempfile() for the .tmp-<pid>-pack-<sha>.* files it creates
via collect_pack_filenames(), so those are cleaned up on graceful exit.
The lower-level paths invoked by git-gc(1) and git-maintenance(1)
(pack-write.c and pack-bitmap-write.c) go through odb_mkstemp() which
wraps mkstemp(2) directly without registering with the tempfile
subsystem, and so do not benefit from this cleanup.

I have some unit tests covering this, but they required instrumenting
the code to add a wait driven by an environment variable so I could
catch/kill a repack on a tiny mock repo. I decided not to commit
those as I think the fix is self-evident and we're just delegating
to the same tempfile cleanup logic and relying on that coverage.

Royce Remer (1):
  pack-write, pack-bitmap-write: register tmp pack files for cleanup

 pack-bitmap-write.c | 3 +++
 pack-write.c        | 5 +++++
 2 files changed, 8 insertions(+)

-- 
2.55.0.1.ga30d533ec0

[PATCH 1/1] pack-write, pack-bitmap-write: register tmp pack files for cleanup

[Royce Remer]

`git repack` correctly uses `register_tempfile()` via
`collect_pack_filenames()` for the `.tmp-<pid>-pack-*` files it
creates, so they are removed when the process exits gracefully.

The lower-level pack-writing functions invoked by `git gc` and
`git maintenance` do not. They call `odb_mkstemp()` which wraps
`mkstemp(2)` directly, bypassing the tempfile subsystem entirely.
A SIGTERM leaves these files stranded on disk where they accumulate
and can exhaust available space:

  objects/pack/tmp_pack_XXXXXX   (create_tmp_packfile)
  objects/pack/tmp_idx_XXXXXX    (write_idx_file)
  objects/pack/tmp_rev_XXXXXX    (write_rev_file_order)
  objects/pack/tmp_mtimes_XXXXXX (write_mtimes_file)
  objects/pack/tmp_bitmap_XXXXXX (bitmap_writer_finish)

Call `register_tempfile()` immediately after each `odb_mkstemp()` so
that the atexit(3) and signal handlers unlink the file on abnormal
exit.

Signed-off-by: Royce Remer <royceremer@gmail.com>
---
 pack-bitmap-write.c | 3 +++
 pack-write.c        | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 1bcb3f98a4..c566419690 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -23,6 +23,7 @@
 #include "oid-array.h"
 #include "config.h"
 #include "alloc.h"
+#include "tempfile.h"
 #include "refs.h"
 #include "strmap.h"
 #include "midx.h"
@@ -1378,6 +1379,7 @@ void bitmap_writer_finish(struct bitmap_writer *writer,
 
 	int fd = odb_mkstemp(writer->repo->objects, &tmp_file,
 			     "pack/tmp_bitmap_XXXXXX");
+	struct tempfile *tmp = register_tempfile(tmp_file.buf);
 
 	if (writer->pseudo_merges_nr)
 		options |= BITMAP_OPT_PSEUDO_MERGES;
@@ -1435,6 +1437,7 @@ void bitmap_writer_finish(struct bitmap_writer *writer,
 
 	if (rename(tmp_file.buf, filename))
 		die_errno("unable to rename temporary bitmap file to '%s'", filename);
+	delete_tempfile(&tmp);
 
 	strbuf_release(&tmp_file);
 	free(offsets);
diff --git a/pack-write.c b/pack-write.c
index 83eaf88541..fa6b532230 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -13,6 +13,7 @@
 #include "path.h"
 #include "repository.h"
 #include "strbuf.h"
+#include "tempfile.h"
 
 void reset_pack_idx_option(struct pack_idx_option *opts)
 {
@@ -87,6 +88,7 @@ const char *write_idx_file(struct repository *repo,
 			fd = odb_mkstemp(repo->objects, &tmp_file,
 					 "pack/tmp_idx_XXXXXX");
 			index_name = strbuf_detach(&tmp_file, NULL);
+			(void)register_tempfile(index_name);
 		} else {
 			unlink(index_name);
 			fd = xopen(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
@@ -263,6 +265,7 @@ char *write_rev_file_order(struct repository *repo,
 			fd = odb_mkstemp(repo->objects, &tmp_file,
 					 "pack/tmp_rev_XXXXXX");
 			path = strbuf_detach(&tmp_file, NULL);
+			(void)register_tempfile(path);
 		} else {
 			unlink(rev_name);
 			fd = xopen(rev_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
@@ -346,6 +349,7 @@ static char *write_mtimes_file(struct repository *repo,
 
 	fd = odb_mkstemp(repo->objects, &tmp_file, "pack/tmp_mtimes_XXXXXX");
 	mtimes_name = strbuf_detach(&tmp_file, NULL);
+	(void)register_tempfile(mtimes_name);
 	f = hashfd(repo->hash_algo, fd, mtimes_name);
 
 	write_mtimes_header(repo->hash_algo, f);
@@ -535,6 +539,7 @@ struct hashfile *create_tmp_packfile(struct repository *repo,
 
 	fd = odb_mkstemp(repo->objects, &tmpname, "pack/tmp_pack_XXXXXX");
 	*pack_tmp_name = strbuf_detach(&tmpname, NULL);
+	(void)register_tempfile(*pack_tmp_name);
 	return hashfd(repo->hash_algo, fd, *pack_tmp_name);
 }
 
-- 
2.55.0.1.ga30d533ec0

Re: [PATCH 1/1] pack-write, pack-bitmap-write: register tmp pack files for cleanup

[Patrick Steinhardt]

On Fri, Sep 25, 2026 at 01:56:33PM -0700, Royce Remer wrote:
> diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
> index 1bcb3f98a4..c566419690 100644
> --- a/pack-bitmap-write.c
> +++ b/pack-bitmap-write.c
> @@ -1378,6 +1379,7 @@ void bitmap_writer_finish(struct bitmap_writer *writer,
>  
>  	int fd = odb_mkstemp(writer->repo->objects, &tmp_file,
>  			     "pack/tmp_bitmap_XXXXXX");
> +	struct tempfile *tmp = register_tempfile(tmp_file.buf);
>  
>  	if (writer->pseudo_merges_nr)
>  		options |= BITMAP_OPT_PSEUDO_MERGES;
> @@ -1435,6 +1437,7 @@ void bitmap_writer_finish(struct bitmap_writer *writer,
>  
>  	if (rename(tmp_file.buf, filename))
>  		die_errno("unable to rename temporary bitmap file to '%s'", filename);
> +	delete_tempfile(&tmp);
>  
>  	strbuf_release(&tmp_file);
>  	free(offsets);

The fact that we add calls to `register_tempfile()` to almost every
single callsites that uses `odb_mkstemp()` makes me wonder whether the
interface itself is maybe misdesigned. Like, should it maybe return a
tempfile instead of returning a file descriptor so that callers don't
have to manually register it?

I also wonder whether `odb_mkstemp()` even sits at the right level to
begin with. It's ultimately specific to the "files" backend, as it
assumes that files live in "objects/". Would it be preferable if we
instead made it part of the "tempfile.h" API, where the only difference
to other functions is that it knows to also support leading directories?

We could for example have a new "_d" suffix for `mks_tempfile()`
functions.

Thanks!

Patrick