From 0b093b3cfbda8a2d75608b338d04c3bdc30b6b3a Mon Sep 17 00:00:00 2001

From: Ray Strode <rstrode@redhat.com>

Date: Mon, 25 Apr 2011 15:52:36 -0400

Subject: [PATCH 1/3] lib: always set is-loaded through maybe_set_is_loaded

maybe_set_is_loaded checks the various asynchronous things that

happen at startup and delays setting is-loaded until all those

initial start up tasks are finished.

We had one place in the code that was bypassing maybe_set_is_loaded

and settings is-loaded directly.  This could mean the user manager

in some scenarios reports its loaded before it actually is.

---

 src/libaccountsservice/act-user-manager.c |    2 +-

 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/libaccountsservice/act-user-manager.c b/src/libaccountsservice/act-user-manager.c

index 7202b07..2af6808 100644

--- a/src/libaccountsservice/act-user-manager.c

+++ b/src/libaccountsservice/act-user-manager.c

@@ -801,7 +801,7 @@ on_new_user_loaded (ActUser        *user,

         g_object_unref (user);

         if (manager->priv->new_users == NULL) {

-                set_is_loaded (manager, TRUE);

+                maybe_set_is_loaded (manager);

         }

 }

-- 

1.7.4.4

From 07f05cd823bb52bee449996a16b90eaa1b57f80f Mon Sep 17 00:00:00 2001

From: Ray Strode <rstrode@redhat.com>

Date: Mon, 25 Apr 2011 16:38:28 -0400

Subject: [PATCH 2/3] lib: add some additional debugging statements

The current debug logs leave a lot of guessing,

this commit tries to be a little more chatty.

---

 src/libaccountsservice/act-user-manager.c |   46 ++++++++++++++++++++++++++--

 1 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/src/libaccountsservice/act-user-manager.c b/src/libaccountsservice/act-user-manager.c

index 2af6808..3e463e6 100644

--- a/src/libaccountsservice/act-user-manager.c

+++ b/src/libaccountsservice/act-user-manager.c

@@ -533,7 +533,8 @@ on_user_changed (ActUser        *user,

                  ActUserManager *manager)

 {

         if (manager->priv->is_loaded) {

-                g_debug ("ActUserManager: user changed");

+                g_debug ("ActUserManager: user %s changed",

+                         act_user_get_user_name (user));

                 g_signal_emit (manager, signals[USER_CHANGED], 0, user);

         }

 }

@@ -571,6 +572,8 @@ on_get_seat_id_finished (DBusGProxy     *proxy,

                                  "current session");

                 }

                 unload_seat (manager);

+

+                g_debug ("ActUserManager: GetSeatId call failed, so trying to set loaded property");

                 maybe_set_is_loaded (manager);

                 return;

         }

