From b1ec3114a49c9e5331a4f7c106a58867499ac1ce Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
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