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

[Taylor Blau <me@ttaylorr.com>]

On Sun, May 10, 2026 at 12:08:14PM -0400, Derrick Stolee wrote:
> I worry that we're assigning _all_ tempfiles to the child in this
> case, not just specific ones like the maintenance lock. But since
> this is specifically called only in daemonize() then it may be
> fine.

Right, I had wondered about this above, and I'm not sure what the right
behavior is here. I think in our case its fine to pass all tempfiles
down to the child, since we're about to exit.

The only other spot that uses `daemonize()` is 'gc --detach' and 'git
daemon'. I think before considering something like this we would want to
reason through what the intended behavior of each of those callers is.
If we end up pursuing a fix in this direction, I think the safest thing
that we could do would be to have callers opt-in to this behavior so we
can do the right thing in git-maintenance, and other callers retain
their existing behavior.

> Is there any concern that since the child didn't create a tempfile
> that the atexit() may not be initialized in the child process? Or
> will that carry over from the fork() automatically? (This is hard
> to test without something causing a die() in the detached process.)

The child will have its own copy of the tempfile list as well as its own
copy of the atexit() handlers. POSIX (via atexit(3p)) says[1] that
atexit() behaves according to the ISO C standard:

    The functionality described on this reference page is aligned with
    the ISO C standard. Any conflict between the requirements described
    here and the ISO C standard is unintentional. This volume of
    POSIX.1‐2017 defers to the ISO C standard.

, and atexit(3) says[2]:

    When a child process is created via fork(2), it inherits copies of
    its parent's registrations.  Upon a successful call to one of the
    exec(3) functions, all registrations are removed.

So the child has an identical copy of both the atexit() handlers and the
tempfile list.

> > 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.
>
> I'd rather revert the geometric maintenance default for a point
> release and let something more complicated like this percolate
> until the next release window.
>
> > 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.
>
> It does limit the behavior to those who previously chose to opt-in to
> this behavior, and likely that's a low number or else we'd have more
> bug reports.

I agree with what you're saying. In an ideal world we would be able to
apply a fix on top that would prevent this bug from occurring, but if
that's not trivial to do then we shouldn't rush it in the 2.54.1 window.

I think that not having this bug whatsoever (as opposed to simply
reverting 452b12c2e0) would be preferable, but as you note we haven't
seen any bug reports in a pre-452b12c2e0 world, so perhaps the risk is
low enough that we could merely revert 452b12c2e0.

> For me to be convinced that this forward fix is the right direction,
> I'd need to see a test that proves the detached process will clean up
> the locks on a normal process end and an early exit.

Yeah, I agree. Testing this is a little tricky, but I played around
with it for a while and came up with the following:

--- 8< ---
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 4700beacc18..77bfa537385 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -1460,6 +1460,56 @@ test_expect_success '--detach causes maintenance to run in background' '
 	)
 '

+test_expect_success PIPE '--detach holds maintenance lock until daemonized child exits' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+
+		mkfifo fifo &&
+
+		git config maintenance.auto false &&
+		git config core.lockfilepid true &&
+
+		git remote add origin /does/not/exist &&
+		git config set remote.origin.uploadpack \
+			"echo \$PPID >child && cat \"$(pwd)/fifo\"" &&
+
+		# Start a detached prefetch maintenance task. Note that
+		# we are backgrounding git-maintenance here in order to
+		# determine its PID to validate that the lockfile was
+		# created by the parent.
+		{ git maintenance run --task=prefetch --detach & } &&
+		parent="$!" &&
+
+		# Open fifo for writing, which will block until the
+		# upload-pack helper opens it for reading. Once exec
+		# returns, we know that the daemonized child is alive
+		# and pinned.
+		exec 8>fifo &&
+
+		test_path_is_file .git/objects/maintenance.lock &&
+		test_path_is_file .git/objects/maintenance~pid.lock &&
+
+		# Verify that the maintenance.lock still exists, and
+		# that it was created by the parent process, not the
+		# child.
+		echo "pid $parent" >expect &&
+		test_cmp expect .git/objects/maintenance~pid.lock &&
+
+		# Close the write end of the FIFO, causing our upload-pack
+		# helper to quit. Wait until the grandparent (from the
+		# perspective of our upload-pack helper, the daemonized
+		# git-maintenance child)
+		exec 8>&- &&
+		gpid="$(ps -o ppid= -p $(cat child) | tr -d " ")" &&
+		test -n $gpid && wait $gpid &&
+
+		test_path_is_missing .git/objects/maintenance.lock &&
+		test_path_is_missing .git/objects/maintenance~pid.lock
+	)
+'
+
 test_expect_success 'repacking loose objects is quiet' '
 	test_when_finished "rm -rf repo" &&
 	git init repo &&
--- >8 ---

I'm not sure how portable it is, though. I think that 'ps -o ppid=' is
OK, since 'start_git_in_background()' uses it, but I'm not sure if $PPID
is available on Windows.

It reliably passes with the fix that I shared earlier in the thread, and
reliably fails without it.

Thanks,
Taylor

[1]: https://man7.org/linux/man-pages/man3/atexit.3p.html
[2]: https://man7.org/linux/man-pages/man3/atexit.3.html