Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optional integration with elogind for auto-quitting the user daemons on user logout #1

Closed
ZhaoLin1457 opened this issue Nov 27, 2020 · 41 comments

Comments

@ZhaoLin1457
Copy link

ZhaoLin1457 commented Nov 27, 2020

I believe that for the non-systemd Linux distributions which uses a traditional INIT system, would be exceptionally useful if this daemon would have an optional integration with elogind for auto-quitting the user daemons on user logout.

There's my reasoning and a little background for this proposal:

The systemd has the ability to run daemons as particular users, eventually one instance for each user, and to stop those particular daemon instances on user logout. This is what they call "user target" daemons, and a notable example of software which is designed to be run in this way is PipeWire, a multimedia server which is used massively by the modern KDE Plasma5 especially under Wayland sessions, and it is even proposed as a PulseAudio replacement into next Fedora 34.

For the non-systemd Linux distributions using a traditional INIT system, notable being Slackware, for the user sessions management there's the alternative of elogind. However, this elogind has no support for running daemons, either systemd wide or as user, and probably it is not supposed to have this, being rather a systemd-logind service alike.

From my own experiments with Slackware development tree, where's about to be merged the KDE Plasma5 and elogind, there's a way to start those "user target" daemon, notable being PulseAudio and PipeWire: via XDG autostart desktop files. However, there's no generic way to stop them, excluding configuring elogind to kill all user processes on logout, which have negative effects on running properly applications like TMux or Screen. That's why this should be generally avoided.

Fortunately, the PulseAudio has the ability to integrate with systemd (or elogind) and to watch the user logout, then to auto-quit its user instances on logout. While the PipeWire has no support for this features, being simply unaware of the running user sessions management.

How this systemd support for "user target" daemons is widelly available for the Linux distributions using it, I suspect that in the future would be even more software relying on it for its user daemons. Support which like I said, is not available yet for the non-systemd distributions using a classic init system. That's why I looked for a generic way to have this feature available.

There enters your "daemon" which is capable to run user daemons, it is even capable to watch their running and to re-spawn them, and it capable to run in background any imaginable program.

I believe that if your daemon gets the ability to watch the users logout, and to auto-quit its instances for a particular user, just like PulseAudio, it could be "the missing piece" for the non-systemd Linux distributions like Slackware (but not limited to) for properly handling those "user target" daemons designed for systemd.

Please consider my proposal, I believe that it would not be so complicated to add (looking at what PulseAudio do) and would transform your "daemon" in a fundamental piece of software for the non-systemd Linux distrbutions which still uses the classical INIT systems like is SysV init.

PS. I believe that what PulseAudio do for its systemd integration is there:

https://github.com/pulseaudio/pulseaudio/blob/master/src/modules/module-systemd-login.c

@raforg
Copy link
Owner

raforg commented Dec 16, 2020

Hi, Thanks for this request. It sounds interesting. I have to start by
saying that, in general, I couldn't disagree more that a daemon should
terminate when the user logs out. The whole point of daemon is to prevent
that. :-) All the other behaviours are just a bonus.

However, you clearly have a good use case for this. And I'm hoping that it
might be possible to achieve by just suppressing some of the daemony
behaviour. I agree with the old adage that any feature that can't be turned
off is a bug, even if it's the primary feature. :-)

And daemon already completely suppresses daemony behaviour when started by
pid 1, or when connected directly to a network socket, or when the
--foreground option is used.

Perhaps this just needs a new option to cause daemon to do the initial
fork(), but to suppress the setsid() and the 2nd fork() that turns it into a
true daemon. I think that might hopefully be enough to cause daemon and its
client processes to be terminated when the user logs out of the the original
process session.

Do you think that that would do the trick? If so, and if you are in a
position to test it, I could send you some candidates to try out (email me).

Does that sound plausible? Or is it wishful thinking?

If it is wishful thinkng, it might be necessary to add lots of code to
listen to dbus or whatever it is that PulseAudio is doing, and pay attention
to the outside world. I'd like to avoid that if possible, but if it's
unavoidable, it's still worth doing.

If you are in a position to contribute such a change, that would be great.
If I need to do it, I'd need to know how to test it properly. Are you in a
position to test it? Or can you give me a detailed instructions on how to
test it?

Also, I can't work on it right now. But I could probably start next month or
so. I expect it'll take some time to wrap my head around what that
PulseAudio module is actually doing, and how PulseAudio uses it.

Is there any elogind-specific documentation on how elogind manages user
sessions (or logind). Or do I just have to read its code as well?

@ZhaoLin1457
Copy link
Author

First of all, thanks you for your interest for my feature(s) proposal!

Unfortunately, I am not a C/C++ programmer, then I cannot help with patches. However, I am more than willing to help with the testing, in any way you consider that is needed.

And I can help with a presentation of what I think is needed your daemon should do, but I guess it's has already all features out this user login sessions awareness.

Further, I will talk about pipewire as client program, but I am certainly that there are many more user target daemons which may people will want to run without systemd.

A little rehash: a systemd "user target" daemon is just a program, which has no idea how to daemonize itself. In practice, when the user logins, the systemd forks itself and runs an instance as this particular user, where it continues to run those user target services. When the user logs out, that systemd instance (running as user) stops its services and exits.

BUT, there are some rules on running those user target services:

  • for every user is ran only a single instance - for example if there are john and mary logged in, for every one user runs a single pipewire instance, no matter if john logins on 3 consoles and mary on 4 consoles.
  • the client program is supervised and if it crashes, the program is executed again.

After considerations and thinking done with my friends from Slackware community, apparently there are two main user targets which may be of interests for a non-systemd Linux distribution:

  • the user console, where the user logins to Linux console, and there the user target services are executed in background right after login.
  • the user graphics, where the user logins to a graphical session, and first is started the X11 server or Wayland, then are executed the user target services in background.

After our experiments, for both console and graphics targets we have ways to start those user target daemons.

  • for the user console we can run a main script from the shells login scripts/profiles, which then to run those daemons.
  • for the user graphics we think that the best way is to use the XDG autostart to launch those services.

Right now, we need to start as user graphics target the pipewire instances needed by Wayland/Plasm5 and we can start it properly with a desktop file put in /etc/xdg/autostart with the following content:

[Desktop Entry]
Version=1.0
Name=PipeWire Media System
Comment=Start the PipeWire Media System
Exec=/usr/bin/pipewire
Terminal=false
Type=Application
X-GNOME-Autostart-Phase=Initialization
X-KDE-autostart-phase=1

However, there are some caveats:

  • the pipewire instance is no supervised for continuous running
  • the pipewire instance is not supervised for a single instance for user
  • the pipewire instance is not stopped when the user logouts and continues to run in backgroud.

We have a way also to stop this pipewire instance on logout, instructing the elogind to kill all user processes on logout - which have also negative effects like killing the screen or tmux instances.

Our idea is to use your daemon as supervisor in those XDG desktop files, in a way like:

Version=1.0
Name=PipeWire Media System
Comment=Start the PipeWire Media System
Exec=/user/bin/daemon <parameters> /usr/bin/pipewire
Terminal=false
Type=Application
X-GNOME-Autostart-Phase=Initialization
X-KDE-autostart-phase=1

