Thread (5 messages) 5 messages, 2 authors, 2025-05-30

Re: [PATCH 11/11] builtin/maintenance: fix locking race when handling "gc" task

From: Patrick Steinhardt <hidden>
Date: 2025-05-30 14:05:26

On Fri, May 30, 2025 at 08:55:49AM -0400, Ben Knoble wrote:
quoted
@@ -1240,6 +1251,7 @@ static int maintenance_task_gc(struct maintenance_run_opts *opts,
  else
      strvec_push(&child.args, "--no-quiet");
  strvec_push(&child.args, "--no-detach");
+    strvec_push(&child.args, "--skip-maintenance-before-detach");
I suspect this would be more obvious to me if I had the manual
available right now, but if we are not detaching (« --no-detach ») why
do we need to skip something before detaching (that presumably won’t
happen)?
We have two levels here: git-maintenance(1) and git-gc(1), where the
former executes the latter when the "gc" task is configured. What is
important to realize is that in this setup it is not git-gc(1) which
detaches -- it is git-maintenance(1). So git-maintenance(1) runs in the
background, but any tasks it invokes itself must run synchronously in
the foreground.

The flow thus looks like this:

  1. git-maintenance(1) starts.
  2. We perform the pre-detach tasks from git-gc(1) in the same process.
  3. We detach and thus the main process exits.
  4. We execute git-gc(1) in the already-detached process.
  5. We wait for git-gc(1) to exit.
  6. The detached git-maintenance(1) exits.

So because (4) is running in the already-detached process we ask
git-gc(1) to not detach again. And because we already ran the pre-detach
tasks we also ask it to not run those again.

Patrick
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help