# [PATCH 0/1] pack-write, pack-bitmap-write: register tmp pack files for cleanup 3 messages from 2026-09-25 to 2026-09-28. Participants: Royce Remer, Patrick Steinhardt. Thread: https://gitlist.dev/t/66394 ## Royce Remer, 2026-09-25 20:56 Subject: [PATCH 0/1] pack-write, pack-bitmap-write: register tmp pack files for cleanup Message-ID: <20260925205633.530651-1-royceremer@gmail.com> ``` 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--pack-.* 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 ``` ## Royce Remer, 2026-09-25 20:56 Subject: [PATCH 1/1] pack-write, pack-bitmap-write: register tmp pack files for cleanup Message-ID: <20260925205633.530651-2-royceremer@gmail.com> In-Reply-To: <20260925205633.530651-1-royceremer@gmail.com> ``` `git repack` correctly uses `register_tempfile()` via `collect_pack_filenames()` for the `.tmp--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 --- 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 ``` ## Patrick Steinhardt, 2026-09-28 07:58 Subject: Re: [PATCH 1/1] pack-write, pack-bitmap-write: register tmp pack files for cleanup Message-ID: In-Reply-To: <20260925205633.530651-2-royceremer@gmail.com> ``` 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 ```