With the following conditions:

  • there is no need for your daemon to sink in background, because the is no pty on XDG autostart, then probably we will use the --foreground option
  • the client program should be supervised for continuous running, then probably we will use the --respawn option
  • there should be started only a single instance of the client program (for example: pipewire) but I have not understand yet how to instruct your daemon for this
  • the daemon should watch the user logout and to quit its client and itself when this happens.
  • ideally the daemon should put its pid or other runtime files into $XDG_RUNTIME_DIR which is /run/user/1000 for example

Now, regarding the elogind API documentation, it is in fact the extracted logind daemon from systemd, then the API for logind from systemd documentation applies well, from what I understand. And there is the systemd docs for logind:

https://www.freedesktop.org/software/systemd/man/sd-login.html

Also from what I understand, the analogy goes so far that you can build your daemon under a systemd driven Linux distribution and against systemd, and this feature of watching user sessions will behave in a similar way.

Another thing is that a friend of mine found a snippet to talk with elogind/systemd which I show you bellow

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <string.h>
#include <errno.h>
#include <pwd.h>
#include <sys/epoll.h>
#include <systemd/sd-login.h>


#define ERR_EXIT(msg)       \
    do                      \
    {                       \
        perror(msg);        \
        exit(EXIT_FAILURE); \
    } while (0)

#define MAX_EVENT_NUMBER 1024
#define EPWAIT_TIMEOUT 500 // millisecond


int main()
{
    int i = 0;

    int epfd = 0;
    int epwait_timeout = EPWAIT_TIMEOUT;
    struct epoll_event ev, events[MAX_EVENT_NUMBER];

    sd_login_monitor *lg_mon = NULL;
    int lg_mon_fd = 0;
    int lg_mon_events = 0;
    uid_t *users = NULL;
    int nusers = 0;
    uint64_t t;
    int lg_mon_timeout = 0; // in msec

    // category: seat, session, uid, machine 
    if (sd_login_monitor_new("uid", &lg_mon) < 0) {
        ERR_EXIT("sd_login_monitor_new");
    }

    lg_mon_fd = sd_login_monitor_get_fd(lg_mon);

    if (lg_mon_fd < 0) {
        ERR_EXIT("sd_login_monitor_get_fd");
    }

    sd_login_monitor_get_timeout(lg_mon, &t); // about this timeout, see man sd_login_monitor(3)

    if (t == (uint64_t)-1) {
        lg_mon_timeout = -1;
    } 
    else  {
        struct timespec ts;
        uint64_t n;
        
        clock_gettime(CLOCK_MONOTONIC, &ts);
        
        n = (uint64_t)ts.tv_sec * 1000000 + ts.tv_nsec / 1000;
        
        lg_mon_timeout = t > n ? (int)((t - n + 999) / 1000) : 0;
    }

    printf("login monitor timeout = %d milliseconds\n", lg_mon_timeout);
    
    epwait_timeout = (lg_mon_timeout == -1) ? epwait_timeout : ((epwait_timeout < lg_mon_timeout) ? epwait_timeout : lg_mon_timeout);

    // about events, see man sd_login_monitor(3)
    lg_mon_events = sd_login_monitor_get_events(lg_mon);

    // epoll
    epfd = epoll_create(256);

    if (epfd < 0) {
        // TODO: should do the cleanup
        ERR_EXIT("epoll_create");
    }

    // set event's file descriptor
    ev.data.fd = lg_mon_fd;
    // set event types
    ev.events = lg_mon_events | EPOLLET;
    // register epoll event
    epoll_ctl(epfd, EPOLL_CTL_ADD, lg_mon_fd, &ev);

    for (;;) {
        // wait for epoll events
        int nfds = epoll_wait(epfd, events, MAX_EVENT_NUMBER, epwait_timeout);

        // process all the events
        for (i = 0; i < nfds; i++) {
            int fd = events[i].data.fd;

            if (fd == lg_mon_fd) {
                // lg_mon_fd needs to be reset, see man sd_login_monitor(3)
                sd_login_monitor_flush(lg_mon);

                nusers = sd_get_uids(&users);
                
                if (nusers < 0) {
                    perror("sd_get_uids");
                } 
                else {
                    i = 0;
                    
                    for (; i < nusers; i++) {

                        printf("%d: %d\n", i + 1, users[i]);
                    }
                    
                    free(users);
                    
                    users = NULL;
                }
            }
        } // for i <- 0 to nfds - 1
    }     // loop

    // cleanup
    close(epfd);
    close(lg_mon_fd);
    
    sd_login_monitor_unref(lg_mon);
    
    return 0;
}

This program could be compiled against elogind with command:

gcc -I/usr/include/elogind -o sd-login-monitor sd-login-monitor.c -lelogind

and again systemd with the command:

gcc -o sd-login-monitor sd-login-monitor.c -lsystemd

That's all. Thanks you very much in anticipation.

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Dec 23, 2020

Hi!

This friend of mine who found on Internet the snippet shown in the previous post, managed to make some changes to snippet, as to report the login sessions of a given user, defined by his UID.

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <string.h>
#include <errno.h>
#include <pwd.h>
#include <sys/epoll.h>
#include <systemd/sd-login.h>


#define ERR_EXIT(msg)       \
    do                      \
    {                       \
        perror(msg);        \
        exit(EXIT_FAILURE); \
    } while (0)

#define MAX_EVENT_NUMBER 1024
#define EPWAIT_TIMEOUT 500 // millisecond


int main()
{
    int i = 0;

    int epfd = 0;
    int epwait_timeout = EPWAIT_TIMEOUT;
    struct epoll_event ev, events[MAX_EVENT_NUMBER];

    sd_login_monitor *lg_mon = NULL;
    int lg_mon_fd = 0;
    int lg_mon_events = 0;
    uint64_t t;
    int lg_mon_timeout = 0; // in msec

    int ret;
    int lg_uid = 1000; // First unpriviledged user.

    // category: seat, session, uid, machine 
    if (sd_login_monitor_new("uid", &lg_mon) < 0) {
        ERR_EXIT("sd_login_monitor_new");
    }

    lg_mon_fd = sd_login_monitor_get_fd(lg_mon);

    if (lg_mon_fd < 0) {
        ERR_EXIT("sd_login_monitor_get_fd");
    }

    sd_login_monitor_get_timeout(lg_mon, &t); // about this timeout, see man sd_login_monitor(3)

    if (t == (uint64_t)-1) {
        lg_mon_timeout = -1;
    } 
    else  {
        struct timespec ts;
        uint64_t n;
        
        clock_gettime(CLOCK_MONOTONIC, &ts);
        
        n = (uint64_t)ts.tv_sec * 1000000 + ts.tv_nsec / 1000;
        
        lg_mon_timeout = t > n ? (int)((t - n + 999) / 1000) : 0;
    }

    printf("login monitor timeout = %d milliseconds\n", lg_mon_timeout);
    
    epwait_timeout = (lg_mon_timeout == -1) ? epwait_timeout : ((epwait_timeout < lg_mon_timeout) ? epwait_timeout : lg_mon_timeout);

    // about events, see man sd_login_monitor(3)
    lg_mon_events = sd_login_monitor_get_events(lg_mon);

    // epoll
    epfd = epoll_create(256);

    if (epfd < 0) {
        // TODO: should do the cleanup
        ERR_EXIT("epoll_create");
    }

    // set event's file descriptor
    ev.data.fd = lg_mon_fd;
    // set event types
    ev.events = lg_mon_events | EPOLLET;
    // register epoll event
    epoll_ctl(epfd, EPOLL_CTL_ADD, lg_mon_fd, &ev);

    for (;;) {
        // wait for epoll events
        int nfds = epoll_wait(epfd, events, MAX_EVENT_NUMBER, epwait_timeout);

        // process all the events
        for (i = 0; i < nfds; i++) {
            int fd = events[i].data.fd;

            if (fd == lg_mon_fd) {
                // lg_mon_fd needs to be reset, see man sd_login_monitor(3)
                sd_login_monitor_flush(lg_mon);

                // we will only count the login sessions for the given UID
                ret = sd_uid_get_sessions(lg_uid, 0, NULL);

                if (ret < 0) {
                    perror("sd_uid_get_sessions");
                }
                else {
                    printf("UID %d has %d login sesion(s)\n", lg_uid, ret);
                }
            }
        } // for i <- 0 to nfds - 1
    }     // loop

    // cleanup
    close(epfd);
    close(lg_mon_fd);
    
    sd_login_monitor_unref(lg_mon);
    
    return 0;
}