@@ -692,6 +695,7 @@ add_user (ActUserManager *manager,

 {

         const char *object_path;

+        g_debug ("ActUserManager: tracking user '%s'", act_user_get_user_name (user));

         g_hash_table_insert (manager->priv->users_by_name,

                              g_strdup (act_user_get_user_name (user)),

                              g_object_ref (user));

@@ -713,7 +717,10 @@ add_user (ActUserManager *manager,

                           manager);

         if (manager->priv->is_loaded) {

+                g_debug ("ActUserManager: loaded, so emitting user-added signal");

                 g_signal_emit (manager, signals[USER_ADDED], 0, user);

+        } else {

+                g_debug ("ActUserManager: not yet loaded, so not emitting user-added signal");

         }

         if (g_hash_table_size (manager->priv->users_by_name) > 1) {

@@ -725,6 +732,10 @@ static void

 remove_user (ActUserManager *manager,

              ActUser        *user)

 {

+        g_debug ("ActUserManager: no longer tracking user '%s' (with object path %s)",

+                 act_user_get_user_name (user),

+                 act_user_get_object_path (user));

+

         g_object_ref (user);

         g_signal_handlers_disconnect_by_func (user, on_user_changed, manager);

@@ -738,7 +749,10 @@ remove_user (ActUserManager *manager,

         }

         if (manager->priv->is_loaded) {

+                g_debug ("ActUserManager: loaded, so emitting user-removed signal");

                 g_signal_emit (manager, signals[USER_REMOVED], 0, user);

+        } else {

+                g_debug ("ActUserManager: not yet loaded, so not emitting user-removed signal");

         }

         g_object_unref (user);

@@ -759,7 +773,6 @@ on_new_user_loaded (ActUser        *user,

         if (!act_user_is_loaded (user)) {

                 return;

         }

-

         g_signal_handlers_disconnect_by_func (user, on_new_user_loaded, manager);

         manager->priv->new_users = g_slist_remove (manager->priv->new_users,

                                                    user);

@@ -783,6 +796,8 @@ on_new_user_loaded (ActUser        *user,

                 return;

         }

+        g_debug ("ActUserManager: user '%s' is now loaded", username);

+

         if (username_in_exclude_list (manager, username)) {

                 g_debug ("ActUserManager: excluding user '%s'", username);

                 g_object_unref (user);

@@ -794,6 +809,8 @@ on_new_user_loaded (ActUser        *user,

         /* If username got added earlier by a different means, trump it now.

          */

         if (old_user != NULL) {

+                g_debug ("ActUserManager: user '%s' was already known, "

+                         "replacing with freshly loaded object", username);

                 remove_user (manager, old_user);

         }

@@ -801,6 +818,7 @@ on_new_user_loaded (ActUser        *user,

         g_object_unref (user);

         if (manager->priv->new_users == NULL) {

+                g_debug ("ActUserManager: no pending users, trying to set loaded property");

                 maybe_set_is_loaded (manager);

         }

 }

@@ -814,8 +832,13 @@ add_new_user_for_object_path (const char     *object_path,

         user = g_hash_table_lookup (manager->priv->users_by_object_path, object_path); 

         if (user != NULL) {

+                g_debug ("ActUserManager: tracking existing user %s with object path %s",

+                         act_user_get_user_name (user), object_path);

                 return user;

         }

+

+        g_debug ("ActUserManager: tracking new user with object path %s", object_path);

+

         user = create_new_user (manager);

         _act_user_update_from_object_path (user, object_path);

@@ -829,6 +852,7 @@ on_new_user_in_accounts_service (DBusGProxy *proxy,

 {

         ActUserManager *manager = ACT_USER_MANAGER (user_data);

+        g_debug ("ActUserManager: new user in accounts service with object path %s", object_path);

         add_new_user_for_object_path (object_path, manager);

 }

@@ -840,6 +864,8 @@ on_user_removed_in_accounts_service (DBusGProxy *proxy,

         ActUserManager *manager = ACT_USER_MANAGER (user_data);

         ActUser        *user;

+        g_debug ("ActUserManager: user removed from accounts service with object path %s", object_path);

+

         user = g_hash_table_lookup (manager->priv->users_by_object_path, object_path);

         manager->priv->new_users = g_slist_remove (manager->priv->new_users, user);

@@ -879,6 +905,7 @@ on_get_current_session_finished (DBusGProxy     *proxy,

                         g_debug ("Failed to identify the current session");

                 }

                 unload_seat (manager);

+                g_debug ("ActUserManager: no current session, so trying to set loaded property");

                 maybe_set_is_loaded (manager);

                 return;

         }

@@ -1155,6 +1182,7 @@ on_list_cached_users_finished (DBusGProxy     *proxy,

                 return;

         }

+        g_debug ("ActUserManager: ListCachedUsers finished, so trying to set loaded property");

         maybe_set_is_loaded (manager);

         g_ptr_array_foreach (paths, (GFunc)add_new_user_for_object_path, manager);

@@ -1721,22 +1749,29 @@ static void

 maybe_set_is_loaded (ActUserManager *manager)

 {

         if (manager->priv->is_loaded) {

+                g_debug ("ActUserManager: already loaded, so not setting loaded property");

                 return;

         }

         if (manager->priv->get_sessions_call != NULL) {

+                g_debug ("ActUserManager: GetSessions call pending, so not setting loaded property");

                 return;

         }

         if (manager->priv->listing_cached_users) {

+                g_debug ("ActUserManager: Listing cached users, so not setting loaded property");

                 return;

         }

         /* Don't set is_loaded yet unless the seat is already loaded

          * or failed to load.

          */

-        if (manager->priv->seat.state != ACT_USER_MANAGER_SEAT_STATE_LOADED

-            && manager->priv->seat.state != ACT_USER_MANAGER_SEAT_STATE_UNLOADED) {

+        if (manager->priv->seat.state == ACT_USER_MANAGER_SEAT_STATE_LOADED) {

+                g_debug ("ActUserManager: Seat loaded, so now setting loaded property");

+        } else if (manager->priv->seat.state == ACT_USER_MANAGER_SEAT_STATE_UNLOADED) {

+                g_debug ("ActUserManager: Seat wouldn't load, so giving up on it and setting loaded property");

+        } else {

+                g_debug ("ActUserManager: Seat still actively loading, so not setting loaded property");

                 return;

         }

@@ -1811,6 +1846,8 @@ on_get_sessions_finished (DBusGProxy     *proxy,

                                   (int) sessions->len);

         g_ptr_array_foreach (sessions, (GFunc) g_free, NULL);

         g_ptr_array_free (sessions, TRUE);

+

+        g_debug ("ActUserManager: GetSessions call finished, so trying to set loaded property");

         maybe_set_is_loaded (manager);

 }

@@ -1880,6 +1917,7 @@ load_seat_incrementally (ActUserManager *manager)

                 load_sessions (manager);

         }

+        g_debug ("ActUserManager: Seat loading sequence complete, so trying to set loaded property");

         maybe_set_is_loaded (manager);

 }

-- 

1.7.4.4

From f938e70e4c71b63235702a48ab6d2400488c3ea6 Mon Sep 17 00:00:00 2001

From: Ray Strode <rstrode@redhat.com>

Date: Mon, 25 Apr 2011 16:58:58 -0400

Subject: [PATCH 3/3] lib: ignore new users before ListCachedUsers finishes

We're going to be loading any users that come in shortly afterward

anyway, so don't bother handling it yet.

---

 src/libaccountsservice/act-user-manager.c |    5 +++++

 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/src/libaccountsservice/act-user-manager.c b/src/libaccountsservice/act-user-manager.c

index 3e463e6..07c8e34 100644

--- a/src/libaccountsservice/act-user-manager.c

+++ b/src/libaccountsservice/act-user-manager.c

@@ -852,6 +852,11 @@ on_new_user_in_accounts_service (DBusGProxy *proxy,

 {

         ActUserManager *manager = ACT_USER_MANAGER (user_data);

+        if (!manager->priv->is_loaded) {

+                g_debug ("ActUserManager: ignoring new user in accounts service with object path %s since not loaded yet", object_path);

+                return;

+        }

+

         g_debug ("ActUserManager: new user in accounts service with object path %s", object_path);

         add_new_user_for_object_path (object_path, manager);

 }

-- 

1.7.4.4

From ce35916e540ead4a84a779841b9b3f81b7595a34 Mon Sep 17 00:00:00 2001

From: Ray Strode <rstrode@redhat.com>

Date: Tue, 26 Apr 2011 10:24:04 -0400

Subject: [PATCH] lib: set is-loaded only after we've actually loaded the

 users

Before we were setting it as soon as we got the unloaded list,

which isn't sufficient, since callers wait for is-loaded to call

list_users, and list_users only returns loaded users.

(bug initially introduced by commit fffd5c51a54682e17ac61e374d629e9aa0dbeeb1)

---

 src/libaccountsservice/act-user-manager.c |   18 ++++++++++++++----

 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/libaccountsservice/act-user-manager.c b/src/libaccountsservice/act-user-manager.c

index 07c8e34..e98140e 100644

--- a/src/libaccountsservice/act-user-manager.c

+++ b/src/libaccountsservice/act-user-manager.c

@@ -793,7 +793,7 @@ on_new_user_loaded (ActUser        *user,

                                    (int) act_user_get_uid (user));

                 }

                 g_object_unref (user);

-                return;

+                goto out;

         }

         g_debug ("ActUserManager: user '%s' is now loaded", username);

@@ -801,7 +801,7 @@ on_new_user_loaded (ActUser        *user,

         if (username_in_exclude_list (manager, username)) {

                 g_debug ("ActUserManager: excluding user '%s'", username);

                 g_object_unref (user);

-                return;

+                goto out;

         }

         old_user = g_hash_table_lookup (manager->priv->users_by_name, username);

@@ -817,6 +817,7 @@ on_new_user_loaded (ActUser        *user,

         add_user (manager, user);

         g_object_unref (user);

+out:

         if (manager->priv->new_users == NULL) {

                 g_debug ("ActUserManager: no pending users, trying to set loaded property");

                 maybe_set_is_loaded (manager);

@@ -1187,8 +1188,12 @@ on_list_cached_users_finished (DBusGProxy     *proxy,

                 return;

         }

-        g_debug ("ActUserManager: ListCachedUsers finished, so trying to set loaded property");

-        maybe_set_is_loaded (manager);

+        /* We now have a batch of unloaded users that we know about. Once that initial

+         * batch is loaded up, we can mark the manager as loaded.

+         *

+         * (see on_new_user_loaded)

+         */

+        g_debug ("ActUserManager: ListCachedUsers finished, will set loaded property after list is fully loaded");

         g_ptr_array_foreach (paths, (GFunc)add_new_user_for_object_path, manager);

         g_ptr_array_foreach (paths, (GFunc)g_free, NULL);

@@ -1768,6 +1773,11 @@ maybe_set_is_loaded (ActUserManager *manager)

                 return;

         }

+        if (manager->priv->new_users != NULL) {

+                g_debug ("ActUserManager: Loading new users, so not setting loaded property");

+                return;

+        }

+

         /* Don't set is_loaded yet unless the seat is already loaded

          * or failed to load.

          */

-- 

1.7.4.4

From c94351c10838daeafedf0c37cea41c7308799802 Mon Sep 17 00:00:00 2001

From: Ray Strode <rstrode@redhat.com>

Date: Wed, 27 Apr 2011 14:36:50 -0400

Subject: [PATCH] lib: don't block loading for get_user() requests

When an app does a get_user() request the user gets added to the

"new user" list while its loading up.

We currently delaying loading until the "new user" list is empty,

but get_user() requests are deferred until initial loading is done,

so if a get_user() request comes in during initial load, load will

never complete.

This commit differentiates new users resulting from the initial

ListCachedUsers call and new users resulting from get_user() requests.

Only the first group inhibit the initial load from completing.

---

 src/libaccountsservice/act-user-manager.c |   33 ++++++++++++++++++++++++++--

 1 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/libaccountsservice/act-user-manager.c b/src/libaccountsservice/act-user-manager.c

index e98140e..379c972 100644

--- a/src/libaccountsservice/act-user-manager.c

+++ b/src/libaccountsservice/act-user-manager.c

@@ -140,6 +140,7 @@ struct ActUserManagerPrivate

         GSList                *new_sessions;

         GSList                *new_users;

+        GSList                *new_users_inhibiting_load;

         GSList                *fetch_user_requests;

         GSList                *exclude_usernames;

@@ -771,11 +772,16 @@ on_new_user_loaded (ActUser        *user,

         ActUser *old_user;

         if (!act_user_is_loaded (user)) {

+                g_debug ("ActUserManager: user '%s' loaded function called when not loaded",

+                         act_user_get_user_name (user));

                 return;

         }

         g_signal_handlers_disconnect_by_func (user, on_new_user_loaded, manager);

+

         manager->priv->new_users = g_slist_remove (manager->priv->new_users,

                                                    user);

+        manager->priv->new_users_inhibiting_load = g_slist_remove (manager->priv->new_users_inhibiting_load,

+                                                                   user);

         username = act_user_get_user_name (user);

@@ -818,9 +824,11 @@ on_new_user_loaded (ActUser        *user,

         g_object_unref (user);

 out:

-        if (manager->priv->new_users == NULL) {

+        if (manager->priv->new_users_inhibiting_load == NULL) {

                 g_debug ("ActUserManager: no pending users, trying to set loaded property");

                 maybe_set_is_loaded (manager);

+        } else {

+                g_debug ("ActUserManager: not all users loaded yet");

         }

 }

@@ -1165,6 +1173,19 @@ set_is_loaded (ActUserManager *manager,

 }

 static void

+add_new_inhibiting_user_for_object_path (const char     *object_path,

+                                         ActUserManager *manager)

+{

+        ActUser *user;

+

+        user = add_new_user_for_object_path (object_path, manager);

+

+        if (!manager->priv->is_loaded) {

+                manager->priv->new_users_inhibiting_load = g_slist_prepend (manager->priv->new_users_inhibiting_load, user);

+        }

+}

+

+static void

 on_list_cached_users_finished (DBusGProxy     *proxy,

                                DBusGProxyCall *call_id,

                                gpointer        data)

@@ -1194,7 +1215,7 @@ on_list_cached_users_finished (DBusGProxy     *proxy,

          * (see on_new_user_loaded)

          */

         g_debug ("ActUserManager: ListCachedUsers finished, will set loaded property after list is fully loaded");

-        g_ptr_array_foreach (paths, (GFunc)add_new_user_for_object_path, manager);

+        g_ptr_array_foreach (paths, (GFunc)add_new_inhibiting_user_for_object_path, manager);

         g_ptr_array_foreach (paths, (GFunc)g_free, NULL);

         g_ptr_array_free (paths, TRUE);

@@ -1616,6 +1637,9 @@ on_user_manager_maybe_ready_for_request (ActUserManager                 *manager

                 return;

         }

+        g_debug ("ActUserManager: user manager now loaded, proceeding with fetch user request for user '%s'",

+                 request->username);

+

         g_signal_handlers_disconnect_by_func (manager, on_user_manager_maybe_ready_for_request, request);

         request->state++;

@@ -1714,6 +1738,7 @@ act_user_manager_get_user (ActUserManager *manager,

         /* if we don't have it loaded try to load it now */

         if (user == NULL) {

+                g_debug ("ActUserManager: trying to track new user with username %s", username);

                 user = create_new_user (manager);

                 if (manager->priv->accounts_proxy != NULL) {

@@ -1773,7 +1798,7 @@ maybe_set_is_loaded (ActUserManager *manager)

                 return;

         }

-        if (manager->priv->new_users != NULL) {

+        if (manager->priv->new_users_inhibiting_load != NULL) {

                 g_debug ("ActUserManager: Loading new users, so not setting loaded property");

                 return;

         }

@@ -2186,6 +2211,8 @@ act_user_manager_finalize (GObject *object)

                          (GFunc) free_fetch_user_request, NULL);

         g_slist_free (manager->priv->fetch_user_requests);

+        g_slist_free (manager->priv->new_users_inhibiting_load);

+

         node = manager->priv->new_users;

         while (node != NULL) {

                 ActUser *user;

-- 

1.7.4.4