Attachment 69424 Details for Bug 50112 – the patch that I'm trying out

[patch] the patch that I'm trying out

9004-ActUserManagerNewSession-do-not-free-if-there-are-pe.patch (text/plain), 4.48 KB, created by Simon McVittie on 2012-11-02 09:36:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2012-11-02 09:36:10 UTC

Size: 4.48 KB

patch

obsolete

>From be4354093b0c3ba75bfd75c8775f3e91d97da3cc Mon Sep 17 00:00:00 2001
>From: Simon McVittie <smcv@debian.org>
>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
>

Actions: View | Splinter Review

Attachments on bug 50112: 61928 | 62894 | 69424

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.