This snippet, watch the hard-coded the first unpriviledged user in Slackwware with UID 1000, and apparently it is capable to count its login sessions, showing something like:

root@darkstar:~/samples# ./sd-login-monitor
login monitor timeout = -1 milliseconds
UID 1000 has 0 login sesion(s)
UID 1000 has 1 login sesion(s)
UID 1000 has 1 login sesion(s)
UID 1000 has 1 login sesion(s)
UID 1000 has 1 login sesion(s)
UID 1000 has 1 login sesion(s)
UID 1000 has 2 login sesion(s)
UID 1000 has 2 login sesion(s)
UID 1000 has 2 login sesion(s)
UID 1000 has 1 login sesion(s)
UID 1000 has 1 login sesion(s)
UID 1000 has 1 login sesion(s)
UID 1000 has 1 login sesion(s)
UID 1000 has 0 login sesion(s)
UID 1000 has 0 login sesion(s)
UID 1000 has 0 login sesion(s)
UID 1000 has 0 login sesion(s)
UID 1000 has 0 login sesion(s)

This is the report after switching to VT1, login as this first user, then doing same on VT2, and doing logout from those two consoles.

So, yes. it tracks the number of login sessions for UID 1000.

With the hope that those new information are useful, I wish you all the best!

@raforg
Copy link
Owner

raforg commented Jan 8, 2021

Hi,

Thanks for all that. It's really very useful, and it looks like everything I should need to get started.

