From b1ec3114a49c9e5331a4f7c106a58867499ac1ce Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Wed, 4 Oct 2017 11:28:50 -0400 Subject: [PATCH 13/13] lib: only track users after act_user_manager_list_users At the moment, as soon as someone calls act_user_manager_get_default() we end up firing of an asynchronous call to get a list of all users. This is problematic since not all programs using accountsservice want a list of users. This commit changes the code to only get a list of users when the caller invokes act_user_manager_list_users. This does mean some calls that were async before are synchronous now, but user proxies were always obtained synchronously, and they're by far the slowest part of listing users, so I don't expect this introduce any noticeable blocking. Longer term, to fix the sync i/o bits, I think we should probably ditch libaccountsservice and just make accountsservice use ObjectManager interfaces over d-bus. --- src/libaccountsservice/act-user-manager.c | 145 +++++++++++++++--------------- 1 file changed, 75 insertions(+), 70 deletions(-) diff --git a/src/libaccountsservice/act-user-manager.c b/src/libaccountsservice/act-user-manager.c index 11049e0..d0b38e2 100644 --- a/src/libaccountsservice/act-user-manager.c +++ b/src/libaccountsservice/act-user-manager.c @@ -172,92 +172,93 @@ typedef struct }; char *object_path; char *description; } ActUserManagerFetchUserRequest; struct ActUserManagerPrivate { GHashTable *normal_users_by_name; GHashTable *system_users_by_name; GHashTable *users_by_object_path; GHashTable *sessions; GDBusConnection *connection; AccountsAccounts *accounts_proxy; ConsoleKitManager *ck_manager_proxy; ActUserManagerSeat seat; GSList *new_sessions; GSList *new_users; GSList *new_users_inhibiting_load; GSList *fetch_user_requests; GSList *exclude_usernames; GSList *include_usernames; guint load_id; gboolean is_loaded; gboolean has_multiple_users; gboolean getting_sessions; - gboolean listing_cached_users; gboolean list_cached_users_done; }; enum { PROP_0, PROP_INCLUDE_USERNAMES_LIST, PROP_EXCLUDE_USERNAMES_LIST, PROP_IS_LOADED, PROP_HAS_MULTIPLE_USERS }; enum { USER_ADDED, USER_REMOVED, USER_IS_LOGGED_IN_CHANGED, USER_CHANGED, LAST_SIGNAL }; static guint signals [LAST_SIGNAL] = { 0, }; static void act_user_manager_class_init (ActUserManagerClass *klass); static void act_user_manager_init (ActUserManager *user_manager); static void act_user_manager_finalize (GObject *object); static gboolean ensure_accounts_proxy (ActUserManager *manager); static gboolean load_seat_incrementally (ActUserManager *manager); static void unload_seat (ActUserManager *manager); static void load_users (ActUserManager *manager); +static void load_user (ActUserManager *manager, + const char *username); static void act_user_manager_queue_load (ActUserManager *manager); -static void queue_load_seat_and_users (ActUserManager *manager); +static void queue_load_seat (ActUserManager *manager); static void load_new_session_incrementally (ActUserManagerNewSession *new_session); static void set_is_loaded (ActUserManager *manager, gboolean is_loaded); static void on_new_user_loaded (ActUser *user, GParamSpec *pspec, ActUserManager *manager); static void give_up (ActUserManager *manager, ActUserManagerFetchUserRequest *request); static void fetch_user_incrementally (ActUserManagerFetchUserRequest *request); static void maybe_set_is_loaded (ActUserManager *manager); static void update_user (ActUserManager *manager, ActUser *user); static gpointer user_manager_object = NULL; G_DEFINE_TYPE (ActUserManager, act_user_manager, G_TYPE_OBJECT) static const GDBusErrorEntry error_entries[] = { { ACT_USER_MANAGER_ERROR_FAILED, "org.freedesktop.Accounts.Error.Failed" }, { ACT_USER_MANAGER_ERROR_USER_EXISTS, "org.freedesktop.Accounts.Error.UserExists" }, { ACT_USER_MANAGER_ERROR_USER_DOES_NOT_EXIST, "org.freedesktop.Accounts.Error.UserDoesNotExist" }, { ACT_USER_MANAGER_ERROR_PERMISSION_DENIED, "org.freedesktop.Accounts.Error.PermissionDenied" }, { ACT_USER_MANAGER_ERROR_NOT_SUPPORTED, "org.freedesktop.Accounts.Error.NotSupported" } }; GQuark act_user_manager_error_quark (void) { static volatile gsize ret = 0; @@ -711,91 +712,87 @@ on_get_seat_id_finished (GObject *object, GAsyncResult *result, gpointer data) { ConsoleKitSession *proxy = CONSOLE_KIT_SESSION (object); ActUserManager *manager = data; GError *error = NULL; char *seat_id; if (!console_kit_session_call_get_seat_id_finish (proxy, &seat_id, result, &error)) { if (error != NULL) { g_debug ("Failed to identify the seat of the " "current session: %s", error->message); g_error_free (error); } else { g_debug ("Failed to identify the seat of the " "current session"); } g_debug ("ActUserManager: GetSeatId call failed, so unloading seat"); unload_seat (manager); goto out; } g_debug ("ActUserManager: Found current seat: %s", seat_id); manager->priv->seat.id = seat_id; manager->priv->seat.state++; - load_seat_incrementally (manager); - out: g_debug ("ActUserManager: unrefing manager owned by GetSeatId request"); g_object_unref (manager); } #ifdef WITH_SYSTEMD static void _get_systemd_seat_id (ActUserManager *manager) { int res; char *seat_id; res = sd_session_get_seat (NULL, &seat_id); if (res == -ENOENT) { seat_id = NULL; } else if (res < 0) { g_warning ("Could not get current seat: %s", strerror (-res)); unload_seat (manager); return; } manager->priv->seat.id = g_strdup (seat_id); free (seat_id); manager->priv->seat.state++; - - queue_load_seat_incrementally (manager); } #endif static void get_seat_id_for_current_session (ActUserManager *manager) { #ifdef WITH_SYSTEMD if (LOGIND_RUNNING()) { _get_systemd_seat_id (manager); return; } #endif console_kit_session_call_get_seat_id (manager->priv->seat.session_proxy, NULL, on_get_seat_id_finished, g_object_ref (manager)); } static gint match_name_cmpfunc (gconstpointer a, gconstpointer b) { return g_strcmp0 ((char *) a, (char *) b); } static gboolean username_in_exclude_list (ActUserManager *manager, const char *username) { @@ -1520,106 +1517,64 @@ load_user_paths (ActUserManager *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) */ if (g_strv_length ((char **) user_paths) > 0) { int i; g_debug ("ActUserManager: ListCachedUsers finished, will set loaded property after list is fully loaded"); for (i = 0; user_paths[i] != NULL; i++) { ActUser *user; user = add_new_user_for_object_path (user_paths[i], manager); if (!manager->priv->is_loaded) { manager->priv->new_users_inhibiting_load = g_slist_prepend (manager->priv->new_users_inhibiting_load, user); } } } else { g_debug ("ActUserManager: ListCachedUsers finished with empty list, maybe setting loaded property now"); maybe_set_is_loaded (manager); } } static void load_included_usernames (ActUserManager *manager) { GSList *l; /* Add users who are specifically included */ for (l = manager->priv->include_usernames; l != NULL; l = l->next) { - ActUser *user; - g_debug ("ActUserManager: Adding included user %s", (char *)l->data); - /* - * The call to act_user_manager_get_user will add the user if it is - * valid and not already in the hash. - */ - user = act_user_manager_get_user (manager, l->data); - if (user == NULL) { - g_debug ("ActUserManager: unable to lookup user '%s'", (char *)l->data); - } - } -} - -static void -on_list_cached_users_finished (GObject *object, - GAsyncResult *result, - gpointer data) -{ - AccountsAccounts *proxy = ACCOUNTS_ACCOUNTS (object); - ActUserManager *manager = data; - gchar **user_paths; - GError *error = NULL; - - manager->priv->listing_cached_users = FALSE; - manager->priv->list_cached_users_done = TRUE; - - if (!accounts_accounts_call_list_cached_users_finish (proxy, &user_paths, result, &error)) { - g_debug ("ActUserManager: ListCachedUsers failed: %s", error->message); - g_error_free (error); - - g_object_unref (manager->priv->accounts_proxy); - manager->priv->accounts_proxy = NULL; - g_debug ("ActUserManager: unrefing manager owned by failed ListCachedUsers call"); - g_object_unref (manager); - return; + load_user (manager, l->data); } - - load_user_paths (manager, (const char * const *) user_paths); - g_strfreev (user_paths); - - load_included_usernames (manager); - - g_debug ("ActUserManager: unrefing manager owned by finished ListCachedUsers call"); - g_object_unref (manager); } static void on_get_x11_display_finished (GObject *object, GAsyncResult *result, gpointer data) { ConsoleKitSession *proxy = CONSOLE_KIT_SESSION (object); ActUserManagerNewSession *new_session = data; 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; } if (!console_kit_session_call_get_x11_display_finish (proxy, &x11_display, result, &error)) { if (error != NULL) { g_debug ("Failed to get the x11 display of session '%s': %s", new_session->id, error->message); g_error_free (error); } else { g_debug ("Failed to get the x11 display of session '%s'", new_session->id); } unload_new_session (new_session); return; @@ -2361,60 +2316,99 @@ fetch_user_with_id_from_accounts_service (ActUserManager *manager, * from @manager. Trying to use this object before its * #ActUser:is-loaded property is %TRUE will result in undefined * behavior. * * Returns: (transfer none): #ActUser object **/ ActUser * act_user_manager_get_user (ActUserManager *manager, const char *username) { ActUser *user; g_return_val_if_fail (ACT_IS_USER_MANAGER (manager), NULL); g_return_val_if_fail (username != NULL && username[0] != '\0', NULL); user = lookup_user_by_name (manager, username); /* 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) { fetch_user_with_username_from_accounts_service (manager, user, username); } } return user; } +static void +load_user (ActUserManager *manager, + const char *username) +{ + ActUser *user; + GError *error = NULL; + char *object_path = NULL; + gboolean user_found; + + g_return_if_fail (ACT_IS_USER_MANAGER (manager)); + g_return_if_fail (username != NULL && username[0] != '\0'); + + user = lookup_user_by_name (manager, username); + + if (user == NULL) { + g_debug ("ActUserManager: trying to track new user with username %s", username); + user = create_new_user (manager); + } + + user_found = accounts_accounts_call_find_user_by_name_sync (manager->priv->accounts_proxy, + username, + &object_path, + NULL, + &error); + + if (!user_found) { + if (error != NULL) { + g_debug ("ActUserManager: Failed to find user '%s': %s", + username, error->message); + g_clear_error (&error); + } else { + g_debug ("ActUserManager: Failed to find user '%s'", + username); + } + } + + _act_user_update_from_object_path (user, object_path); +} + /** * act_user_manager_get_user_by_id: * @manager: the manager to query. * @id: the uid of the user to get. * * Retrieves a pointer to the #ActUser object for the user with the * given uid from @manager. Trying to use this object before its * #ActUser:is-loaded property is %TRUE will result in undefined * behavior. * * Returns: (transfer none): #ActUser object */ ActUser * act_user_manager_get_user_by_id (ActUserManager *manager, uid_t id) { ActUser *user; gchar *object_path; g_return_val_if_fail (ACT_IS_USER_MANAGER (manager), NULL); object_path = g_strdup_printf ("/org/freedesktop/Accounts/User%lu", (gulong) id); user = g_hash_table_lookup (manager->priv->users_by_object_path, object_path); g_free (object_path); if (user != NULL) { return g_object_ref (user); } else { g_debug ("ActUserManager: trying to track new user with uid %lu", (gulong) id); user = create_new_user (manager); @@ -2425,93 +2419,95 @@ act_user_manager_get_user_by_id (ActUserManager *manager, } return user; } static void listify_hash_values_hfunc (gpointer key, gpointer value, gpointer user_data) { GSList **list = user_data; *list = g_slist_prepend (*list, value); } /** * act_user_manager_list_users: * @manager: a #ActUserManager * * Get a list of system user accounts * * Returns: (element-type ActUser) (transfer container): List of #ActUser objects */ GSList * act_user_manager_list_users (ActUserManager *manager) { GSList *retval; g_return_val_if_fail (ACT_IS_USER_MANAGER (manager), NULL); + if (!manager->priv->list_cached_users_done) { + load_users (manager); + + if (manager->priv->seat.state == ACT_USER_MANAGER_SEAT_STATE_GET_SEAT_PROXY) + queue_load_seat_incrementally (manager); + } + retval = NULL; g_hash_table_foreach (manager->priv->normal_users_by_name, listify_hash_values_hfunc, &retval); return g_slist_sort (retval, (GCompareFunc) act_user_collate); } 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->getting_sessions) { 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; - } - if (manager->priv->new_users_inhibiting_load != 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 + /* Don't set is_loaded yet unless the seat is already loaded enough * or failed to load. */ - if (manager->priv->seat.state == ACT_USER_MANAGER_SEAT_STATE_LOADED) { + if (manager->priv->seat.state >= ACT_USER_MANAGER_SEAT_STATE_GET_ID) { 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; } set_is_loaded (manager, TRUE); } static GSList * slist_deep_copy (const GSList *list) { GSList *retval; GSList *l; if (list == NULL) return NULL; retval = g_slist_copy ((GSList *) list); for (l = retval; l != NULL; l = l->next) { l->data = g_strdup (l->data); } return retval; } static void @@ -2555,125 +2551,134 @@ static void load_console_kit_sessions (ActUserManager *manager) { if (manager->priv->seat.seat_proxy == NULL) { g_debug ("ActUserManager: no seat proxy; can't load sessions"); return; } manager->priv->getting_sessions = TRUE; console_kit_seat_call_get_sessions (manager->priv->seat.seat_proxy, NULL, on_get_sessions_finished, g_object_ref (manager)); } static void load_sessions (ActUserManager *manager) { #ifdef WITH_SYSTEMD if (LOGIND_RUNNING()) { reload_systemd_sessions (manager); maybe_set_is_loaded (manager); return; } #endif load_console_kit_sessions (manager); } static void load_users (ActUserManager *manager) { - g_assert (manager->priv->accounts_proxy != NULL); - g_debug ("ActUserManager: calling 'ListCachedUsers'"); + GError *error = NULL; + char **user_paths = NULL; + gboolean could_list = FALSE; if (!ensure_accounts_proxy (manager)) { return; } - accounts_accounts_call_list_cached_users (manager->priv->accounts_proxy, - NULL, - on_list_cached_users_finished, - g_object_ref (manager)); - manager->priv->listing_cached_users = TRUE; + g_debug ("ActUserManager: calling 'ListCachedUsers'"); + + could_list = accounts_accounts_call_list_cached_users_sync (manager->priv->accounts_proxy, + &user_paths, + NULL, &error); + + if (!could_list) { + g_debug ("ActUserManager: ListCachedUsers failed: %s", error->message); + g_clear_error (&error); + return; + } + + load_user_paths (manager, (const char * const *) user_paths); + g_strfreev (user_paths); + + load_included_usernames (manager); + + manager->priv->list_cached_users_done = TRUE; } static gboolean load_seat_incrementally (ActUserManager *manager) { manager->priv->seat.load_idle_id = 0; switch (manager->priv->seat.state) { case ACT_USER_MANAGER_SEAT_STATE_GET_SESSION_ID: get_current_session_id (manager); break; case ACT_USER_MANAGER_SEAT_STATE_GET_SESSION_PROXY: get_session_proxy (manager); break; case ACT_USER_MANAGER_SEAT_STATE_GET_ID: get_seat_id_for_current_session (manager); break; case ACT_USER_MANAGER_SEAT_STATE_GET_SEAT_PROXY: get_seat_proxy (manager); break; case ACT_USER_MANAGER_SEAT_STATE_LOADED: g_debug ("ActUserManager: Seat loading sequence complete"); break; default: g_assert_not_reached (); } if (manager->priv->seat.state == ACT_USER_MANAGER_SEAT_STATE_LOADED) { load_sessions (manager); } maybe_set_is_loaded (manager); return FALSE; } static gboolean load_idle (ActUserManager *manager) { - /* The order below is important: load_seat_incrementally might - set "is-loaded" immediately and we thus need to call - load_users before it. - */ - load_users (manager); manager->priv->seat.state = ACT_USER_MANAGER_SEAT_STATE_UNLOADED + 1; load_seat_incrementally (manager); manager->priv->load_id = 0; return FALSE; } static void -queue_load_seat_and_users (ActUserManager *manager) +queue_load_seat (ActUserManager *manager) { if (manager->priv->load_id > 0) { return; } manager->priv->load_id = g_idle_add ((GSourceFunc)load_idle, manager); } static void act_user_manager_get_property (GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) { ActUserManager *manager; manager = ACT_USER_MANAGER (object); switch (prop_id) { case PROP_IS_LOADED: g_value_set_boolean (value, manager->priv->is_loaded); break; case PROP_HAS_MULTIPLE_USERS: g_value_set_boolean (value, manager->priv->has_multiple_users); break; case PROP_INCLUDE_USERNAMES_LIST: g_value_set_pointer (value, manager->priv->include_usernames); break; case PROP_EXCLUDE_USERNAMES_LIST: g_value_set_pointer (value, manager->priv->exclude_usernames); @@ -2820,61 +2825,61 @@ act_user_manager_class_init (ActUserManagerClass *klass) * @user: the #ActUser that changed * * One of the users has changed */ signals [USER_CHANGED] = g_signal_new ("user-changed", G_TYPE_FROM_CLASS (klass), G_SIGNAL_RUN_LAST, G_STRUCT_OFFSET (ActUserManagerClass, user_changed), NULL, NULL, g_cclosure_marshal_VOID__OBJECT, G_TYPE_NONE, 1, ACT_TYPE_USER); g_type_class_add_private (klass, sizeof (ActUserManagerPrivate)); } /** * act_user_manager_queue_load: * @manager: a #ActUserManager * * Queue loading users into user manager. This must be called, and the * #ActUserManager:is-loaded property must be %TRUE before calling * act_user_manager_list_users() */ static void act_user_manager_queue_load (ActUserManager *manager) { g_return_if_fail (ACT_IS_USER_MANAGER (manager)); if (! manager->priv->is_loaded) { - queue_load_seat_and_users (manager); + queue_load_seat (manager); } } static gboolean ensure_accounts_proxy (ActUserManager *manager) { GError *error = NULL; if (manager->priv->accounts_proxy != NULL) { return TRUE; } manager->priv->accounts_proxy = accounts_accounts_proxy_new_sync (manager->priv->connection, G_DBUS_PROXY_FLAGS_NONE, ACCOUNTS_NAME, ACCOUNTS_PATH, NULL, &error); if (error != NULL) { g_debug ("ActUserManager: getting account proxy failed: %s", error->message); g_clear_error (&error); return FALSE; } g_dbus_proxy_set_default_timeout (G_DBUS_PROXY (manager->priv->accounts_proxy), G_MAXINT); g_object_bind_property (G_OBJECT (manager->priv->accounts_proxy), "has-multiple-users", G_OBJECT (manager), "has-multiple-users", -- 2.14.1