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

Place xdg-desktop-portal-gtk.service in session.slice #504

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Atemu
Copy link

@Atemu Atemu commented Nov 11, 2024

By default, systemd would place it in app.slice which doesn't make sense; that's where apps go, not daemons that are part of the desktop session.

session.slice makes a lot more sense and it's where the other xdg and/or portal services are placed too.

cc @smcv @Vladimir-csp

By default, systemd would place it in `app.slice` which doesn't make sense;
that's where apps go, not daemons that are part of the desktop session.

`session.slice` makes a lot more sense and it's where the other xdg and/or
portal services are placed too.
@Atemu
Copy link
Author

Atemu commented Nov 11, 2024

For some more context, here's my current systemd user unit hierarchy:

$ $ systemctl --user status | rg '\.slice$|\.service$'
   CGroup: /user.slice/user-1000.slice/[email protected]
           ├─app.slice
           │ ├─app-rofi-easyeffects-19676.service
           │ ├─app-rofi-firefox\x20\x2d\x2dname\x20firefox-24101.service
           │ ├─app-rofi-signal\x2ddesktop\x20\x2d\x2dno\x2dsandbox-12745.service
           │ ├─dconf.service
           │ ├─dunst.service
           │ ├─emacs.service
           │ ├─emacsclient-26056.service
           │ ├─foot-12775.service
           │ ├─foot-24573.service
           │ ├─foot-2565.service
           │ ├─foot-29533.service
           │ └─xdg-desktop-portal-gtk.service
           ├─background.slice
           │ └─[email protected]
           └─session.slice
             ├─dbus.service
             ├─hypridle-power.service
             ├─hypridle.service
             ├─pipewire-pulse.service
             ├─pipewire.service
             ├─[email protected]
             ├─wireplumber.service
             ├─xdg-desktop-portal-hyprland.service
             ├─xdg-desktop-portal.service
             ├─xdg-document-portal.service
             └─xdg-permission-store.service

As you can clearly see, xdg-desktop-portal-gtk.service is quite out of place here.

@smcv
Copy link
Collaborator

smcv commented Nov 11, 2024

We need to be a bit careful about this: it isn't completely obvious that it's 100% correct to do this.

One possible unintended side-effect of doing this is that if x-d-p-gtk launches an application - for example a web browser like Firefox or Chromium to implement the OpenURI interface, or a viewer like Evince or Xpdf to implement the OpenFile interface - then the web browser or viewer could end up also being in session.slice, which we probably don't want.

Is there a functional reason (like perhaps resource management) why x-d-p-gtk needs to be in session.slice, or is your reasoning just "this looks wrong"?

@Vladimir-csp
Copy link
Contributor

I haven't even noticed that x-d-p-gtk goes to app.slice, all other portals are in session.slice. It supposedly gives more priority and responsivenes.

If it needs to launch an application, it probably should launch it in its own unit anyway, otherwise xdg-desktop-portal-gtk.service would just accumulate a bunch of unrelated child processes.

@swick
Copy link

swick commented Nov 11, 2024

Indeed, putting x-d-p-gtk into session.slice and also launching everything into their own cgroup in app.slice or both things we should do. Someone needs to put the work in and check all the places where we might launch something.

@smcv
Copy link
Collaborator

smcv commented Nov 11, 2024

launching everything into their own cgroup

I think most of that would need to happen in GLib, because x-d-p-gtk mostly just calls g_app_info_launch_uris() or g_app_info_launch() or similar. The only explicit subprocess launch that I can immediately see is the app chooser dialog running gnome-software.

The desktop-independent x-d-p probably has the same problem, because it also calls g_app_info_launch_uris().

Atemu added a commit to Atemu/nixos-config that referenced this pull request Nov 12, 2024
@Atemu
Copy link
Author

Atemu commented Nov 12, 2024

if x-d-p-gtk launches an application - for example a web browser like Firefox or Chromium to implement the OpenURI interface, or a viewer like Evince or Xpdf to implement the OpenFile interface - then the web browser or viewer could end up also being in session.slice, which we probably don't want.

How could I verify whether this actually happens?

@swick
Copy link

swick commented Nov 13, 2024

The only explicit subprocess launch that I can immediately see is the app chooser dialog running gnome-software.

Right, that needs fixing then but otherwise it sounds like we could move it to session.slice.

@swick
Copy link

swick commented Nov 13, 2024

Mh, I skimmed the implementations of g_app_info_launch and I don't see it doing anything regarding the cgroup it ends up in so I guess this needs some work in glib first...

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

Successfully merging this pull request may close these issues.

4 participants