However, when I compile the above on Debian, it doesn't seem to work the way I would expect. Running it from an ssh session as my only login to the virtual machine, I get the first line "login monitor timeout = -1 milliseconds", but nothing after that. And I am UID 1000 so being logged in via ssh should count as a session (I assume). I expected it to output a line every 500ms saying that there was 1 session, but it didn't (Note that I hadn't read the code properly yet).

But when I did a graphical login to the VM, it output 6 lines, 3 saying 1 session, and another 3 saying 2 sessions. It's as though it was counting the ssh login but only after the graphical login happened. There probably wasn't an event to retrieve until then, which probably makes sense.

But additional ssh logins while it's running don't cause it to output anything, which doesn't make sense. Do ssh logins count as a session or not? They seem to, since logging out of the graphical session causes it to report 1 session afterwards, which I assume is the original ssh login. If it isn't, what is it?

Anyway, I'll read the code properly and the associated manpages until it all makes sense.

I can get it to behave like I expect if I output the number of sessions after each epoll_wait() timeout. But it seems as though the first ssh login counts as a session but that any additional concurrent ssh logins don't add to the number of sessions. That's probably fine for your purposes. As long as the number of sessions goes to zero when all logins are over, that should be fine.

I want to release a minor update before adding this. But it shouldn't take too long.

cheers,
raf

P.S. The --foreground option is probably OK if autostart backgrounds it but you should probably test it with and without that option and see if it makes any difference. The --respawn option will keep it running if it terminates, but be warned that daemon will introduce delays if it thinks that the client process is terminating too quickly too many times. That can be tweaked and overridden with other options. To ensure that only a single instance is running, you need to use the --name option. That will be needed for this anyway. Putting the pidfile into $XDG_RUNTIME_DIR requires using the --pidfiles option. There'll be a new option for this new auto-stopping behaviour, but I haven't decided on its name yet (names are hard). It'll probably be --bind (as in bound to the user session).

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Jan 9, 2021

Hi,

first of all, thank you very much for those fantastic news! I am anytime at your disposition for any "field testing" you consider to be needed on the real target operating system of this feature: Slackware.

Regarding epoll and printing the user sessions, I've noticed the behavior described by you, and as a uneducated guess, apparently, is not only about pooling, but also about (e)logind sending events about things happening within user sessions.

So, we get an output "when something happens" on the (e)logind management of the user sessions, then maybe is logical that our sessions probe will not see the event of logging in, because when it is started, that already happened. And also looks quite certain that the daemon will never see an initial count of 0 user session, like I got watching them from root account.

However, in my humble opinion, the most important information notified by (e)logind is that the user sessions count dropped at 0, case when our supervisor (your daemon) should quit, together with the supervised program.

Probably is possible to query (e)logind and to classify the user sessions, either being remote or local console, or graphical. But for now let's keep the things KISS, and we will see if there's a need to tune further the behavior.

In other hand, I should inform you about some possible bad news...

Like I said already, we intend to launch the supervised PipeWire from a desktop file on XDG autostart - and there the shell variables are not expanded. So, a desktop file like the one bellow will not work as expected:

Version=1.0
Name=PipeWire Media System
Comment=Start the PipeWire Media System
Exec=/user/bin/daemon --foreground --respawn --name pipepire-daemon --pidfiles $XDG_RUNTIME_DIR <parameters> /usr/bin/pipewire
Terminal=false
Type=Application
X-GNOME-Autostart-Phase=Initialization
X-KDE-autostart-phase=1

I did some tests using a "fake" script, and I am very sure that $XDG_RUNTIME_DIR will be passed ad-literam, without being expanded to the real value, i.e. /run/user/1000 but the daemon will get literally "$XDG_RUNTIME_DIR" as value.

So, there I have an idea: either your daemon to get the ability to lookup itself on the environment variables for the values starting with $ or even better, your new --bind option to imply switching automatically the --pidfiles value to /run/user/<UID>

In other hand, even the $XDG_RUNTIME_DIR looks as the best place to put the PID files, a friend of mine raised the point that the /run/user/<UID> folder is created by (e)logind on first user login session and removed by the same (e)logind when the user logs out, then he questioned if is proper to keep there our PID files, because there's the risk them to be deleted by (e)logind before the daemon to react.

Similar concerns looks being raised by Lennart Poetering into PulseAudio code:

    /* Positive exit_idle_time is only useful when we have no session tracking
     * capability, so we can set it to 0 now that we have detected a session.
     * The benefit of setting exit_idle_time to 0 is that pulseaudio will exit
     * immediately when the session ends. That in turn is useful, because some
     * systems (those that use pam_systemd but don't use systemd for managing
     * pulseaudio) clean $XDG_RUNTIME_DIR on logout, but fail to terminate all
     * services that depend on the files in $XDG_RUNTIME_DIR. The directory
     * contains our sockets, and if the sockets are removed without terminating
     * pulseaudio, a quick relogin will likely cause trouble, because a new
     * instance will be spawned while the old instance is still running. */

https://github.com/pulseaudio/pulseaudio/blob/master/src/modules/module-systemd-login.c#L88

So, for those reasons he proposed an eventual alternate path for the PID files, something like: $HOME/.daemon or $HOME/.local/share/daemon, and in any case the directory should be auto-created, if does not exists.

Certainly, I would love to test (and to report back) which one is the better way, after you to add the changes in daemon's code.

To test myself the different variants with a single build of daemon, I think that would be very nice if it will auto-expand at least some variables for --pidfiles like is $UID, $USER and $HOME and also will be exceptional to deal with $XDG_RUNTIME_DIR

@raforg
Copy link
Owner

raforg commented Jan 18, 2021

Thanks for the offer to test it. I can test it on Debian, but you testing it on Slackware with the actual intended use case will be more helpful.

The problem of the environment variables on the command line could be solved by invoking daemon like this:

sh -c 'daemon ... --pidfiles $XDG_RUNTIME_DIR ...'

That way the shell does the expansion. However, I might add basic environment variable and ~ home directory expansion anyway, as it could be helpful in the configuration files as well. But technically, it would not be backwards compatible, which I wouldn't like, so I'd probably add an --expand option to enable it.

The pidfile directory being deleted prematurely would be a problem, so I would also recommend not using it. Another reason not to use it is that daemon's --list option works slightly better when all of the pidfiles in the pidfiles directory were started by daemon(1). The reasons are explained in the manpage which suggests considering having a separate pidfiles directory just for daemons that are started by daemon(1).

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Jan 19, 2021

Hi,

regarding the PID files path, what you think about this idea:

The --bind option is supposed to be used always with daemon running as whatever user, then while using this option, the daemon could automatically adjust the PID files location to ~/.daemon which directory to be auto-created, if it does not exists yet.

On our brainstorming to find the best solution while requesting the minimal changes to you, this looks the simpler solution to have an "per user" setup which does not disturb the functionality for another users. And at least in Slackware, the path ~/.daemon is not used by any other program.

And thanks for considering the volatility of XDG_RUNTIME_DIR and the recommendation given along.

Also, from our side experiments, looks like this path may or may not be deleted before the daemon reacts. And as you says, the daemon will do its job better when it have a dedicated place to put its PID files.

@raforg
Copy link
Owner

raforg commented Feb 3, 2021

Thanks for the suggestion, but I don't think it's a good idea for the --bind option to implicitly affect the pidfiles directory. It sounds sensible, but I think it would be better if that remained explicit. My reason is that when the user wants to check on or interact with a daemon (i.e. with --running --stop --restart --signal --list), they will need to provide the --pidfiles option explicitly. If --bind implicitly alters the pidfiles directory, this will obscure the fact that the user has to match an implicitly selected pidfiles directory. And the documentation about default pidfiles directories is already complicated enough. I'd rather not complicate it further.

But being explicit doesn't necessarily mean that it has to be on the command line. It can be in a configuration file instead. Then it will apply both to uses of the --bind option, and to uses of those other options. I think that's a better way to manage the pidfiles directory. For example, /etc/daemon.conf (or /etc/dademon.conf.d/*) could contain:

* pidfiles=~/.daemon

But I will make deamon auto-create the pidfiles directory if it is within the user's home directory. That's an excellent idea.

By the way, when I said that the automatic deletion of the $XDG_RUNTIME_DIR directory would be a problem, I was thinking of using a separate watchdog process to stop the daemon. I even had a funny name for it: "daemokles". That method would have relied on the existence of the pidfile. But I'm thinking it will be more reliable to handle it within the daemon process itself, which means that the deletion of the pidfiles directory wouldn't prevent it from doing the right thing. But I'd still rather avoid using a volatile pidfiles directory.

P.S. Sorry for all the delays in responding. I appreciate your patience. And I am getting much closer to doing this. :-)

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Feb 26, 2021

Let me know when you need me to test this feature.

And thanks for your efforts!

BTW, would be very nice if this feature would become available on the near future, as Slackware is right now on 15.0 alpha1, and we hope for proposing quickly your daemon for adoption, to be available on Slackware 15.0 for supervising the PipeWire services and eventual other "user target" daemons.

@raforg
Copy link
Owner

raforg commented Feb 28, 2021

Hi, I finally implemented the new --bind option. It's attached. Please test it. It works with systemd. Hopefully it'll work with elogind as well. The configure scripts looks for both. Note that I haven't yet done the environment variable and ~ expansion that we discussed yet, or the automatic creation of pidfiles directories within the user's home directory yet. But they'll be ready soon. Daemons that use --bind don't need to be named, and so they don't need pidfiles, so those features aren't essential for testing --bind.

daemon-0.8a.tar.gz

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 1, 2021

Hi,

after building the daemon with /usr prefix, I've decided to hire it right for its actual job: babysitting the PipeWire's user target daemon.

[Desktop Entry]
Version=1.0
Name=PipeWire Media System
Comment=Start the PipeWire Media System
Exec=/usr/bin/daemon -fB /usr/bin/pipewire
Terminal=false
Type=Application
X-GNOME-Autostart-Phase=Initialization
X-KDE-autostart-phase=1

The result of my experiment is that it works perfectly, quitting itself (and the PipeWire instance) on KDE Plasma5 session logout both as root and unprivileged user.

You are great, man! Thank you very much, you did a fantastic job!

Here, I have some notes:

  • you forgot an > character on line 530 and the man generator yells.
  • the Slackware uses /usr/man for manpages, so would be very nice to have on Makefile or even on configure a variable or parameter to setup appropriately the MAN_SYSDIR from $(PREFIX)/share/man to $(PREFIX)/man . How about a MAN_DIR and/or a --mandir parameter?
  • same applies for docs dir, which is for us: /usr/doc
  • also would be very nice nice to have the ability to configure the --sysconfdir aka the /etc and anyways I believe that CONF_INSDIR should respect DESTDIR, as in CONF_INSDIR := $(DESTDIR)/etc

Now I will try to build a proper Slackware package for our smart daemon supervisor...

PS. I think that failing on flushing the sd monitor and getting the sessions count should be treated as fatal system errors because probably something very bad happens on logind.

@raforg
Copy link
Owner

raforg commented Mar 1, 2021

Thanks for the testing. That's great news. And thanks for the manpage bug.

What's the output of uname -a on Slackware? It'll be easier for me to just make
all of those locations automatic on Slackware without needing to create additional
configure options.

I assume Slackware will still want to use /etc for config files. If not, that can be changed
with "configure --prefix=/usr/local" or similar. e.g. for macports, --prefix=/opt/local
causes daemon to use /opt/local/etc. On BSD systems, the default is /usr/local/etc.
But --prefix=/usr would still use /etc.

But I'll add $(DESTDIR) to CONF_INSDIR anyway. Thanks again.

I'll make the other changes quickly (now that I've gotten started).

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 1, 2021

Hi,
there's the output of uname -a

root@darkstar:~# uname -a
Linux darkstar.example.org 5.10.18 #1 SMP Tue Feb 23 15:26:27 CST 2021 x86_64 Intel(R) Atom(TM) x5-Z8350  CPU @ 1.44GHz GenuineIntel GNU/Linux
root@darkstar:~# 

Unfortunatelly, the Slackware does not customize the uname output too much, but you can look for /etc/slackware-version and if you find this file, definitely the build runs on Slackware or one of its derivative distributions.

And yes, we use /etc for sysconfdir but I asked for $(DESTDIR) tuning for making packages, where I should run

make DESTDIR=/tmp/package-daemon install

That temporary folder is then used by the build script to create the Slackware package, which is essentially a tarball.

BTW, I wonder if you can't do a (beta?) release focusing on fixes of build system now that that --bind works, as to permit me to push for public testing on the Slackware forum and to propose the adoption of your daemon on Slackware. Then, you can add the other features on the next release.

This will be really useful.

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 1, 2021

BTW, I seen that your build system can generate packages of different types.

So, I wonder if you can add also Slackware on your known packages to build, as it is very simple:

  1. you install everything on a temporary folder like /tmp/package-daemon, with the Slackware specs for /etc, /usr, /usr/doc and /usr/man
  2. You put you docs (e.g. readmes or licenses) on /usr/doc/daemon-$VERSION
  3. You install a description file on /install/slack-desc like one bellow
  4. You run from your temporary folder the command: /sbin/makepkg -l y -c n /tmp/daemon-$VERSION-$ARCH-1.txz

Where $VERSION is the version of daemon and $ARCH is the build architecture.

The result will be a Slackware package named like /tmp/daemon-0.8.0-x86_64-1.txz

This is an example of /install/slack-desc

# HOW TO EDIT THIS FILE:
# The "handy ruler" below makes it easier to edit a package description. Line
# up the first '|' above the ':' following the base package name, and the '|'
# on the right side marks the last column you can put a character in. You must
# make exactly 11 lines for the formatting to be correct. It's also
# customary to leave one space after the ':'.


      |-----handy-ruler------------------------------------------------------|
daemon: daemon (turns other processes into daemons)
daemon:
daemon: Daemon turns other processes into daemons. There are many tasks that
daemon: need to be performed to correctly set up a daemon process. This can
daemon: be tedious. Daemon performs these tasks for other processes. This is
daemon: useful for writing daemons in languages other than C, C++ or Perl
daemon: (e.g. `/bin/sh`, Java).
daemon:
daemon: If you want to write daemons in languages that can link against C
daemon: functions (e.g. C, C++), see libslack which contains the core
daemon: functionality of daemon.

It should have exactly 11 lines and the text to respect the size of the handy ruler.

Ideally, you should put also a link to daemon homepage, but trying to do the things like you do for RPM, there is no space for.

So, this one would be perfect:

# HOW TO EDIT THIS FILE:
# The "handy ruler" below makes it easier to edit a package description. Line
# up the first '|' above the ':' following the base package name, and the '|'
# on the right side marks the last column you can put a character in. You must
# make exactly 11 lines for the formatting to be correct. It's also
# customary to leave one space after the ':'.


      |-----handy-ruler------------------------------------------------------|
daemon: daemon (turns other processes into daemons)
daemon:
daemon: Daemon turns other processes into daemons. There are many tasks that
daemon: need to be performed to correctly set up a daemon process. This can
daemon: be tedious. Daemon performs these tasks for other processes. This is
daemon: useful for writing daemons in languages other than C, C++ or Perl
daemon: (e.g. `/bin/sh`, Java).
daemon: 
daemon: http://libslack.org/daemon/
daemon: 
daemon: 

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 1, 2021

To see what Slackware do with its package builds, here is a build script daemon.Slackbuild which I believe that generate a proper package for Slackware:

#!/bin/bash

# Copyright 2005-2018  Patrick J. Volkerding, Sebeka, MN, USA
# All rights reserved.
#
# Redistribution and use of this script, with or without modification, is
# permitted provided that the following conditions are met:
#
# 1. Redistributions of this script must retain the above copyright
#    notice, this list of conditions and the following disclaimer.
#
#  THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED
#  WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
#  MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO
#  EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
#  SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
#  PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
#  OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
#  WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
#  OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
#  ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

cd $(dirname $0) ; CWD=$(pwd)

PKGNAM=daemon
VERSION=${VERSION:-$(echo $PKGNAM-*.tar.?z | rev | cut -f 3- -d . | cut -f 1 -d - | rev)}
BUILD=${BUILD:-1}

NUMJOBS=${NUMJOBS:-" -j$(expr $(nproc) + 1) "}

# Automatically determine the architecture we're building on:
if [ -z "$ARCH" ]; then
  case "$( uname -m )" in
    i?86) export ARCH=i586 ;;
    arm*) export ARCH=arm ;;
    # Unless $ARCH is already set, use uname -m for all other archs:
       *) export ARCH=$( uname -m ) ;;
  esac
fi

# If the variable PRINT_PACKAGE_NAME is set, then this script will report what
# the name of the created package would be, and then exit. This information
# could be useful to other scripts.
if [ ! -z "${PRINT_PACKAGE_NAME}" ]; then
  echo "$PKGNAM-$VERSION-$ARCH-$BUILD.txz"
  exit 0
fi

TMP=${TMP:-/tmp}
PKG=$TMP/package-$PKGNAM

rm -rf $PKG
mkdir -p $TMP $PKG
cd $TMP
rm -rf $PKGNAM-$VERSION
tar xvf $CWD/$PKGNAM-$VERSION.tar.?z || exit 1
cd $PKGNAM-$VERSION || exit 1
cat $CWD/$PKGNAM-$VERSION.diff | patch -p1 || exit 1
chown -R root:root .
find . \
  \( -perm 777 -o -perm 775 -o -perm 711 -o -perm 555 -o -perm 511 \) \
  -exec chmod 755 {} \+ -o \
  \( -perm 666 -o -perm 664 -o -perm 600 -o -perm 444 -o -perm 440 -o -perm 400 \) \
  -exec chmod 644 {} \+

./configure --prefix=/usr

make $NUMJOBS || make || exit 1
make html || exit 1
make install DESTDIR=$PKG MAN_INSDIR=$PKG/usr/man
make install-daemon-conf DESTDIR=$PKG

mkdir -p $PKG/usr/doc/$PKGNAM-$VERSION
cp -a \
  COPYING* INSTALL LICENSE README* REFERENCES daemon.1.html \
  $PKG/usr/doc/$PKGNAM-$VERSION

# If there's a CHANGELOG, installing at least part of the recent history
# is useful, but don't let it get totally out of control:
if [ -r CHANGELOG ]; then
  DOCSDIR=$(echo $PKG/usr/doc/*-$VERSION)
  cat CHANGELOG | head -n 1000 > $DOCSDIR/CHANGELOG
  touch -r CHANGELOG $DOCSDIR/CHANGELOG
fi

mkdir -p $PKG/install
cat $CWD/slack-desc > $PKG/install/slack-desc

# Build the package:
cd $PKG
/sbin/makepkg -l y -c n $TMP/$PKGNAM-$VERSION-$ARCH-$BUILD.txz

This is the daemon-0.8a.diff which I used to fix some things, as we talked already:

diff -urN daemon-0.8a.orig/Makefile daemon-0.8a/Makefile
--- daemon-0.8a.orig/Makefile	2021-02-28 15:55:52.000000000 +0200
+++ daemon-0.8a/Makefile	2021-03-01 14:37:18.333068813 +0200
@@ -42,7 +42,7 @@
 endif
 HDR_INSDIR := $(PREFIX)/include
 DATA_INSDIR := $(PREFIX)/share
-CONF_INSDIR := /etc
+CONF_INSDIR := $(DESTDIR)/etc
 APP_MANSECT := 1
 LIB_MANSECT := 3
 FMT_MANSECT := 5
diff -urN daemon-0.8a.orig/daemon.c daemon-0.8a/daemon.c
--- daemon-0.8a.orig/daemon.c	2021-02-28 15:55:50.000000000 +0200
+++ daemon-0.8a/daemon.c	2021-03-01 14:36:58.844068234 +0200
@@ -527,7 +527,7 @@
 
 =item C<-B>, C<--bind>
 
-Automatically terminate the client process (and I<daemon(1) itself) as soon
+Automatically terminate the client process (and I<daemon(1)> itself) as soon
 as the user has no I<systemd-logind(8)> (or I<elogind(8)>) user sessions. In
 other words, automatically terminate when the user logs out. If the user has
 no sessions to start with, the client process will be terminated

The slack-desc file is like ones from the previous post.

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 1, 2021

BTW, why we need a separate line of make install-daemon-conf DESTDIR=$PKG to install the configuration file and the /etc folders?

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 2, 2021

Please permit the tilde expansion for home, also for root, like this

diff -urN daemon-20210303_92768de.orig/daemon.c daemon-20210303_92768de/daemon.c
--- daemon-20210303_92768de.orig/daemon.c	2021-03-02 16:41:47.000000000 +0200
+++ daemon-20210303_92768de/daemon.c	2021-03-02 17:01:40.022784620 +0200
@@ -1312,6 +1312,51 @@
 	expanding = mem_strdup(input);
 	len = strlen(expanding);
 
+	/* Replace home directory notation: ~ or ~username */
+
+	for (i = 0; i < len; ++i)
+	{
+		if (expanding[i] == '~' && (i == 0 || is_space(expanding[i - 1]) || expanding[i - 1] == ':' || expanding[i - 1] == '='))
+		{
+			size_t usernamelen = strcspn(expanding + i + 1, ":/ \t");
+			struct passwd *pwd;
+			
+			if (usernamelen)
+			{
+				char *username = null;
+
+				if (asprintf(&username, "%.*s", (int)usernamelen, expanding + i + 1) == -1)
+					fatalsys("failed to expand");
+
+				pwd = getpwnam(username);
+				free(username);
+			}
+			else
+			{
+				uid_t uid = g.uid ? g.uid : getuid();
+
+				pwd = getpwuid(uid);
+			}
+
+			if (pwd)
+			{
+				size_t homedirlen;
+
+				if (asprintf(&expanded, "%.*s%s%s", i, expanding, pwd->pw_dir, expanding + i + 1 + usernamelen) == -1)
+					fatalsys("failed to expand");
+
+				free(expanding);
+				expanding = expanded;
+				expanded = null;
+
+				homedirlen = strlen(pwd->pw_dir);
+				len -= 1 + usernamelen;
+				len += homedirlen;
+				i += homedirlen -  1;
+			}
+		}
+	}
+
 	/* But not for root, unless --idiot */
 
 	if (!g.idiot && (getuid() == 0 || geteuid() == 0))
@@ -1359,51 +1404,6 @@
 		}
 	}
 
