From 0b093b3cfbda8a2d75608b338d04c3bdc30b6b3a Mon Sep 17 00:00:00 2001 From: Ray Strode 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 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 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 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 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