From 48bf40b905d534ab003722778e0ad16b489e24a5 Mon Sep 17 00:00:00 2001

From: Ray Strode <rstrode@redhat.com>

Date: Mon, 6 Oct 2014 10:58:48 -0400

Subject: [PATCH 1/4] display: don't leak a reference to the display object

 from the display object

It means display objects never get finalized, and so the slave doesn't

get killed in an orderly way at shutdown.

---

 daemon/gdm-display.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/daemon/gdm-display.c b/daemon/gdm-display.c

index 5d06036..d16c094 100644

--- a/daemon/gdm-display.c

+++ b/daemon/gdm-display.c

@@ -286,61 +286,61 @@ gdm_display_add_user_authorization (GdmDisplay *display,

         g_object_unref (display);

         return ret;

 }

 static void

 on_name_vanished (GDBusConnection *connection,

                   const char      *name,

                   gpointer         user_data)

 {

         queue_finish (GDM_DISPLAY (user_data));

 }

 static gboolean

 gdm_display_real_set_slave_bus_name (GdmDisplay *display,

                                      const char *name,

                                      GError    **error)

 {

         g_free (display->priv->slave_bus_name);

         display->priv->slave_bus_name = g_strdup (name);

         if (display->priv->slave_name_id > 0) {

                 g_bus_unwatch_name (display->priv->slave_name_id);

         }

         display->priv->slave_name_id = g_bus_watch_name_on_connection (display->priv->connection,

                                                                        name,

                                                                        G_BUS_NAME_WATCHER_FLAGS_NONE,

                                                                        NULL, /* name appeared */

                                                                        on_name_vanished,

-                                                                       g_object_ref (display),

+                                                                       display,

                                                                        NULL);

         g_clear_object (&display->priv->slave_bus_proxy);

         display->priv->slave_bus_proxy = GDM_DBUS_SLAVE (gdm_dbus_slave_proxy_new_sync (display->priv->connection,

                                                                                         G_DBUS_PROXY_FLAGS_NONE,

                                                                                         name,

                                                                                         GDM_SLAVE_PATH,

                                                                                         NULL, NULL));

         g_object_bind_property (G_OBJECT (display->priv->slave_bus_proxy),

                                 "session-id",

                                 G_OBJECT (display),

                                 "session-id",

                                 G_BINDING_DEFAULT);

         return TRUE;

 }

 gboolean

 gdm_display_set_slave_bus_name (GdmDisplay *display,

                                 const char *name,

                                 GError    **error)

 {

         gboolean ret;

         g_return_val_if_fail (GDM_IS_DISPLAY (display), FALSE);

         g_debug ("GdmDisplay: Setting slave bus name:%s on display %s", name, display->priv->x11_display_name);

         g_object_ref (display);

         ret = GDM_DISPLAY_GET_CLASS (display)->set_slave_bus_name (display, name, error);

-- 

1.8.3.1

From 3cacba38e16686aa93ef47c6e0201938863aa6fb Mon Sep 17 00:00:00 2001

From: Ray Strode <rstrode@redhat.com>

Date: Mon, 6 Oct 2014 11:00:00 -0400

Subject: [PATCH 2/4] launch-environment: shutdown worker process as well as

 environment pgrp

The worker isn't part of the environment process group, so it needs to

be kill explicitly even if the environment group is already getting

killed.

---

 daemon/gdm-launch-environment.c | 11 +++++------

 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/daemon/gdm-launch-environment.c b/daemon/gdm-launch-environment.c

index 932cc06..551dc96 100644

--- a/daemon/gdm-launch-environment.c

+++ b/daemon/gdm-launch-environment.c

@@ -502,68 +502,67 @@ gdm_launch_environment_start (GdmLaunchEnvironment *launch_environment)

                           "session-died",

                           G_CALLBACK (on_session_died),

                           launch_environment);

         gdm_session_start_conversation (launch_environment->priv->session, "gdm-launch-environment");

         if (launch_environment->priv->dbus_session_bus_address) {

                 gdm_session_select_program (launch_environment->priv->session, launch_environment->priv->command);

         } else {

                 /* wrap it in dbus-launch */

                 char *command = g_strdup_printf ("%s %s", DBUS_LAUNCH_COMMAND, launch_environment->priv->command);

                 gdm_session_select_program (launch_environment->priv->session, command);

                 g_free (command);

         }

         res = TRUE;

  out:

         if (local_error) {

                 g_critical ("GdmLaunchEnvironment: %s", local_error->message);

                 g_clear_error (&local_error);

         }

         return res;

 }

 gboolean

 gdm_launch_environment_stop (GdmLaunchEnvironment *launch_environment)

 {

         if (launch_environment->priv->pid > 1) {

                 gdm_signal_pid (-launch_environment->priv->pid, SIGTERM);

-        } else {

-                if (launch_environment->priv->session != NULL) {

-                        gdm_session_stop_conversation (launch_environment->priv->session, "gdm-launch-environment");

-                        gdm_session_close (launch_environment->priv->session);

+        }

-                        g_clear_object (&launch_environment->priv->session);

-                }

+        if (launch_environment->priv->session != NULL) {

+                gdm_session_stop_conversation (launch_environment->priv->session, "gdm-launch-environment");

+                gdm_session_close (launch_environment->priv->session);

+                g_clear_object (&launch_environment->priv->session);

                 g_signal_emit (G_OBJECT (launch_environment), signals [STOPPED], 0);

         }

         return TRUE;

 }

 GdmSession *

 gdm_launch_environment_get_session (GdmLaunchEnvironment *launch_environment)

 {

         return launch_environment->priv->session;

 }

 char *

 gdm_launch_environment_get_session_id (GdmLaunchEnvironment *launch_environment)

 {

         return g_strdup (launch_environment->priv->session_id);

 }

 static void

 _gdm_launch_environment_set_verification_mode (GdmLaunchEnvironment           *launch_environment,

                                                GdmSessionVerificationMode      verification_mode)

 {

         launch_environment->priv->verification_mode = verification_mode;

 }

 static void

 _gdm_launch_environment_set_x11_display_name (GdmLaunchEnvironment *launch_environment,

                                               const char           *name)

 {

         g_free (launch_environment->priv->x11_display_name);

-- 

1.8.3.1

From 4703ecbca68b4798e95630fced0274e17ee0d5f4 Mon Sep 17 00:00:00 2001

From: Ray Strode <rstrode@redhat.com>

Date: Mon, 6 Oct 2014 11:01:10 -0400

Subject: [PATCH 3/4] slave: stop from dispose not finalize

gdm_slave_stop will reference the passed in object,

so doing it after the object is finalized, is wrong.

This commit moves the stopping to dispose.

---

 daemon/gdm-simple-slave.c | 11 +++++------

 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/daemon/gdm-simple-slave.c b/daemon/gdm-simple-slave.c

index 5057eb4..1ce0284 100644

--- a/daemon/gdm-simple-slave.c

+++ b/daemon/gdm-simple-slave.c

@@ -86,61 +86,61 @@ struct GdmSimpleSlavePrivate

         GdmSession        *session;

         /* this spawns and controls the greeter session */

         GdmLaunchEnvironment *greeter_environment;

         GHashTable        *open_reauthentication_requests;

         GDBusProxy        *accountsservice_proxy;

         guint              have_existing_user_accounts : 1;

         guint              accountsservice_ready : 1;

         guint              waiting_to_connect_to_display : 1;

         guint              start_session_when_ready : 1;

         guint              waiting_to_start_session : 1;

         guint              session_is_running : 1;

 #ifdef  HAVE_LOGINDEVPERM

         gboolean           use_logindevperm;

 #endif

 #ifdef  WITH_PLYMOUTH

         guint              plymouth_is_running : 1;

 #endif

         guint              doing_initial_setup : 1;

 };

 enum {

         PROP_0,

 };

 static void     gdm_simple_slave_class_init     (GdmSimpleSlaveClass *klass);

 static void     gdm_simple_slave_init           (GdmSimpleSlave      *simple_slave);