-	/* Replace home directory notation: ~ or ~username */
-
-	for (i = 0; i < len; ++i)
-	{
-		if (expanding[i] == '~' && (i == 0 || is_space(expanding[i - 1]) || expanding[i - 1] == ':' || expanding[i - 1] == '='))
-		{
-			size_t usernamelen = strcspn(expanding + i + 1, ":/ \t");
-			struct passwd *pwd;
-			
-			if (usernamelen)
-			{
-				char *username = null;
-
-				if (asprintf(&username, "%.*s", (int)usernamelen, expanding + i + 1) == -1)
-					fatalsys("failed to expand");
-
-				pwd = getpwnam(username);
-				free(username);
-			}
-			else
-			{
-				uid_t uid = g.uid ? g.uid : getuid();
-
-				pwd = getpwuid(uid);
-			}
-
-			if (pwd)
-			{
-				size_t homedirlen;
-
-				if (asprintf(&expanded, "%.*s%s%s", i, expanding, pwd->pw_dir, expanding + i + 1 + usernamelen) == -1)
-					fatalsys("failed to expand");
-
-				free(expanding);
-				expanding = expanded;
-				expanded = null;
-
-				homedirlen = strlen(pwd->pw_dir);
-				len -= 1 + usernamelen;
-				len += homedirlen;
-				i += homedirlen -  1;
-			}
-		}
-	}
-
 	return expanding;
 }

