From be4354093b0c3ba75bfd75c8775f3e91d97da3cc Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 2 Nov 2012 08:15:27 +0000 Subject: [PATCH] ActUserManagerNewSession: do not free if there are pending async calls While the ActUserManagerNewSession is the user_data for an asynchronous call, it is not safe to free the struct: when the async call completes, it will dereference the struct. In GIO, cancellation happens asynchronously, so using a GCancellable is not enough to prevent this: the cancelled async call still won't complete until we get back to the main loop. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=50112 Bug-Debian: http://bugs.debian.org/681826 --- src/libaccountsservice/act-user-manager.c | 35 +++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/libaccountsservice/act-user-manager.c b/src/libaccountsservice/act-user-manager.c index 6ca9678..e6a53bb 100644 --- a/src/libaccountsservice/act-user-manager.c +++ b/src/libaccountsservice/act-user-manager.c @@ -107,6 +107,7 @@ typedef struct GCancellable *cancellable; uid_t uid; char *x11_display; + gsize pending_calls; } ActUserManagerNewSession; typedef enum { @@ -961,6 +962,11 @@ unload_new_session (ActUserManagerNewSession *new_session) { ActUserManager *manager; + /* From here down to the check on pending_calls is idempotent, + * like GObject dispose(); it can be called twice if the new session + * is unloaded while there are still async calls pending. + */ + manager = new_session->manager; if (new_session->cancellable != NULL && @@ -970,16 +976,29 @@ unload_new_session (ActUserManagerNewSession *new_session) new_session->cancellable = NULL; } - manager->priv->new_sessions = g_slist_remove (manager->priv->new_sessions, - new_session); - if (new_session->proxy != NULL) { g_object_unref (new_session->proxy); + new_session->proxy = NULL; } g_free (new_session->x11_display); + new_session->x11_display = NULL; g_free (new_session->id); - g_object_unref (manager); + new_session->id = NULL; + + if (manager != NULL) { + manager->priv->new_sessions = g_slist_remove (manager->priv->new_sessions, + new_session); + + new_session->manager = NULL; + g_object_unref (manager); + } + + if (new_session->pending_calls != 0) { + /* don't "finalize" until we run out of pending calls + * that have us as their user_data */ + return; + } g_slice_free (ActUserManagerNewSession, new_session); } @@ -1025,7 +1044,10 @@ on_get_unix_user_finished (GObject *object, GError *error = NULL; guint uid; + new_session->pending_calls--; + if (new_session->cancellable == NULL || g_cancellable_is_cancelled (new_session->cancellable)) { + unload_new_session (new_session); return; } @@ -1087,6 +1109,7 @@ get_uid_for_new_session (ActUserManagerNewSession *new_session) g_assert (new_session->proxy != NULL); + new_session->pending_calls++; console_kit_session_call_get_unix_user (new_session->proxy, new_session->cancellable, on_get_unix_user_finished, @@ -1228,7 +1251,10 @@ on_get_x11_display_finished (GObject *object, GError *error = NULL; char *x11_display; + new_session->pending_calls--; + if (new_session->cancellable == NULL || g_cancellable_is_cancelled (new_session->cancellable)) { + unload_new_session (new_session); return; } @@ -1318,6 +1344,7 @@ get_x11_display_for_new_session (ActUserManagerNewSession *new_session) g_assert (new_session->proxy != NULL); + new_session->pending_calls++; console_kit_session_call_get_x11_display (new_session->proxy, new_session->cancellable, on_get_x11_display_finished, -- 1.7.10.4