-static void     gdm_simple_slave_finalize       (GObject             *object);

+static void     gdm_simple_slave_dispose        (GObject             *object);

 static void     gdm_simple_slave_open_reauthentication_channel (GdmSlave             *slave,

                                                                 const char           *username,

                                                                 GPid                  pid_of_caller,

                                                                 uid_t                 uid_of_caller,

                                                                 GAsyncReadyCallback   callback,

                                                                 gpointer              user_data,

                                                                 GCancellable         *cancellable);

 static gboolean wants_initial_setup (GdmSimpleSlave *slave);

 G_DEFINE_TYPE (GdmSimpleSlave, gdm_simple_slave, GDM_TYPE_SLAVE)

 static void create_new_session (GdmSimpleSlave  *slave);

 static void start_session      (GdmSimpleSlave  *slave);

 static void queue_start_session (GdmSimpleSlave *slave,

                                  const char     *service_name);

 static gboolean

 chown_file (GFile   *file,

             uid_t    uid,

             gid_t    gid,

             GError **error)

 {

         if (!g_file_set_attribute_uint32 (file, G_FILE_ATTRIBUTE_UNIX_UID, uid,

                                           G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,

                                           NULL, error)) {

                 return FALSE;

         }

         if (!g_file_set_attribute_uint32 (file, G_FILE_ATTRIBUTE_UNIX_GID, gid,

                                           G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,

                                           NULL, error)) {

@@ -1621,92 +1621,91 @@ gdm_simple_slave_stop (GdmSlave *slave)

                 g_free (username);

 #ifdef  HAVE_LOGINDEVPERM

                 gdm_simple_slave_revoke_console_permissions (self);

 #endif

                 self->priv->session_is_running = FALSE;

         }

         if (self->priv->session != NULL) {

                 gdm_session_close (self->priv->session);

                 g_clear_object (&self->priv->session);

         }

         if (self->priv->server != NULL) {

                 gdm_server_stop (self->priv->server);

                 g_clear_object (&self->priv->server);

         }

         g_clear_object (&self->priv->accountsservice_proxy);

         return TRUE;

 }

 static void

 gdm_simple_slave_class_init (GdmSimpleSlaveClass *klass)

 {

         GObjectClass  *object_class = G_OBJECT_CLASS (klass);

         GdmSlaveClass *slave_class = GDM_SLAVE_CLASS (klass);

-        object_class->finalize = gdm_simple_slave_finalize;

+        object_class->dispose = gdm_simple_slave_dispose;

         slave_class->start = gdm_simple_slave_start;

         slave_class->stop = gdm_simple_slave_stop;

         slave_class->open_session = gdm_simple_slave_open_session;

         slave_class->open_reauthentication_channel = gdm_simple_slave_open_reauthentication_channel;

         slave_class->open_reauthentication_channel_finish = gdm_simple_slave_open_reauthentication_channel_finish;

         g_type_class_add_private (klass, sizeof (GdmSimpleSlavePrivate));

 }

 static void

 gdm_simple_slave_init (GdmSimpleSlave *slave)

 {

         slave->priv = GDM_SIMPLE_SLAVE_GET_PRIVATE (slave);

 #ifdef  HAVE_LOGINDEVPERM

         slave->priv->use_logindevperm = FALSE;

 #endif

         slave->priv->open_reauthentication_requests = g_hash_table_new_full (NULL,

                                                                              NULL,

                                                                              (GDestroyNotify)

                                                                              NULL,

                                                                              (GDestroyNotify)

                                                                              g_object_unref);

 }

 static void