As difference of Debian, the Slackware has a fully functional root account, so people may be interested to run your daemon as root for particular "user target" services.

And I think that talking with password/shadow library is quite safe, right?

So, a minimal expanding (only for home tilde) would be quite safe also for root, ight?

OR, at least please leave to root the ability to expand its own home tilde, as looks like we need to use just a config file like:

/etc/daemon.conf.d/pipewire.conf

pipewire bind,respawn,pidfiles=~/.daemon

for a XDG autostart file like

[Desktop Entry]
Version=1.0
Name=PipeWire Media System
Comment=Start the PipeWire Media System
Exec=/usr/bin/daemon -f -n pipewire /usr/bin/pipewire
Terminal=false
Type=Application
X-GNOME-Autostart-Phase=Initialization
X-KDE-autostart-phase=1

With this setup the daemon supervisor will work equally for any user, be it the root or an unprivileged user.

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 2, 2021

There's a patch which is more appropriate to your original idea, as it permit for root only the expansion of a simple tilde (its own home)

diff -urN daemon-20210303_92768de.orig/daemon.c daemon-20210303_92768de/daemon.c
--- daemon-20210303_92768de.orig/daemon.c	2021-03-02 17:16:43.000000000 +0200
+++ daemon-20210303_92768de/daemon.c	2021-03-02 19:30:59.481215928 +0200
@@ -1300,23 +1300,72 @@
 {
 	char *expanding = null;
 	char *expanded = null;
-	int i, len;
+	int i, len, minimal;
 
 	/* Check arguments */
 
 	if (!input)
 		return set_errnull(EINVAL);
 
-	/* Replace simple environment variables: $VARNAME or ${VARNAME} */
+	/* Prepare the variables. */
 
 	expanding = mem_strdup(input);
 	len = strlen(expanding);
 
-	/* But not for root, unless --idiot */
+	/* The root is allowed only for a minimal variables expansion. */
+
+	minimal = !g.idiot && (getuid() == 0 || geteuid() == 0);
+    
+	/* Replace home directory notation: ~ or ~username */
+
+	for (i = 0; i < len; ++i)
+	{
+		if (expanding[i] == '~' && (i == 0 || is_space(expanding[i - 1]) || expanding[i - 1] == ':' || expanding[i - 1] == '='))
+		{
+			size_t usernamelen = strcspn(expanding + i + 1, ":/ \t");
+			struct passwd *pwd;
+			
+			if (usernamelen == 0)
+			{   
+				uid_t uid = g.uid ? g.uid : getuid();
+
+				pwd = getpwuid(uid);
+			}
+			else if (! minimal)
+			{
+				char *username = null;
+
+				if (asprintf(&username, "%.*s", (int)usernamelen, expanding + i + 1) == -1)
+					fatalsys("failed to expand");
+
+				pwd = getpwnam(username);
+				free(username);
+			}
+
+			if (pwd)
+			{
+				size_t homedirlen;
+
+				if (asprintf(&expanded, "%.*s%s%s", i, expanding, pwd->pw_dir, expanding + i + 1 + usernamelen) == -1)
+					fatalsys("failed to expand");
+
+				free(expanding);
+				expanding = expanded;
+				expanded = null;
+
+				homedirlen = strlen(pwd->pw_dir);
+				len -= 1 + usernamelen;
+				len += homedirlen;
+				i += homedirlen -  1;
+			}
+		}
+	}
 
-	if (!g.idiot && (getuid() == 0 || geteuid() == 0))
+	if (minimal)
 		return expanding;
 
+	/* Replace simple environment variables: $VARNAME or ${VARNAME} */
+
 	for (i = 0; i < len; ++i)
 	{
 		if (expanding[i] == '$')
@@ -1359,51 +1408,6 @@
 		}
 	}
 
