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