Doing this means we get more applications into their own cgroup/systemd unit. This is important for systemd-oomd to work properly, as it tends to work on a cgroup level.
390 lines
14 KiB
Diff
390 lines
14 KiB
Diff
From 1e4e58a73f12cded2d67f58571ea42906f3c3f94 Mon Sep 17 00:00:00 2001
|
|
From: Benjamin Berg <bberg@redhat.com>
|
|
Date: Mon, 27 Jul 2020 22:22:32 +0200
|
|
Subject: [PATCH 2/4] gdesktopappinfo: Move launched applications into
|
|
transient scope
|
|
|
|
Try to move the spawned executable into its own systemd scope. To avoid
|
|
possible race conditions and ensure proper accounting, we delay the
|
|
execution of the real command until after the DBus call to systemd has
|
|
finished.
|
|
|
|
From the two approaches we can take here, this is better in the sense
|
|
that we have a child that the API consumer can watch. API consumers
|
|
should not be doing this, however, gnome-session needs to watch children
|
|
during session startup. Until gnome-session is fixed, we will not be
|
|
able to change this.
|
|
|
|
The alternative approach is to delegate launching itself to systemd by
|
|
creating a transient .service unit instead. This is cleaner and has e.g.
|
|
the advantage that systemd will take care of log redirection and similar
|
|
issues.
|
|
|
|
Note that this patch is incomplete. The DBus call is done in a "fire and
|
|
forget" manner, which is fine in most cases, but means that "gio open"
|
|
will fail to move the child into the new scope as gio quits before the
|
|
DBus call finishes.
|
|
---
|
|
gio/gdesktopappinfo.c | 263 ++++++++++++++++++++++++++++++++++++------
|
|
1 file changed, 228 insertions(+), 35 deletions(-)
|
|
|
|
diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c
|
|
index 1a4b97918..2f1e6828b 100644
|
|
--- a/gio/gdesktopappinfo.c
|
|
+++ b/gio/gdesktopappinfo.c
|
|
@@ -2730,6 +2730,148 @@ notify_desktop_launch (GDBusConnection *session_bus,
|
|
|
|
#define _SPAWN_FLAGS_DEFAULT (G_SPAWN_SEARCH_PATH)
|
|
|
|
+#if defined(__linux__) && !defined(__BIONIC__)
|
|
+typedef struct {
|
|
+ int pipe[2];
|
|
+ GSpawnChildSetupFunc user_setup;
|
|
+ gpointer user_setup_data;
|
|
+} SpawnWrapperData;
|
|
+
|
|
+static void
|
|
+launch_uris_with_spawn_delay_exec (gpointer user_data)
|
|
+{
|
|
+ SpawnWrapperData *data = user_data;
|
|
+
|
|
+ /* Clear CLOEXEC again, as that was set due to
|
|
+ * G_SPAWN_LEAVE_DESCRIPTORS_OPEN not being set. */
|
|
+ fcntl (data->pipe[0], F_SETFD, 0);
|
|
+
|
|
+ /* No need to close read side, we have CLOEXEC set. */
|
|
+
|
|
+ if (data->user_setup)
|
|
+ data->user_setup (data->user_setup_data);
|
|
+}
|
|
+
|
|
+static gchar *
|
|
+systemd_unit_name_escape (const gchar *in)
|
|
+{
|
|
+ /* Adapted from systemd source */
|
|
+ GString * const str = g_string_sized_new (strlen (in));
|
|
+
|
|
+ for (; *in; in++)
|
|
+ {
|
|
+ if (g_ascii_isalnum (*in) || *in == ':' || *in == '_' || *in == '.')
|
|
+ g_string_append_c (str, *in);
|
|
+ else
|
|
+ g_string_append_printf (str, "\\x%02x", *in);
|
|
+ }
|
|
+ return g_string_free (str, FALSE);
|
|
+}
|
|
+
|
|
+static void
|
|
+create_systemd_scope (GDBusConnection *session_bus,
|
|
+ GDesktopAppInfo *info,
|
|
+ gint pid,
|
|
+ GAsyncReadyCallback callback,
|
|
+ gpointer user_data)
|
|
+{
|
|
+ GVariantBuilder builder;
|
|
+ const char *app_name = g_get_application_name ();
|
|
+ char *appid = NULL;
|
|
+ char *appid_escaped = NULL;
|
|
+ char *snid_escaped = NULL;
|
|
+ char *unit_name = NULL;
|
|
+
|
|
+ /* In this order:
|
|
+ * 1. Actual application ID from file
|
|
+ * 2. Stripping the .desktop from the desktop ID
|
|
+ * 3. Fall back to using the binary name
|
|
+ */
|
|
+ if (info->app_id)
|
|
+ appid = g_strdup (info->app_id);
|
|
+ else if (info->desktop_id && g_str_has_suffix (info->desktop_id, ".desktop"))
|
|
+ appid = g_strndup (info->desktop_id, strlen (info->desktop_id) - 8);
|
|
+ else
|
|
+ appid = g_path_get_basename (info->binary);
|
|
+
|
|
+ appid_escaped = systemd_unit_name_escape (appid);
|
|
+
|
|
+ /* Generate a name conforming to
|
|
+ * https://systemd.io/DESKTOP_ENVIRONMENTS/
|
|
+ * We use the PID to disambiguate, as that should be unique enough.
|
|
+ */
|
|
+ unit_name = g_strdup_printf ("app-glib-%s-%d.scope", appid_escaped, pid);
|
|
+
|
|
+ g_variant_builder_init (&builder, G_VARIANT_TYPE ("(ssa(sv)a(sa(sv)))"));
|
|
+ g_variant_builder_add (&builder, "s", unit_name);
|
|
+ g_variant_builder_add (&builder, "s", "fail");
|
|
+
|
|
+ g_variant_builder_open (&builder, G_VARIANT_TYPE ("a(sv)"));
|
|
+
|
|
+ /* Add a generic human readable description, can be changed at will. */
|
|
+ if (app_name)
|
|
+ g_variant_builder_add (&builder,
|
|
+ "(sv)",
|
|
+ "Description",
|
|
+ g_variant_new_take_string (g_strdup_printf ("Application launched by %s",
|
|
+ app_name)));
|
|
+ g_variant_builder_add (&builder,
|
|
+ "(sv)",
|
|
+ "PIDs",
|
|
+ g_variant_new_fixed_array (G_VARIANT_TYPE_UINT32, &pid, 1, 4));
|
|
+ /* Default to let systemd garbage collect failed applications we launched. */
|
|
+ g_variant_builder_add (&builder,
|
|
+ "(sv)",
|
|
+ "CollectMode",
|
|
+ g_variant_new_string ("inactive-or-failed"));
|
|
+
|
|
+ g_variant_builder_close (&builder);
|
|
+
|
|
+ g_variant_builder_open (&builder, G_VARIANT_TYPE ("a(sa(sv))"));
|
|
+ g_variant_builder_close (&builder);
|
|
+
|
|
+ g_dbus_connection_call (session_bus,
|
|
+ "org.freedesktop.systemd1",
|
|
+ "/org/freedesktop/systemd1",
|
|
+ "org.freedesktop.systemd1.Manager",
|
|
+ "StartTransientUnit",
|
|
+ g_variant_builder_end (&builder),
|
|
+ G_VARIANT_TYPE ("(o)"),
|
|
+ G_DBUS_CALL_FLAGS_NO_AUTO_START,
|
|
+ 1000,
|
|
+ NULL,
|
|
+ callback,
|
|
+ user_data);
|
|
+
|
|
+ g_free (appid);
|
|
+ g_free (appid_escaped);
|
|
+ g_free (snid_escaped);
|
|
+ g_free (unit_name);
|
|
+}
|
|
+
|
|
+static void
|
|
+systemd_scope_created_cb (GObject *object,
|
|
+ GAsyncResult *result,
|
|
+ gpointer user_data)
|
|
+{
|
|
+ GVariant *res = NULL;
|
|
+ GError *error = NULL;
|
|
+
|
|
+ res = g_dbus_connection_call_finish (G_DBUS_CONNECTION (object), result, &error);
|
|
+ if (error != NULL)
|
|
+ {
|
|
+ g_debug ("Failed to move new child into scope: %s", error->message);
|
|
+ g_error_free (error);
|
|
+ }
|
|
+
|
|
+ /* Unblock the waiting wrapper binary. */
|
|
+ close (GPOINTER_TO_INT (user_data));
|
|
+
|
|
+ if (res)
|
|
+ g_variant_unref (res);
|
|
+}
|
|
+#endif
|
|
+
|
|
static gboolean
|
|
g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo *info,
|
|
GDBusConnection *session_bus,
|
|
@@ -2750,13 +2892,14 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo *info,
|
|
GList *old_uris;
|
|
GList *dup_uris;
|
|
|
|
- char **argv, **envp;
|
|
+ GStrv argv = NULL, envp = NULL;
|
|
+ GStrv wrapped_argv = NULL;
|
|
+ GList *launched_uris = NULL;
|
|
+ char *sn_id = NULL;
|
|
int argc;
|
|
|
|
g_return_val_if_fail (info != NULL, FALSE);
|
|
|
|
- argv = NULL;
|
|
-
|
|
if (launch_context)
|
|
envp = g_app_launch_context_get_environment (launch_context);
|
|
else
|
|
@@ -2770,27 +2913,19 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo *info,
|
|
do
|
|
{
|
|
GPid pid;
|
|
- GList *launched_uris;
|
|
GList *iter;
|
|
- char *sn_id = NULL;
|
|
- char **wrapped_argv;
|
|
int i;
|
|
- gsize j;
|
|
- const gchar * const wrapper_argv[] =
|
|
- {
|
|
- "/bin/sh",
|
|
- "-e",
|
|
- "-u",
|
|
- "-c", "export GIO_LAUNCHED_DESKTOP_FILE_PID=$$; exec \"$@\"",
|
|
- "sh", /* argv[0] for sh */
|
|
- };
|
|
+#if defined(__linux__) && !defined(__BIONIC__)
|
|
+ SpawnWrapperData wrapper_data;
|
|
+#endif
|
|
+ GSpawnChildSetupFunc setup = user_setup;
|
|
+ gpointer setup_data = user_setup_data;
|
|
|
|
old_uris = dup_uris;
|
|
if (!expand_application_parameters (info, exec_line, &dup_uris, &argc, &argv, error))
|
|
- goto out;
|
|
+ return FALSE;
|
|
|
|
/* Get the subset of URIs we're launching with this process */
|
|
- launched_uris = NULL;
|
|
for (iter = old_uris; iter != NULL && iter != dup_uris; iter = iter->next)
|
|
launched_uris = g_list_prepend (launched_uris, iter->data);
|
|
launched_uris = g_list_reverse (launched_uris);
|
|
@@ -2799,7 +2934,7 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo *info,
|
|
{
|
|
g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
|
|
_("Unable to find terminal required for application"));
|
|
- goto out;
|
|
+ return FALSE;
|
|
}
|
|
|
|
if (info->filename)
|
|
@@ -2808,7 +2943,6 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo *info,
|
|
info->filename,
|
|
TRUE);
|
|
|
|
- sn_id = NULL;
|
|
if (launch_context)
|
|
{
|
|
GList *launched_files = create_files_for_uris (launched_uris);
|
|
@@ -2837,38 +2971,96 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo *info,
|
|
* with a wrapper program (grep the GLib git history for
|
|
* `gio-launch-desktop` for an example of this which could be
|
|
* resurrected). */
|
|
- wrapped_argv = g_new (char *, argc + G_N_ELEMENTS (wrapper_argv) + 1);
|
|
+ wrapped_argv = g_new (char *, argc + 6 + 1);
|
|
+
|
|
+ wrapped_argv[0] = g_strdup ("/bin/sh");
|
|
+ wrapped_argv[1] = g_strdup ("-e");
|
|
+ wrapped_argv[2] = g_strdup ("-u");
|
|
+ wrapped_argv[3] = g_strdup ("-c");
|
|
+ /* argument 4 is filled in below */
|
|
+ wrapped_argv[5] = g_strdup ("sh");
|
|
|
|
- for (j = 0; j < G_N_ELEMENTS (wrapper_argv); j++)
|
|
- wrapped_argv[j] = g_strdup (wrapper_argv[j]);
|
|
for (i = 0; i < argc; i++)
|
|
- wrapped_argv[i + G_N_ELEMENTS (wrapper_argv)] = g_steal_pointer (&argv[i]);
|
|
+ wrapped_argv[i + 6] = g_steal_pointer (&argv[i]);
|
|
|
|
- wrapped_argv[i + G_N_ELEMENTS (wrapper_argv)] = NULL;
|
|
- g_free (argv);
|
|
- argv = NULL;
|
|
+ wrapped_argv[i + 6] = NULL;
|
|
+ g_clear_pointer (&argv, g_free);
|
|
+
|
|
+#if defined(__linux__) && !defined(__BIONIC__)
|
|
+ /* Create pipes, if we use a setup func, then set cloexec,
|
|
+ * otherwise our wrapper script will close both sides. */
|
|
+ if (!g_unix_open_pipe (wrapper_data.pipe, 0, NULL))
|
|
+ {
|
|
+ g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
|
|
+ _("Unable to create pipe for systemd synchronization"));
|
|
+ return FALSE;
|
|
+ }
|
|
+
|
|
+ /* Set CLOEXEC on the write pipe, so we don't need to deal with it in the child. */
|
|
+ fcntl (wrapper_data.pipe[1], F_SETFD, FD_CLOEXEC);
|
|
+
|
|
+ if (!(spawn_flags & G_SPAWN_LEAVE_DESCRIPTORS_OPEN))
|
|
+ {
|
|
+ /* In this case, we use a setup function (which could probably also
|
|
+ * overwrite envp to set GIO_LAUNCHED_DESKTOP_FILE_PID).
|
|
+ *
|
|
+ * Note that this does not incur an additional cost because
|
|
+ * G_SPAWN_LEAVE_DESCRIPTOR_OPEN must be set in order to use
|
|
+ * posix_spawn. */
|
|
+ wrapper_data.user_setup = setup;
|
|
+ wrapper_data.user_setup_data = setup_data;
|
|
+
|
|
+ setup = launch_uris_with_spawn_delay_exec;
|
|
+ setup_data = &wrapper_data;
|
|
+ }
|
|
+
|
|
+ wrapped_argv[4] = g_strdup_printf ("export GIO_LAUNCHED_DESKTOP_FILE_PID=$$; cat <&%1$d; exec \"$@\" %1$d<&-",
|
|
+ wrapper_data.pipe[0]);
|
|
+#else
|
|
+ wrapped_argv[4] = g_strdup ("export GIO_LAUNCHED_DESKTOP_FILE_PID=$$; exec \"$@\"");
|
|
+#endif
|
|
|
|
if (!g_spawn_async_with_fds (info->path,
|
|
wrapped_argv,
|
|
envp,
|
|
spawn_flags,
|
|
- user_setup,
|
|
- user_setup_data,
|
|
+ setup,
|
|
+ setup_data,
|
|
&pid,
|
|
stdin_fd,
|
|
stdout_fd,
|
|
stderr_fd,
|
|
error))
|
|
{
|
|
+#if defined(__linux__) && !defined(__BIONIC__)
|
|
+ close (wrapper_data.pipe[0]);
|
|
+ close (wrapper_data.pipe[1]);
|
|
+#endif
|
|
+
|
|
if (sn_id)
|
|
g_app_launch_context_launch_failed (launch_context, sn_id);
|
|
|
|
g_free (sn_id);
|
|
g_list_free (launched_uris);
|
|
|
|
- goto out;
|
|
+ goto err;
|
|
}
|
|
|
|
+#if defined(__linux__) && !defined(__BIONIC__)
|
|
+ /* We close write side asynchronously later on when the dbus call
|
|
+ * to systemd finishes. */
|
|
+ close (wrapper_data.pipe[0]);
|
|
+
|
|
+ if (session_bus)
|
|
+ create_systemd_scope (session_bus,
|
|
+ info,
|
|
+ pid,
|
|
+ systemd_scope_created_cb,
|
|
+ GINT_TO_POINTER (wrapper_data.pipe[1]));
|
|
+ else
|
|
+ close (wrapper_data.pipe[1]);
|
|
+#endif
|
|
+
|
|
if (pid_callback != NULL)
|
|
pid_callback (info, pid, pid_callback_data);
|
|
|
|
@@ -2893,19 +3085,20 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo *info,
|
|
sn_id,
|
|
launched_uris);
|
|
|
|
- g_free (sn_id);
|
|
- g_list_free (launched_uris);
|
|
-
|
|
- g_strfreev (wrapped_argv);
|
|
- wrapped_argv = NULL;
|
|
+ g_clear_pointer (&sn_id, g_free);
|
|
+ g_clear_pointer (&launched_uris, g_list_free);
|
|
+ g_clear_pointer (&wrapped_argv, g_strfreev);
|
|
}
|
|
while (dup_uris != NULL);
|
|
|
|
completed = TRUE;
|
|
|
|
- out:
|
|
+out:
|
|
g_strfreev (argv);
|
|
g_strfreev (envp);
|
|
+ g_clear_pointer (&wrapped_argv, g_strfreev);
|
|
+ g_list_free (launched_uris);
|
|
+ g_free (sn_id);
|
|
|
|
return completed;
|
|
}
|
|
--
|
|
2.29.2
|
|
|