-	/* Replace home directory notation: ~ or ~username */
-
-	for (i = 0; i < len; ++i)
-	{
-		if (expanding[i] == '~' && (i == 0 || is_space(expanding[i - 1]) || expanding[i - 1] == ':' || expanding[i - 1] == '='))
-		{
-			size_t usernamelen = strcspn(expanding + i + 1, ":/ \t");
-			struct passwd *pwd;
-			
-			if (usernamelen)
-			{
-				char *username = null;
-
-				if (asprintf(&username, "%.*s", (int)usernamelen, expanding + i + 1) == -1)
-					fatalsys("failed to expand");
-
-				pwd = getpwnam(username);
-				free(username);
-			}
-			else
-			{
-				uid_t uid = g.uid ? g.uid : getuid();
-
-				pwd = getpwuid(uid);
-			}
-
-			if (pwd)
-			{
-				size_t homedirlen;
-
-				if (asprintf(&expanded, "%.*s%s%s", i, expanding, pwd->pw_dir, expanding + i + 1 + usernamelen) == -1)
-					fatalsys("failed to expand");
-
-				free(expanding);
-				expanding = expanded;
-				expanded = null;
-
-				homedirlen = strlen(pwd->pw_dir);
-				len -= 1 + usernamelen;
-				len += homedirlen;
-				i += homedirlen -  1;
-			}
-		}
-	}
-
 	return expanding;
 }

@raforg
Copy link
Owner

raforg commented Mar 2, 2021

I think I can release the new version tomorrow.

Thanks for the info about /etc/slackware-version. That's enough to configure for Slackware automatically.

I don't really want to include building a Slackware package in the makefiles. The existing binary package-building parts of the makefiles are very old and haven't been used in a decade. I'm sure many of them are out of date. I think it's best if people with expertise in the packaging system of each platform handle that. I can't keep up with it all.

"make install-daemon-conf" is separate because it's optional. Daemon will work without the config files. But more importantly, installing config files is complicated if there is already one there. Different platforms handle it differently. This way, the person installing (or building a package) has to decide how to handle it, and I won't get blamed for destroying someone's existing config file, or trying to handle it myself in a way that doesn't fit in with the platform's usual way of doing things.

Thanks so much for spotting the bug in expand(). That's fantastic. But your solution isn't quite right. The environment variables should be expanded before the user home directory notation (not after), so that environment variables can contain user home directory notation (just in case). And it's OK for root to expand another user's home directory, since the information is coming from /etc/passwd, which is trusted, and not from the process environment, which isn't. I didn't intend to prevent root from expanding home directories. It was just a mistake. It's fixed now. Thanks again.

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 3, 2021

Thanks again for your hard work! I am very happy to see the see the new release.

BTW, as a last minute suggestion: that man page part about the interaction of systemd's configuration of KillUserProcesses=no applies also well for elogind as I've used this "kill mode" activation on elogind configuration to quit the PipeWire daemon on user logout.

So, also on elogind is this option, which probably have same effects like in systemd.

@raforg
Copy link
Owner

raforg commented Mar 3, 2021

Does elogind kill all user processes on logout if KillUserProcesses=yes like systemd does? And is the KillUserProcesses setting in the same configuration file (i.e. /etc/systemd/logind.conf)?

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 3, 2021

Does elogind kill all user processes on logout if KillUserProcesses=yes like systemd does? And is the KillUserProcesses setting in the same configuration file (i.e. /etc/systemd/logind.conf)?

Yes, it does just like systemd, the single difference I seen is that the config file is differently named: /etc/elogind/logind.conf

That KillUserProcesses=yes is the kill mode I talked about.

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 3, 2021

BTW, please take a look at this post:

https://www.linuxquestions.org/questions/slackware-14/daemon-a-small-and-powerful-daemon-supervisor-with-e-logind-support-for-babysitting-our-pipewire%27s-user-target-daemons-testing-wanted-4175691414/page2.html#post6226689

Looks like a forum member got something like bellow after using a chained execution on client (PipeWire)

$ ps aux | grep pipewire
bob      23364  0.0  0.0   4112  2680 ?        S    20:06   0:00 /usr/bin/daemon -f -n pipewire /usr/bin/pipewire
bob      23369  0.0  0.1 101600  9560 ?        S<l  20:06   0:00 pipewire: /usr/bin/pipewire
bob      23374  0.1  0.2 147404 16756 ?        S<l  20:06   0:00 /usr/bin/pipewire-media-session bluez5
bob      23375  0.0  0.0  81468  5472 ?        S<l  20:06   0:00 /usr/bin/pipewire-pulse
bob      23678  0.0  0.0   3996  2152 pts/1    S+   20:07   0:00 grep pipewire

Then, after logout:

bob      23364  0.0  0.0   4112  2680 ?        S    20:06   0:00 /usr/bin/daemon -f -n pipewire /usr/bin/pipewire
bob      23369  0.0  0.0      0     0 ?        Z<   20:06   0:00 [pipewire] <defunct>
bob      23375  0.0  0.0  81468  5472 ?        S<l  20:06   0:00 /usr/bin/pipewire-pulse
bob      23765  0.0  0.0   4004  2128 tty1     S+   20:08   0:00 grep pipewire

This pipewire-pulse is a PulseAudio compat audio server, which was started by pipewire itself, just like pipewire-media-session.

So, we have a wrong daemon usage or a bug? Or we found a bug on PipeWire?

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 3, 2021

So, we have a wrong daemon usage or a bug? Or we found a bug on PipeWire?

Looks that was just a false alarm!

Apparently was just a wrong setup and we find the proper way to run the pipewire-pulse.

Separately, by a second instance of daemon

@raforg
Copy link
Owner

raforg commented Mar 3, 2021

Oh, dear. Zombies are never fun. It's very tempting to blame someone else's software when things go wrong like this, but I'm never confident that that's a valid response. Without being able to replicate and investigate the problem, it's hard to know the real cause. But I think I can make a good guess.

A zombie process happens when a process terminates, and its parent process doesn't wait() for it, so as to obtain its termination status. Pipewire has terminated, but daemon hasn't wait()ed for its status yet.

