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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> Le 30 mai 2025 à 10:05, Patrick Steinhardt <ps@xxxxxx> a écrit :
> 
> On Fri, May 30, 2025 at 08:55:49AM -0400, Ben Knoble wrote:
>>> @@ -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

Aha, thanks! I thought I understood the sequence, but I was wrong about some details.

I was wondering if not detaching should just imply skipping work before a (non-existent) detach—if there’s no detach, should we do any pre-detach work at all? But presumably that does the wrong thing for (non-detaching) invocations that come from outside git-maintenance, doesn’t it? Hm.

Maybe the flip-around for me is that « pre-detach work » here actually refers to « foreground work », which we obviously want to do even if we aren’t detaching, and which maintenance (which has already done this) needs to skip.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux