Subject: bug#30948: [PATCH core-updates] guix: Reap finished child processes in build containers.

From: Maxim Cournoyer <[email-masked]>
Date: Thu, 24 Nov 2022 11:44:27 -0500

Hi,

[email-masked] (Ludovic Courtès) writes:

> Hello,
>
> Carlo Zancanaro <[email-masked]> skribis:
>
>> On Fri, Mar 30 2018, Ludovic Courtès wrote:
>>>> From what I can understand it's one of pid 1's responsiblities to
>>>> reap child processes, so I would expect this to be set up for every
>>>> builder, before the builder is run.
>>>
>>> True, but for derivations it’s also “optional” because eventually
>>> guix-daemon terminates all its child processes.
>>
>> As long as the build process doesn't rely on behaviour that, strictly
>> speaking, it should be allowed to rely on. It's not an issue of
>> resource leaking, it's an issue of correctness.
>
> Right.
>
>>>> Given it's not specific to the gnu-build-system, I don't think it
>>>> really fits there.

For what it's worth, I agree.  The evaluation container should have the
correct signal handling configured for *any* code about to be evaluated,
not just when on demand, if we want to fix this fully in a way that
won't come back to haunt us in some edge case.

>>> Yes, but note that it would be inherited by all the build systems.
>>
>> Except for trivial-build-system, which is probably fine. I still don't
>> think it fits in a specific build system, given it's a behaviour that
>> transcends the specific action happening within the container.
>>
>> Putting it in gnu-build-system will solve the problem in all realistic
>> cases, so that's probably fine. It's still subtly incorrect, but will
>> only be a problem if something using the trivial build system relies
>> on pid 1 to reap a process, or if we make a new build system not
>> deriving from gnu-build-system (which seems unlikely, but not
>> impossible).
>
> I agree, every Guile process running as PID 1 should reap processes.

Agreed too.

> My view is just that this mechanism belongs in “user code”, not in the
> low-level mechanisms such as ‘build-expression->derivation’ and
> ‘gexp->derivation’.  It’s a matter of separation of concerns.

Why?  On my Guix System, such signal handling is handled by Shepherd, if
I'm not mistaken.  As I user, I can trust the foundation to be sane,
rather than having to provide the bits to make it so myself.

> Of course we don’t want to duplicate that code every time, but the way
> we should factorize it, IMO, is by putting it in a “normal” module that
> people will use.
>
> Putting it in gnu-build-system is an admittedly hacky but easy way to
> have it widely shared.

I think we can do better than hacky here :-)

-- 
Thanks,
Maxim