[gdm-list] [PATCH] fix race condition when setting the model for the face browser

[Ludwig Nussel <ludwig nussel suse de>]

Hi,

Ok, so this was driving me nuts the whole day until I understood
what's going on. r4381 changed the way how the face browser gets
constructed. Previously the treeview got an empty model and then the
model was populated with user names. Now the model is populated
first and then attached to the treeview. Looks like this causes the
selection's "changed" signal to be emmited which in turn triggers an
interruption message to the slave process. It is treated as a reply
to GDM_NEEDPIC there. Sometimes (race condition?) the real reply for
GDM_NEEDPIC then ends up as reply for the username prompt which
causes the first pam conversation to fail.

Attached patch fixes that by not populating the model before it has
been attached to the treeview again.

While debuggint that problem I saw two problems:

- There is no code in place that prevents interruption messages
  (those with BEL) at the wrong time.
- The way pictures are transported into the greeter is really ugly.
  IMHO the greeter could just as well open the files itself. There
  is no need to let the slave process do it.

cu
Ludwig

-- 
 (o_   Ludwig Nussel
 //\   SUSE LINUX Products GmbH, Development
 V_/_  http://www.suse.de/


Index: gdm2/gui/gdmlogin.c
===================================================================
--- gdm2.orig/gui/gdmlogin.c
+++ gdm2/gui/gdmlogin.c
@@ -1999,17 +1999,11 @@ process_operation (guchar       op_code,
 }
 
 
-static int
+static void
 gdm_login_browser_populate (void)
 {
     GList *li;
-    int i = 0;
 
-    /* Create browser_model before calling gdm_login_browser_populate */
-       browser_model = (GtkTreeModel *)gtk_list_store_new (3,
-							   GDK_TYPE_PIXBUF,
-							   G_TYPE_STRING,
-							   G_TYPE_STRING);
     for (li = users; li != NULL; li = li->next) {
 	    GdmUser *usr = li->data;
 	    GtkTreeIter iter = {0};
@@ -2032,9 +2026,8 @@ gdm_login_browser_populate (void)
 				GREETER_ULIST_LABEL_COLUMN, label,
 				-1);
 	    g_free (label);
-	    i++;
     }
-    return (i);
+    return;
 }
 
 static void
@@ -2592,6 +2585,12 @@ gdm_login_gui_init (void)
 			      G_CALLBACK (browser_change_focus),
 			      NULL);
 
+	    /* Create browser_model before calling gdm_login_browser_populate */
+	   browser_model = (GtkTreeModel *)gtk_list_store_new (3,
+							   GDK_TYPE_PIXBUF,
+							   G_TYPE_STRING,
+							   G_TYPE_STRING);
+
 	    gtk_tree_view_set_model (GTK_TREE_VIEW (browser), browser_model);
 	    column = gtk_tree_view_column_new_with_attributes
 		    (_("Icon"),
@@ -3574,18 +3573,16 @@ main (int argc, char *argv[])
 	    }
     }
 
-    if (browser_ok && gdm_config_get_bool (GDM_KEY_BROWSER)) {
-	/*
-	 * Do not display face browser widget if no users, check this
-	 * before callign gdm_login_gui_init ()
-	 */
-	int num_users = gdm_login_browser_populate ();
-	if (num_users == 0)
-		browser_ok = FALSE;
-    }
+    /* Do not display face browser widget if no users */
+    if(!users)
+	browser_ok = FALSE;
 
     gdm_login_gui_init ();
 
+    if (browser_ok && gdm_config_get_bool (GDM_KEY_BROWSER)) {
+	gdm_login_browser_populate ();
+    }
+
     ve_signal_add (SIGHUP, gdm_reread_config, NULL);
 
     hup.sa_handler = ve_signal_notify;