Should Guix Home daemonize Shepherd?

Should Guix Home daemonize Shepherd?

[Kevin Boulain]

Hello,

I did a quick search in the tracker and the mailing list archives but
couldn't find a similar topic, apologies if this was already
discussed.

I was playing with Guix Home and I'm a bit surprised by how user
services are managed, especially the Shepherd itself.

Let's say I start from a clean state (I ran 'guix home reconfigure'
and rebooted the Guix machine) and I login via SSH:

$ ssh host
[...]
Service root has been started.
WARNING: Use of `load' in declarative module (#{ g56}#).  Add
#:declarative? #f to your define-module invocation.
Starting services...
Service example has been started.
$ exit

On the first login, the user's Shepherd is started and my 'example'
service is run. So far so good.

Now I login again:

$ ssh host
[...]
Starting services...
Service example has been started.
$ exit

The service is restarted again, meaning Shepherd and all the services
were killed when I logged out the first time. Is that expected?

Now, it's even weirder if I open two connections or more:

1$ ssh host
[...]
Starting services...
Service example has been started.
1$ # Don't exit this shell just yet.

2$ ssh host
2$ # Notice how the services weren't started again, I believe
$XDG_RUNTIME_DIR/on-first-login-executed is working as expected.

1$ exit # Now I logout from my first session.
2$ ps waux | grep $service # Nothing! The service has been killed
because I logged out from the first session.

Now, as long as the second session is kept open, the services won't be
restarted automatically, even if I open a third session. I need to
close all existing sessions and login again for the services to be
started.

Finally, I'm not sure Shepherd should share the terminal, it leads to
unfortunate situations like this (in addition to writing messages to
stdout like "Respawning $service", garbling any other process'
output):

$ ssh host
[...]
Starting services...
Service example has been started.
$ ^C # A single ^C will kill Shepherd, so I assume it wasn't properly
put in the background.
Exiting shepherd...
Service example has been stopped.
$ # I lost all my services :(

So, should the Shephered be daemonized when
$XDG_RUNTIME_DIR/on-first-login-executed is set?
I believe it would be a lot less surprising to users, unless the goal
was to prevent any lingering session in the first place.

---

For the record, I'm reproducing the above examples with this configuration:

;; guix home --dry-run reconfigure example.scm

(define-module (example)
  #:use-module (guix gexp)
  #:use-module (gnu home)
  #:use-module (gnu home services)
  #:use-module (gnu home services shepherd)
  #:use-module (gnu services)
  #:use-module (gnu packages base))

(define (example-shepherd-service _)
  (list
   (shepherd-service
    (documentation "Example service.")
    (provision '(example))
    (start #~(make-forkexec-constructor
              (list #$(file-append coreutils "/bin/sleep") "60")
              #:log-file (string-append (getenv "XDG_LOG_HOME")
"/example.log")))
    (stop #~(make-kill-destructor)))))

(define example-service-type
  (service-type
   (name 'example)
   (extensions
    (list
     (service-extension home-shepherd-service-type example-shepherd-service)))
   (default-value #nil)
   (description "Example service")))

(home-environment
 (services
  (list (service example-service-type))))

Re: Should Guix Home daemonize Shepherd?

[Kevin Boulain]

[-- Attachment #1: Type: text/plain, Size: 2067 bytes --]

So, I've done some digging and I'm coming back with two findings :)

First, Guix Home correctly tells the user Shepherd to daemonize itself
via an 'action'
(https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/home/services/shepherd.scm#n64)
but, from my understanding, the daemonization process is missing at
least a setsid call
(https://git.savannah.gnu.org/cgit/shepherd.git/tree/modules/shepherd/service.scm#n1421),
see the attached 'shepherd-setsid.patch'. This fixes the Shepherd
dying when exiting the SSH session or the Shepherd catching the ^C.
I guess it should also close std{in,out,err} like it's done for the
regular services
(https://git.savannah.gnu.org/cgit/shepherd.git/tree/modules/shepherd/service.scm#n806)
but this answers a part of my initial post.

Second, elogind (it's required by Guix Home to get the XDG_*
environment variables and also part of %desktop-services) will remove
/run/user/$uid when the session ends. It's standard, but the problem
is that Guix Home's 'on-first-login-executed' is located there,
alongside 'shepherd/socket' and probably the other user daemon's
sockets. This easily results in duplicate services being rerun when
the old ones haven't been killed because 'KillUserProcesses' is set to
'no' by default
(https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/services/desktop.scm#n937).
I don't think it's a bad idea to set KillUserProcesses=no (I remember
that when this was first introduced a lot of users complained, see for
example https://github.com/tmux/tmux/issues/428) but now we're in an
awkward position unless Guix Home users move everything out of
XDG_RUNTIME_DIR (for example, tmux's socket is in /tmp).

Thoughts? I must admit, I'm not sure how to address the elogind issue,
XDG_RUNTIME_DIR is ingrained in a lot of places (even in the Shepherd
https://git.savannah.gnu.org/cgit/shepherd.git/tree/modules/shepherd/support.scm#n284)
and asking users to override socket flags (and others) for all the
services they run (if at all possible) sounds a bit counterintuitive.
Or am I missing something obvious?

[-- Attachment #2: shepherd-setsid.patch --]
[-- Type: application/octet-stream, Size: 909 bytes --]

Detach from the controlling terminal when daemonizing

https://lists.gnu.org/archive/html/guix-devel/2022-03/msg00040.html

diff --git c/modules/shepherd/service.scm w/modules/shepherd/service.scm
index ad8608b..62f97bc 100644
--- c/modules/shepherd/service.scm
+++ w/modules/shepherd/service.scm
@@ -1420,8 +1420,12 @@ we want to receive these signals."
           (else
            (if (zero? (primitive-fork))
                (begin
-                 (catch-system-error (prctl PR_SET_CHILD_SUBREAPER 1))
-                 #t)
+                 (setsid)
+                 (if (zero? (primitive-fork))
+                     (begin
+                       (catch-system-error (prctl PR_SET_CHILD_SUBREAPER 1))
+                       #t)
+                     (primitive-exit 0)))
                (primitive-exit 0))))))
      (persistency
       "Save the current state of running and non-running services.

Re: Should Guix Home daemonize Shepherd?

[Andrew Tropin]

[-- Attachment #1: Type: text/plain, Size: 3845 bytes --]

On 2022-03-12 18:49, Kevin Boulain wrote:

> So, I've done some digging and I'm coming back with two findings :)
>
> First, Guix Home correctly tells the user Shepherd to daemonize itself
> via an 'action'
> (https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/home/services/shepherd.scm#n64)
> but, from my understanding, the daemonization process is missing at
> least a setsid call
> (https://git.savannah.gnu.org/cgit/shepherd.git/tree/modules/shepherd/service.scm#n1421),
> see the attached 'shepherd-setsid.patch'. This fixes the Shepherd
> dying when exiting the SSH session or the Shepherd catching the ^C.
> I guess it should also close std{in,out,err} like it's done for the
> regular services
> (https://git.savannah.gnu.org/cgit/shepherd.git/tree/modules/shepherd/service.scm#n806)
> but this answers a part of my initial post.

CCed Leo, Ludovic and Carlo.

>
> Second, elogind (it's required by Guix Home to get the XDG_*
> environment variables and also part of %desktop-services) will remove
> /run/user/$uid when the session ends. 

Actually, XDG_RUNTIME_DIR can be provided not only by elogind, but also
by pam_rundir or something similar, however in general it's true,
runtime dir will be removed when session ends.

> It's standard, but the problem is that Guix Home's
> 'on-first-login-executed' is located there, alongside
> 'shepherd/socket' and probably the other user daemon's sockets. This
> easily results in duplicate services being rerun when the old ones
> haven't been killed because 'KillUserProcesses' is set to 'no' by
> default
> (https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/services/desktop.scm#n937).
> I don't think it's a bad idea to set KillUserProcesses=no (I remember
> that when this was first introduced a lot of users complained, see for
> example https://github.com/tmux/tmux/issues/428) but now we're in an
> awkward position unless Guix Home users move everything out of
> XDG_RUNTIME_DIR (for example, tmux's socket is in /tmp).
>
> Thoughts? I must admit, I'm not sure how to address the elogind issue,
> XDG_RUNTIME_DIR is ingrained in a lot of places (even in the Shepherd
> https://git.savannah.gnu.org/cgit/shepherd.git/tree/modules/shepherd/support.scm#n284)
> and asking users to override socket flags (and others) for all the
> services they run (if at all possible) sounds a bit counterintuitive.

This is a tough question, faced it when only started to work on Guix
Home.  One idea I had back in the days is to have a possibility to get
lingering user shepherd, which starts on boot, the implementation
doesn't seem trivial so I decided to postpone experiments in this
direction for a better times.  Not sure if it's any perfect, but at
least is something to think about.

> Or am I missing something obvious?
> Detach from the controlling terminal when daemonizing
>
> https://lists.gnu.org/archive/html/guix-devel/2022-03/msg00040.html
>
> diff --git c/modules/shepherd/service.scm w/modules/shepherd/service.scm
> index ad8608b..62f97bc 100644
> --- c/modules/shepherd/service.scm
> +++ w/modules/shepherd/service.scm
> @@ -1420,8 +1420,12 @@ we want to receive these signals."
>            (else
>             (if (zero? (primitive-fork))
>                 (begin
> -                 (catch-system-error (prctl PR_SET_CHILD_SUBREAPER 1))
> -                 #t)
> +                 (setsid)
> +                 (if (zero? (primitive-fork))
> +                     (begin
> +                       (catch-system-error (prctl PR_SET_CHILD_SUBREAPER 1))
> +                       #t)
> +                     (primitive-exit 0)))
>                 (primitive-exit 0))))))
>       (persistency
>        "Save the current state of running and non-running services.

-- 
Best regards,
Andrew Tropin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

Re: Should Guix Home daemonize Shepherd?

[Ludovic Courtès]

Hi,

Kevin Boulain <kevinboulain@gmail.com> skribis:

> First, Guix Home correctly tells the user Shepherd to daemonize itself
> via an 'action'
> (https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/home/services/shepherd.scm#n64)
> but, from my understanding, the daemonization process is missing at
> least a setsid call
> (https://git.savannah.gnu.org/cgit/shepherd.git/tree/modules/shepherd/service.scm#n1421),
> see the attached 'shepherd-setsid.patch'. This fixes the Shepherd
> dying when exiting the SSH session or the Shepherd catching the ^C.
> I guess it should also close std{in,out,err} like it's done for the
> regular services
> (https://git.savannah.gnu.org/cgit/shepherd.git/tree/modules/shepherd/service.scm#n806)
> but this answers a part of my initial post.

[...]

> Detach from the controlling terminal when daemonizing
>
> https://lists.gnu.org/archive/html/guix-devel/2022-03/msg00040.html
>
> diff --git c/modules/shepherd/service.scm w/modules/shepherd/service.scm
> index ad8608b..62f97bc 100644
> --- c/modules/shepherd/service.scm
> +++ w/modules/shepherd/service.scm
> @@ -1420,8 +1420,12 @@ we want to receive these signals."
>            (else
>             (if (zero? (primitive-fork))
>                 (begin
> -                 (catch-system-error (prctl PR_SET_CHILD_SUBREAPER 1))
> -                 #t)
> +                 (setsid)
> +                 (if (zero? (primitive-fork))
> +                     (begin
> +                       (catch-system-error (prctl PR_SET_CHILD_SUBREAPER 1))
> +                       #t)
> +                     (primitive-exit 0)))

The extra ‘setsid’ call LGTM, but why add an extra ‘primitive-fork’
call?

Thanks in advance,
Ludo’.

Re: Should Guix Home daemonize Shepherd?

[Kevin Boulain]

On Sun, 15 May 2022 at 15:20, Ludovic Courtès <ludo@gnu.org> wrote:
>
> The extra ‘setsid’ call LGTM, but why add an extra ‘primitive-fork’
> call?

Sorry, but don't use this patch.
The setsid call makes things slightly better but if Shepherd prints
anything (for example, a service's status update), it will crash
(IIRC) because stdout still references the terminal. Ideally,
exec-command (from the same file) could be used to also close the open
file descriptors (including the terminal's) but that's done too late
and would also close the control socket (and probably other important
stuff).
I wasn't quite sure how to work around that while keeping the existing behavior.

On Fri, 6 May 2022 at 10:07, Andrew Tropin <andrew@trop.in> wrote:
>
> Actually, XDG_RUNTIME_DIR can be provided not only by elogind, but also
> by pam_rundir or something similar, however in general it's true,
> runtime dir will be removed when session ends.

That was my thought at first (for example
https://github.com/ifreund/dumb_runtime_dir can create the directory
but not remove it, solving a part of the issue). Sadly, we lose
elogind's built-in power management and some other packages depend on
it anyway.