Re: unexpected auto-maintenance, was Re: git hogs the CPU, RAM and storage despite its config

From: Taylor Blau <me@ttaylorr.com>
Date: May 9, 2026, 21:52 UTC
Message-ID: <af+snTGFeoUUyfPU@nand.local>
In-Reply-To: <20260509175249.GA2336928@coredump.intra.peff.net>

On Sat, May 09, 2026 at 01:52:49PM -0400, Jeff King wrote:

Show 6 quoted lines

> I don't think this affected the old "git gc --detach" because it takes
> the lock after daemonizing[1]. We can't do the same here, though, since we
> need to hold the lock for the foreground tasks. So either we need to
> release and re-take the lock between foreground and background tasks, or
> we need to teach the daemonize() function to update the "owner" field on
> all of the tempfiles to the new child[2].

I agree. We can't simply do what was done in 329e6e8794c (gc: save log from daemonized gc --auto and print it next time, 2015-09-19), for exactly the reason that you stated.

Dropping and re-acquiring the lock is possible, but it's racy since there is a gap during the critical window while we fork(). I believe that the only airtight way to do this is to update the owner field of the tempfiles we want to pass down during daemonization.

(As an aside, do we want to do that for all tempfiles? It might be nice to have a "->reassign_on_fork" flag or something on the tempfile struct in case there are instances where the parent wants to retain ownership of the tempfile after fork()-ing, but I can't think of any off the top of my head. If we do introduce such a field, it should probably default to "true" to avoid any foot-guns.)

> Ultimately fixing the lock bug will solve that. Though if doing so is
> too complicated for a quick maint release, I'm tempted to say we should
> consider reverting 452b12c2e0 for a potential v2.54.1 (as there were a
> few other regression fixes so far, I assume we'll have one soon-ish).

I think something like the following (untested) would do the trick:

--- 8< ---

diff --git a/setup.c b/setup.c
index 7ec4427368a..c07aeac4f7d 100644
--- a/setup.c
+++ b/setup.c
@@ -22,6 +22,7 @@
 #include "chdir-notify.h"
 #include "path.h"
 #include "quote.h"
+#include "tempfile.h"
 #include "trace.h"
 #include "trace2.h"
 #include "worktree.h"
@@ -2162,12 +2163,17 @@ int daemonize(void)
 	errno = ENOSYS;
 	return -1;
 #else
-	switch (fork()) {
+	pid_t ppid = getpid();
+	pid_t pid;
+
+	switch ((pid = fork())) {
 		case 0:
+			reassign_tempfile_ownership(ppid, getpid());
 			break;
 		case -1:
 			die_errno(_("fork failed"));
 		default:
+			reassign_tempfile_ownership(ppid, pid);
 			exit(0);
 	}
 	if (setsid() == -1)
diff --git a/tempfile.c b/tempfile.c
index 82dfa3d82f2..f0fdf582794 100644
--- a/tempfile.c
+++ b/tempfile.c
@@ -373,3 +373,15 @@ int delete_tempfile(struct tempfile **tempfile_p)

 	return err ? -1 : 0;
 }
+
+void reassign_tempfile_ownership(pid_t from, pid_t to)
+{
+	volatile struct volatile_list_head *pos;
+
+	list_for_each(pos, &tempfile_list) {
+		struct tempfile *p = list_entry(pos, struct tempfile, list);
+
+		if (is_tempfile_active(p) && p->owner == from)
+			p->owner = to;
+	}
+}
diff --git a/tempfile.h b/tempfile.h
index 2d2ae5b657d..783d7470b54 100644
--- a/tempfile.h
+++ b/tempfile.h
@@ -282,4 +282,16 @@ int delete_tempfile(struct tempfile **tempfile_p);
  */
 int rename_tempfile(struct tempfile **tempfile_p, const char *path);

+/*
+ * Reassign ownership of all active tempfiles whose `owner` field
+ * matches `from` to `to`.
+ *
+ * This is intended for use by `daemonize()`; after `fork(2)`-ing, the
+ * parent transfers ownership to the daemonized child so that its
+ * atexit handler does not unlink tempfiles that should outlive it,
+ * and the child claims the inherited tempfiles so that they are
+ * cleaned up when the daemon exits.
+ */
+void reassign_tempfile_ownership(pid_t from, pid_t to);
+
 #endif /* TEMPFILE_H */
--- >8 ---

That's not too terrible to write, and I would feel OK about putting it
in a 2.54.1 release soon-ish provided that others think it is reasonable.

Simply reverting 452b12c2e0 (builtin/maintenance: use "geometric"
strategy by default, 2026-02-24) feels somewhat unsatisfying, since it
is merely making the bug less likely rather than eliminating it
entirely.

So in that sense I would prefer to "fix forward" here rather than to
mask over the bug. But even the relatively short diff above is not so
straightforward to reason through, review, or test, so I'm open to other
ideas on how to proceed here.

Thanks,
Taylor