Load all users only when really needed

LightDM internally maintains a list of all users in the "system". This
is problematic when the user database is on a remote LDAP server and
contains thousands of users, because loading all of them can block
lightdm for minutes. This blocking seems to happen at least three times:
1) when lightdm daemon is started, 2) when a lightdm
child (seat/session?) is started and 3) when a user moves focus in the
greeter (gtk) from username to password field.

This patch eliminates long blocking by loading the users on
demand (using getpwnam() instead of getpwent()). The whole test suite
passes, but I'm afraid that some functionality could be broken. Feedback
welcome.

diff --git a/common/user-list.c b/common/user-list.c
index 58cb0284001525f03ebcd30a9f17ae23998d90ec..d5d95e208e32ee7f904afcdfd590f74843a76a85 100644 (file)
--- a/common/user-list.c
+++ b/common/user-list.c
@@ -945,8 +945,6 @@ load_users (CommonUserList *user_list)
         g_dbus_connection_signal_unsubscribe (priv->bus, priv->user_removed_signal);
         priv->user_removed_signal = 0;
 
-        load_passwd_file (user_list, FALSE);
-
         /* Watch for changes to user list */
 
         passwd_file = g_file_new_for_path (PASSWD_FILE);
@@ -1008,6 +1006,7 @@ common_user_list_get_user_by_name (CommonUserList *user_list, const gchar *usern
     g_return_val_if_fail (COMMON_IS_USER_LIST (user_list), NULL);
     g_return_val_if_fail (username != NULL, NULL);
 
+    /* Load users from AccountsService, do nothing when it does not run */
     load_users (user_list);
 
     CommonUser *user = get_user_by_name (user_list, username);

diff --git a/liblightdm-gobject/user.c b/liblightdm-gobject/user.c
index e47d563be7932e8ca2220d8d951df76d62c480f2..75f65438e8fc6799cb0e05ed715dad16c1789054 100644 (file)
--- a/liblightdm-gobject/user.c
+++ b/liblightdm-gobject/user.c
@@ -147,7 +147,7 @@ user_list_removed_cb (CommonUserList *common_list, CommonUser *common_user, Ligh
 }
 
 static void
-initialize_user_list_if_needed (LightDMUserList *user_list)
+initialize_user_list_if_needed (LightDMUserList *user_list, const gchar *username)
 {
     LightDMUserListPrivate *priv = GET_LIST_PRIVATE (user_list);
     GList *common_users;
@@ -156,21 +156,30 @@ initialize_user_list_if_needed (LightDMUserList *user_list)
     if (priv->initialized)
         return;
 
-    common_users = common_user_list_get_users (common_user_list_get_instance ());
-    for (link = common_users; link; link = link->next)
+    if (!username)
     {
-        CommonUser *user = link->data;
+        common_users = common_user_list_get_users (common_user_list_get_instance ());
+        for (link = common_users; link; link = link->next)
+        {
+            CommonUser *user = link->data;
+            LightDMUser *lightdm_user = wrap_common_user (user);
+            priv->lightdm_list = g_list_prepend (priv->lightdm_list, lightdm_user);
+        }
+        priv->lightdm_list = g_list_reverse (priv->lightdm_list);
+
+        priv->initialized = TRUE;
+    }
+    else
+    {
+        CommonUser *user = common_user_list_get_user_by_name (common_user_list_get_instance (), username);
         LightDMUser *lightdm_user = wrap_common_user (user);
-        priv->lightdm_list = g_list_prepend (priv->lightdm_list, lightdm_user);
+        priv->lightdm_list = g_list_append (priv->lightdm_list, lightdm_user);
     }
-    priv->lightdm_list = g_list_reverse (priv->lightdm_list);
 
     CommonUserList *common_list = common_user_list_get_instance ();
     g_signal_connect (common_list, USER_LIST_SIGNAL_USER_ADDED, G_CALLBACK (user_list_added_cb), user_list);
     g_signal_connect (common_list, USER_LIST_SIGNAL_USER_CHANGED, G_CALLBACK (user_list_changed_cb), user_list);
     g_signal_connect (common_list, USER_LIST_SIGNAL_USER_REMOVED, G_CALLBACK (user_list_removed_cb), user_list);
-
-    priv->initialized = TRUE;
 }
 
 /**
@@ -183,7 +192,7 @@ gint
 lightdm_user_list_get_length (LightDMUserList *user_list)
 {
     g_return_val_if_fail (LIGHTDM_IS_USER_LIST (user_list), 0);
-    initialize_user_list_if_needed (user_list);
+    initialize_user_list_if_needed (user_list, NULL);
     return g_list_length (GET_LIST_PRIVATE (user_list)->lightdm_list);
 }
 
@@ -200,7 +209,7 @@ GList *
 lightdm_user_list_get_users (LightDMUserList *user_list)
 {
     g_return_val_if_fail (LIGHTDM_IS_USER_LIST (user_list), NULL);
-    initialize_user_list_if_needed (user_list);
+    initialize_user_list_if_needed (user_list, NULL);
     return GET_LIST_PRIVATE (user_list)->lightdm_list;
 }
 
@@ -221,7 +230,7 @@ lightdm_user_list_get_user_by_name (LightDMUserList *user_list, const gchar *use
     g_return_val_if_fail (LIGHTDM_IS_USER_LIST (user_list), NULL);
     g_return_val_if_fail (username != NULL, NULL);
 
-    initialize_user_list_if_needed (user_list);
+    initialize_user_list_if_needed (user_list, username);
 
     for (link = GET_LIST_PRIVATE (user_list)->lightdm_list; link; link = link->next)
     {