all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
To: ludo@gnu.org (Ludovic Courtès)
Cc: 30948@debbugs.gnu.org, Carlo Zancanaro <carlo@zancanaro.id.au>
Subject: bug#30948: [PATCH core-updates] guix: Reap finished child processes in build containers.
Date: Thu, 24 Nov 2022 11:44:27 -0500	[thread overview]
Message-ID: <87cz9cnvys.fsf_-_@gmail.com> (raw)
In-Reply-To: <874lkxoanq.fsf@gnu.org> ("Ludovic Courtès"'s message of "Fri, 30 Mar 2018 17:17:29 +0200")

Hi,

ludo@gnu.org (Ludovic Courtès) writes:

> Hello,
>
> Carlo Zancanaro <carlo@zancanaro.id.au> 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




  parent reply	other threads:[~2022-11-24 16:45 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-26 11:16 [bug#30948] [PATCH core-updates] guix: Reap finished child processes in build containers Carlo Zancanaro
2018-03-26 23:39 ` Carlo Zancanaro
2018-03-29 20:07 ` Ludovic Courtès
2018-03-29 21:15   ` Carlo Zancanaro
2018-03-30  8:16     ` Ludovic Courtès
2018-03-30 11:17       ` Carlo Zancanaro
2018-03-30 15:17         ` Ludovic Courtès
2022-11-24 16:40           ` Maxim Cournoyer
2022-11-24 16:44           ` Maxim Cournoyer [this message]
2022-11-26 15:11             ` bug#30948: " Ludovic Courtès
2022-11-27  3:00               ` Maxim Cournoyer
2022-11-28 15:04                 ` Ludovic Courtès
2022-11-28 20:10                   ` Maxim Cournoyer
2022-11-29  2:07                   ` Maxim Cournoyer
2023-12-17 20:23                   ` bug#30948: [PATCH core-updates] build-system/gnu: Turn PID 1 into an “init”-style process by default Ludovic Courtès
2023-12-17 21:46                     ` Maxim Cournoyer
2023-12-18 17:46                       ` bug#30948: [PATCH core-updates] guix: Reap finished child processes in build containers Ludovic Courtès
2023-12-30  3:36                         ` Maxim Cournoyer
2023-12-19 22:56                     ` Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87cz9cnvys.fsf_-_@gmail.com \
    --to=maxim.cournoyer@gmail.com \
    --cc=30948@debbugs.gnu.org \
    --cc=carlo@zancanaro.id.au \
    --cc=ludo@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.