Session suspension and restoration protocol

Tue Jun 19 13:15:44 UTC 2018

On 2018/6月/19 01:22, Simon McVittie wrote:
> On Tue, 19 Jun 2018 at 11:18:17 +0200, Markus Ongyerth wrote:
> > On 2018/6月/18 05:05, Roman Gilg wrote:
> > > * using D-Bus interface only to secure against sandboxed clients
> > What? Why exactly? When I first read this, I expected that the client is 
> > supposed to use the portal stuff to call out of a sandbox, but reading through 
> > the actual protocol, I think the only usage of D-Bus is to launch the 
> > applications from the compositor?
> 
> Any app implementing DBusActivatable (see the Desktop Entry Specification
Why would we arbitrarily limit this mechanism to this subset of applications?
This is a wayland extension protocol, and unless there's a good reason to 
limit it further, this should be avoided
> for more details) needs to be able to receive method calls from desktop
> infrastructure outside its sandbox anyway.
> For instance, Flatpak always
> allows method calls into the sandbox, while restricting method calls
> out of the sandbox according to app-specific rules.
> 
> > Since this needs explicit client support either way, IMO this could be 
> > implmented by adding a required cmdline argument (e.g.
> > --org-freedsktop-restore=UID) instead.
> 
> In desktop environments that support it, it's advantageous to launch apps
> as D-Bus services, or even as systemd user services if they support
> being launched like that (dbus-daemon will automatically delegate
> activation to systemd --user where supported), so that they always run
> in a reasonably consistent execution environment. If various different
> components (compositor, main menu if it's separate from the compositor,
> random app that wants to open a web page, etc.) launch an app using
> fork-and-exec with the Exec= command line from its .desktop file, then
> the app will inherit its execution environment (environment variables,
> file descriptors, rlimits, other process flags) from an arbitrary parent
> process, depending on the precise details of how they were launched. If
> they're DBusActivatable, they'll all be children of systemd --user (if
> supported) or dbus-daemon --session (if not), making their execution
> environment a lot more predictable.
Most of the compositor env makes sense for applications. Pushing open fds 
(outside std fds) into the child might be a compositor bug, or may be 
intended. E.g. for preconfigured wl_clients.
Some others make sense to prevent, like the rlimits, i agree. But the 
compositor isn't some arbitrary client either.
> 
> This indirect launching also avoids doing a fork-and-exec in the
> compositor or session manager, which can be problematic under low-memory
> conditions: the kernel needs to allocate enough virtual memory space for
> a second copy of the parent process (even though the physical memory that
> backs it is going to be copy-on-write, and in fork-and-exec it only exists
> briefly and is mostly unmodified), and when a compositor has your GPU's
> address space mmapped, that's actually a significant practical problem.
> I think Endless have had trouble with this in GNOME Shell on 32-bit -
> there's easily enough address space to have the GPU mmapped once, but
> if GNOME Shell does a fork-and-exec, the child process briefly has the
> GPU mmapped for a second time, and there isn't always enough address
> space free to do that.
Yea, that's indeed a problem.
> 
> The DBusActivatable protocol doesn't support sending arbitrary
> command-line arguments to the launched app, because in general,
> command-line arguments are not meaningful for an app that is already
> running.
This is for restoration, and the activation is only used for exactly restoring 
the application after it's closed.
Sending this to an already running application is the weird case that only 
exists because of using dbus here.

> (It does support sending the URLs of files to be opened.)
> Assuming we want to avoid always having to fall back to fork-and-exec,
> which is undesirable for the reasons above, we'll need some way to
> pass the state to restore via D-Bus - either a new method call as Roman
> proposed here, or the platform_data argument to Activate() as I suggested.
> 
> It would be possible to have a D-Bus preferred path and a fork-and-exec
> fallback path that are defined to be equivalent, like the Desktop
> Entry Specification does now for ordinary launching and for "Desktop
> Actions". That would have to work via either a command-line argument or an
> environment variable, because in fork-and-exec world those are the only
> tools we have. However, the meanings (and even syntax) of command-line
> arguments are conceptually "owned" by the app author, not by some third
> party like freedesktop.org; and environment variables are undesirable
> here because they inherit to child processes.
This is opt in either way, it can't just be supported without a client knowing 
about this, so it's not like we'll get arbitrary collisions with existing 
flags.
And if it's suffiently specific (--org-freedesktop-restore) I don't think 
there's a huge problem with coliding either way.

Toolkits (and sometimes runtimes) eat arguments already. In more or less well 
confined contexts. It's not like this would be something novel that noeone did 
so far.

> 
> Having two code paths also means that a high-quality implementation has
> to test and support both (potentially forever), while in a lower-quality
> implementation the less-preferred code path will probably regress at
> some point, which is bad if your compositor or app is in the minority
> that is relying on it.
Let's drop the dbus one :)

> 
>     smcv
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180619/5f4a9ec7/attachment.sig>