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

RFC: better systemd --user integration #301

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

intelfx
Copy link

@intelfx intelfx commented Apr 19, 2024

Right now, processes running under systemd --user confuse needrestart in several ways:

  1. for processes that did not double-fork, needrestart considers systemd --user as the logical parent (which means that needrestart will bogusly report systemd itself as the process that needs to be restarted);
  2. when the systemd --user instance itself needs a restart, needrestart won't do anything about it;
  3. if the process to be restarted is a user service under systemd --user (as opposed to an arbitrary process started in a user scope), needrestart won't use this information in any way.

This PR offers a clean solution for (1) by disregarding the parent if it is a systemd --user instance (implemented via matching the cgroup of the parent against /init.scope$), and pretty hacky solutions for (2) and (3) (hence the RFC status).

intelfx added 5 commits April 25, 2024 04:22
Ignore $ppid if it is an instance of the systemd manager.
This prevents `systemd --user` from being treated as a parent process
of all non-double-forked `systemd --user` services.
Ideally, we'd do something more reasonable, like treating them similarly
to system services and maybe even offering to restart some of them or
at the very least printing them in a format suitable for copy-pasting,
but for now just replace the process name with the service name and
save them under a separate key to hopefully distinguish them from
other processes under `systemd --user`.
@Vladimir-csp
Copy link
Contributor

Can this be useful for #289?

@intelfx
Copy link
Author

intelfx commented May 29, 2024

@Vladimir-csp yes, I believe it can. This needs a bit more UI work (the code I wrote piggybacks on existing data structures used for raw session processes, so it does not offer to perform a systemctl --user restart), but in principle this code already performs detection of user services which is what you ask for.

@intelfx
Copy link
Author

intelfx commented Nov 20, 2024

@liske ping, is there any chance this PR could get reviewed? There has been a new release since this PR was posted, so I'm assuming there must be some sort of problem with this code specifically?

@liske liske added this to the v3.9 milestone Nov 20, 2024
@liske
Copy link
Owner

liske commented Nov 20, 2024

@liske ping, is there any chance this PR could get reviewed? There has been a new release since this PR was posted, so I'm assuming there must be some sort of problem with this code specifically?

Sorry the security fixes for 3.8 have taken some time over the last few weeks and I was not able to take time for this more complex issue, yet. Scheduling it for the next release.

@intelfx
Copy link
Author

intelfx commented Nov 20, 2024

Thanks. I just wanted to make sure this PR would actually be desired before I rebase it on top of the new changes.

@Vladimir-csp
Copy link
Contributor

Systemd-managed user sessions with apps in units were already a reality in Gnome and at least some other major DEs.

I helped a bit to make it happen in independent compositors also.

@liske
Copy link
Owner

liske commented Nov 24, 2024

I need to reschedule this for 3.10. Needrestart 3.9 will be release ASAP to fix the regression reported in #317.

@liske liske modified the milestones: v3.9, v3.10 Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants