From: "Ludovic Courtès" <ludo@gnu.org>
To: Ulf Herrman <striness@tilde.club>
Cc: 61803@debbugs.gnu.org
Subject: [bug#61803] [PATCH 0/3] [shepherd] improve race-free spawn+wait
Date: Thu, 02 Mar 2023 23:16:33 +0100 [thread overview]
Message-ID: <87h6v23ike.fsf@gnu.org> (raw)
In-Reply-To: <87cz5x1jr2.fsf@tilde.club> (Ulf Herrman's message of "Sat, 25 Feb 2023 16:08:01 -0600")
Hi Ulf,
Ulf Herrman <striness@tilde.club> skribis:
> These patches fill out shepherd's procedures for running processes to
> completion. They add a replacement for 'system' to complement the
> existing replacement for 'system*', and add a 'fork+exec+wait-process'
> procedure so that the flexibility of that family of procedures is
> available for this use case as well. It also improves error handling in
> the event that an exception occurs while spawning a process in the
> process monitor, which would normally kill that essential fiber.
Nice!
> Note: I previously tried to send this to guix-devel, but it didn't seem
> to make it (I didn't see it in the archives after half a day), and after
> some consideration I recalled that guix-patches exists. Is this the
> right place for shepherd patches?
Yes, as Tobias confirmed already. :-)
> From 64370a98dfc17f0531de7397a38362c03a1d89bc Mon Sep 17 00:00:00 2001
> From: ulfvonbelow <striness@tilde.club>
> Date: Sat, 25 Feb 2023 00:42:41 -0600
> Subject: [PATCH 1/3] service: Propagate exceptions while spawning in process
> monitor.
>
> * modules/shepherd/service.scm (unboxed-errors): new procedure.
> (boxed-errors): new syntax.
> (process-monitor): use it to propagate exceptions from fork+exec-command via
> reply channel.
> (spawn-via-monitor): new procedure.
> (spawn-command): use it.
Good catch! I added a test and a copyright line for you (let me know if
I got it wrong) and pushed as 18989f2fffa6ecdbd0f9b77834e1a54c9c45ee73.
> From 51ee63ace6f3f52eb196c990664cc6b9af3d3683 Mon Sep 17 00:00:00 2001
> From: ulfvonbelow <striness@tilde.club>
> Date: Sat, 25 Feb 2023 00:46:27 -0600
> Subject: [PATCH 2/3] service: accept fork+exec-command argument list in
> monitor.
>
> Sometimes it's necessary to run startup / shutdown programs as a certain user,
> in a certain directory, with certain environment variables, etc. Shepherd
> currently provides a replacement for system* that won't race against the
> child process auto-reaper, but this lacks the flexibility Shepherd users are
> used to.
>
> * modules/shepherd/service.scm (process-monitor): treat command instead as
> argument list to fork+exec-command.
> (spawn-via-monitor): update to new convention.
> (fork+exec+wait-command): new procedure.
I’ll take a closer look to this one and report back soon.
> From 177592ee9d4b7fc6dcc80e545e8ad615a1d6786c Mon Sep 17 00:00:00 2001
> From: ulfvonbelow <striness@tilde.club>
> Date: Sat, 25 Feb 2023 00:56:57 -0600
> Subject: [PATCH 3/3] service: add spawn-shell-command replacement for
> `system'.
>
> We already have a replacement for `system*' that avoids racing, but not for
> `system'.
>
> * configure.ac (SHELL): new substitution variable.
> * modules/shepherd/system.scm.in (%shell-filename): new variable.
> * modules/shepherd/service.scm
> (spawn-shell-command, real-system): new procedures.
> * modules/shepherd.scm (main): replace `system' with `spawn-shell-command'.
Out of curiosity, do you have a need for ‘system’? I’m inclined to
recommend against its use, in which case this patch is unnecessary.
> +(define %shell-filename "@SHELL@")
This is the configure-time shell so it will be wrong when
cross-compiling.
I’d just do:
(define %shell (or (getenv "SHELL") "/bin/sh"))
Thanks!
Ludo’.
next prev parent reply other threads:[~2023-03-02 22:17 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-25 22:08 [bug#61803] [PATCH 0/3] [shepherd] improve race-free spawn+wait Ulf Herrman
2023-02-26 14:17 ` Tobias Geerinckx-Rice via Guix-patches via
2023-03-02 22:16 ` Ludovic Courtès [this message]
2023-03-09 3:48 ` Ulf Herrman
2023-03-11 14:57 ` Ludovic Courtès
2023-03-04 22:09 ` 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=87h6v23ike.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=61803@debbugs.gnu.org \
--cc=striness@tilde.club \
/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.