-gdm_simple_slave_finalize (GObject *object)

+gdm_simple_slave_dispose (GObject *object)

 {

         GdmSimpleSlave *slave;

         g_return_if_fail (object != NULL);

         g_return_if_fail (GDM_IS_SIMPLE_SLAVE (object));

         slave = GDM_SIMPLE_SLAVE (object);

         g_return_if_fail (slave->priv != NULL);

         gdm_slave_stop (GDM_SLAVE (slave));

-        g_hash_table_unref (slave->priv->open_reauthentication_requests);

+        g_clear_pointer (&slave->priv->open_reauthentication_requests,

+                         g_hash_table_unref);

         if (slave->priv->greeter_reset_id > 0) {

                 g_source_remove (slave->priv->greeter_reset_id);

                 slave->priv->greeter_reset_id = 0;

         }

-

-        G_OBJECT_CLASS (gdm_simple_slave_parent_class)->finalize (object);

 }

 GdmSlave *

 gdm_simple_slave_new (const char *id)

 {

         GObject *object;

         object = g_object_new (GDM_TYPE_SIMPLE_SLAVE,

                                "display-id", id,

                                NULL);

         return GDM_SLAVE (object);

 }

-- 

1.8.3.1

From dc48d7c3fd45f0c8d623f7766438a5f86e212125 Mon Sep 17 00:00:00 2001

From: Ray Strode <rstrode@redhat.com>

Date: Mon, 6 Oct 2014 11:02:50 -0400

Subject: [PATCH 4/4] systemd: change to KillMode=process

By default systemd will kill the entire control-group in one fell

swoop.  The problem is, things don't get shutdown in the right

order then, and there's a race where GDM will being restarting

X servers and slaves as soon as they're killed.

This mucks with shutdown.

---

 data/gdm.service.in | 1 +

 1 file changed, 1 insertion(+)

diff --git a/data/gdm.service.in b/data/gdm.service.in

index 29eea4f..a92c745 100644

--- a/data/gdm.service.in

+++ b/data/gdm.service.in

@@ -1,17 +1,18 @@

 [Unit]

 Description=GNOME Display Manager

 Conflicts=getty@tty@GDM_INITIAL_VT@.service plymouth-quit.service

 After=systemd-user-sessions.service getty@tty@GDM_INITIAL_VT@.service plymouth-quit.service

 [Service]

 ExecStart=@sbindir@/gdm

 ExecStartPost=-/bin/bash -c "TERM=linux /usr/bin/clear > /dev/tty1"

 ExecStop=-/bin/bash -c "TERM=linux /usr/bin/clear > /dev/tty1"

+KillMode=process

 Restart=always

 IgnoreSIGPIPE=no

 BusName=org.gnome.DisplayManager

 StandardOutput=syslog

 StandardError=inherit

 [Install]

 Alias=display-manager.service

-- 

1.8.3.1