Daemon wait()s for its client's termination status after it has both received the SIGCHLD signal to let it know that a child process and died, and after that child process's stdout file descriptor has closed (daemon wants to make sure that it reads any final output from the client).

But it's possible for the client's stdout file descriptor to stay open after it terminates: i.e. when the client has started its own child process (e.g. pipewire-pulse) which shares its parent's process's stdout, and that other process is still running, and is still keeping stdout open.

Daemon is effectively waiting for pipewire-pulse to terminate as well. That will close the shared stdout. But pipewire hasn't terminated pipewire-pulse. It probably assumes that whatever sent it a SIGTERM will also send a SIGTERM to pipewire-pulse. But daemon won't do that because it doesn't know anything about pipewire-pulse. Daemon assumes that the client process will handle the termination of its own child processes when it receives a SIGTERM signal.

So, it looks like this happened:

  • daemon started pipewire
  • pipewire started pipewire-pulse
  • the user logged out
  • daemon sent pipewire SIGTERM
  • pipewire didn't propagate SIGTERM to pipewire-pulse to terminate it
  • pipewire-pulse and pipewire are sharing the same stdout which stayed open
  • daemon is still waiting for the pipewire's stdout to close before it finishes up and wait()s for the status
  • so pipewire-pulse stays around as a zombie

If pipewire and pipewire-pulse do share the same stdout file descriptor, that would prevent pipewire's stdout from closing (from daemon's point of view), and this would result. There are several approaches to fixing this:

  1. pipewire, upon receiving SIGTERM, could send SIGTERM to pipewire-pulse
  2. daemon could be started with --ignore-eof to not wait for eof on the client's stdout
  3. pipewire-pulse could be started by daemon --bind rather than by pipewire (as suggested by 0XBF/LuckyCyborg)

Option 1 requires getting pipewire changed.

Option 2 should prevent pipewire becoming a zombie, and cause it and the daemon process to disappear from the process list, but it won't do anything to terminate pipewire-pulse. That's still running.

Option 3 is probably the quickest and easiest approach that should get both the pipewire and pipewire-pulse processes terminated cleanly.

Please ask 0XBF to try option 3 and let me know if it works.

@raforg
Copy link
Owner

raforg commented Mar 3, 2021

Good. I thought getting a separate daemon to run pipewire-pulse would fix it.

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 3, 2021

Please, you can show me a daemon command which executes a given script in background and logs the output to syslog in a separate file, e.g. /var/log/daemontest.log ?

I want to show that on forum as demo of daemon abilities, with a script like:

#!/bin/sh

while true; do
  date
  sleep 10
done

@ZhaoLin1457
Copy link
Author

Good. I thought getting a separate daemon to run pipewire-pulse would fix it.

Yes, looks like this is the proper solution.

@raforg
Copy link
Owner

raforg commented Mar 3, 2021

The --output (or -o) option captures both the client's stdout and stderr to a file or syslog:

log=/var/log/daemontest.log
daemon --output="$log" -- cmd

There are also --stdout (-O) and --stderr (-E) options to separate stdout and stderr.

The --errlog (or -l) and --dbglog (or -b) options capture any error or debug output from daemon itself:

daemon --output="$log" --errlog="$log" --dbglog="$log" -- cmd

If you specify an actual filepath, it bypasses syslog and writes directly to the file.

To use syslog, specify a syslog facility and priority (e.g. "local0.err") instead of a file path.

@ZhaoLin1457
Copy link
Author

Thanks you!

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 4, 2021

I can have a suggestion?

Instead of --disable-logind, how about to have --enable-logind ?

I believe that those who wants this feature (like us) could activate it, while the others will got the "standard" daemon supervisor.

@raforg
Copy link
Owner

raforg commented Mar 4, 2021

I think it's easier the way it is, because if libsystemd or libelogind are present (with header files), that's enough to include the functionality. Someone doesn't have to make sure that one of those libraries is installed, and also say that they want it used. Having it installed should make it obvious that they want it used.

The only reason I added --disable-logind was so that I had an easy way to disable it before committing changes to the code, so the checked in code didn't have it by default.

But having said that, if --enable-logind existed and wasn't the default (when possible), then I wouldn't have to always remember to --disable-logind before committing code. So I'll change it.

By the way, the change earlier involving $(DESTDIR) was no good. It didn't interoperate with certain uses of configure --prefix. But it should be OK now. I stopped treating $(DESTDIR) as a prefix to $(PREFIX), and now just use $(DESTDIR) during install and uninstall only.

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 4, 2021

I may make another little suggestion?

There's a way to setup the CFLAGS or a similar flag to configure?

Slackware comes in different flavors, and bellow is a valid logic on our build scripts:

if [ "$ARCH" = "i386" ]; then
  SLKCFLAGS="-O2 -march=i386 -mcpu=i686"
elif [ "$ARCH" = "i486" ]; then
  SLKCFLAGS="-O2 -march=i486 -mtune=i686"
elif [ "$ARCH" = "i586" ]; then
  SLKCFLAGS="-O2 -march=i586 -mtune=i686"
elif [ "$ARCH" = "i686" ]; then
  SLKCFLAGS="-O2 -march=i686"
elif [ "$ARCH" = "s390" ]; then
  SLKCFLAGS="-O2"
elif [ "$ARCH" = "x86_64" ]; then
  SLKCFLAGS="-O2 -fPIC"
elif [ "$ARCH" = "armv7hl" ]; then
  SLKCFLAGS="-O3 -march=armv7-a -mfpu=vfpv3-d16"
else
  SLKCFLAGS="-O2"
fi

Usually, the variable SLKCFLAGS is expected to be used like this:

CFLAGS="$SLKCFLAGS" \
./configure \
  --prefix=/usr \
  --enable-logind

BTW, looks like the Slackware configure part fails to set properly the man dir to /usr/man

@raforg
Copy link
Owner

raforg commented Mar 4, 2021

Currently, you need to pass CPPFLAGS, CFLAGS and LDFLAGS to make directly, not configure.
The configure script only handles CC itself.

@ZhaoLin1457
Copy link
Author

I understand. Thanks!

@ZhaoLin1457
Copy link
Author

ZhaoLin1457 commented Mar 4, 2021

This what does your configure right now:

MAN_SYSDIR := $(PREFIX)/share/man <--------------
MAN_LOCDIR := $(PREFIX)/man

So, the man page is installed on /usr/share/man instead of /usr/man which is where it should be on Slackware.

@raforg
Copy link
Owner

raforg commented Mar 4, 2021

I forgot that MAN_LOCDIR is only used when PREFIX != /usr.

MAN_SYSDIR is used when PREFIX is /usr.

The slackware config changes MAN_SYSDIR as well now.

And it changes PREFIX=/usr so you don't need --prefix=/usr.

Try again with the new commit.

@ZhaoLin1457
Copy link
Author

Thanks! 👍

@ZhaoLin1457
Copy link
Author

Just a little cosmetic change on configure, if you are kind:

echo "Configuring for slackware"

to be

echo "Configuring for Slackware"

Thanks!

@raforg
Copy link
Owner

raforg commented Mar 4, 2021

Sure. I've just released daemon-0.8. Thanks for all the suggestions.

@ZhaoLin1457
Copy link
Author

Thank you for all your hard work!

👏 👏 👏 👏 👏 👏 👏 👏 👏 👏 👏 👏 👏 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants