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