Bug #557505 - [bbdb] hangs and is unresponsive

Reverted some parts of bug #599199 and did the same differently

3 files changed, 229 insertions, 58 deletions

diff --git a/plugins/bbdb/bbdb.c b/plugins/bbdb/bbdb.c
index 05fff957fd..a6bba6eec1 100644
--- a/plugins/bbdb/bbdb.c
+++ b/plugins/bbdb/bbdb.c
@@ -105,14 +105,56 @@ find_esource_by_uri (ESourceList *source_list, const gchar *target_uri)
 	return NULL;
 }
 
+/* How often check, in minutes. Read only on plugin enable. Use <= 0 to disable polling. */
+static gint
+get_check_interval (void)
+{
+	GConfClient *gconf;
+	GConfValue *value;
+	gint res = BBDB_BLIST_DEFAULT_CHECK_INTERVAL;
+
+	gconf = gconf_client_get_default ();
+	value = gconf_client_get (gconf, GCONF_KEY_GAIM_CHECK_INTERVAL, NULL);
+
+	if (value) {
+		if (value->type == GCONF_VALUE_INT) {
+			gint interval = gconf_value_get_int (value);
+
+			if (interval > 0)
+				res = interval * 60;
+			else
+				res = interval;
+		}
+
+		gconf_value_free (value);
+	}
+
+	g_object_unref (gconf);
+
+	return res;
+}
+
 gint
 e_plugin_lib_enable (EPlugin *ep, gint enable)
 {
+	static guint update_source = 0;
+
+	if (update_source) {
+		g_source_remove (update_source);
+		update_source = 0;
+	}
+
 	/* Start up the plugin. */
 	if (enable) {
+		gint interval;
+
 		d(fprintf (stderr, "BBDB spinning up...\n"));
 
-		g_idle_add (bbdb_timeout, NULL);
+		g_idle_add (bbdb_timeout, ep);
+
+		interval = get_check_interval ();
+		if (interval > 0)
+			update_source = g_timeout_add_seconds (interval, (GSourceFunc) bbdb_timeout, NULL);
 	}
 
 	return 0;
@@ -124,7 +166,8 @@ bbdb_timeout (gpointer data)
 	if (bbdb_check_gaim_enabled ())
 		bbdb_sync_buddy_list_check ();
 
-	return FALSE;
+	/* not a NULL for a one-time idle check, thus stop it there */
+	return data == NULL;
 }
 
 typedef struct
@@ -149,11 +192,10 @@ G_LOCK_DEFINE_STATIC (todo);
 static gpointer
 bbdb_do_in_thread (gpointer data)
 {
-	EBook *book;
+	EBook *book = data;
 
 	/* Open the addressbook */
-	book = bbdb_open_addressbook (AUTOMATIC_CONTACTS_ADDRESSBOOK);
-	if (book == NULL) {
+	if (!book || !bbdb_open_ebook (book)) {
 		G_LOCK (todo);
 
 		g_slist_foreach (todo, (GFunc)free_todo_struct, NULL);
@@ -205,17 +247,18 @@ bbdb_do_thread (const gchar *name, const gchar *email)
 		todo = g_slist_append (todo, td);
 	} else {
 		GError *error = NULL;
+		EBook *book = bbdb_create_ebook (AUTOMATIC_CONTACTS_ADDRESSBOOK);
 
 		/* list was empty, add item and create a thread */
 		todo = g_slist_append (todo, td);
-		g_thread_create (bbdb_do_in_thread, NULL, FALSE, &error);
+		g_thread_create (bbdb_do_in_thread, book, FALSE, &error);
 
 		if (error) {
 			g_warning ("%s: Creation of the thread failed with error: %s", G_STRFUNC, error->message);
 			g_error_free (error);
 
 			G_UNLOCK (todo);
-			bbdb_do_in_thread (NULL);
+			bbdb_do_in_thread (book);
 			G_LOCK (todo);
 		}
 	}
@@ -370,13 +413,12 @@ bbdb_do_it (EBook *book, const gchar *name, const gchar *email)
 }
 
 EBook *
-bbdb_open_addressbook (gint type)
+bbdb_create_ebook (gint type)
 {
 	GConfClient *gconf;
 	gchar        *uri;
 	EBook       *book = NULL;
 
-	gboolean     status;
 	GError      *error = NULL;
 	gboolean     enable = TRUE;
 	gconf = gconf_client_get_default ();
@@ -409,14 +451,25 @@ bbdb_open_addressbook (gint type)
 		return NULL;
 	}
 
-	status = e_book_open (book, FALSE, &error);
-	if (! status) {
+	return book;
+}
+
+gboolean
+bbdb_open_ebook (EBook *book)
+{
+	GError *error = NULL;
+
+	if (!book)
+		return FALSE;
+
+	if (!e_book_open (book, FALSE, &error)) {
 		g_warning ("bbdb: failed to open addressbook: %s\n", error->message);
 		g_error_free (error);
-		return NULL;
+		g_object_unref (book);
+		return FALSE;
 	}
 
-	return book;
+	return TRUE;
 }
 
 gboolean
diff --git a/plugins/bbdb/bbdb.h b/plugins/bbdb/bbdb.h
index 145da456f9..fe7b998274 100644
--- a/plugins/bbdb/bbdb.h
+++ b/plugins/bbdb/bbdb.h
@@ -22,17 +22,30 @@
 #define __BBDB_H__
 
 /* Where to store the config values */
-#define GCONF_KEY_ENABLE "/apps/evolution/autocontacts/enable_autocontacts"
-#define GCONF_KEY_ENABLE_GAIM "/apps/evolution/autocontacts/auto_sync_gaim"
-#define GCONF_KEY_WHICH_ADDRESSBOOK "/apps/evolution/autocontacts/addressbook_source"
-#define GCONF_KEY_WHICH_ADDRESSBOOK_GAIM "/apps/evolution/autocontacts/gaim_addressbook_source"
-#define GCONF_KEY_GAIM_LAST_SYNC "/apps/evolution/autocontacts/gaim_last_sync_md5"
+#define GCONF_ROOT_PATH			  "/apps/evolution/autocontacts"
+#define GCONF_KEY_ENABLE		  GCONF_ROOT_PATH "/enable_autocontacts"
+#define GCONF_KEY_ENABLE_GAIM		  GCONF_ROOT_PATH "/auto_sync_gaim"
+#define GCONF_KEY_WHICH_ADDRESSBOOK	  GCONF_ROOT_PATH "/addressbook_source"
+#define GCONF_KEY_WHICH_ADDRESSBOOK_GAIM  GCONF_ROOT_PATH "/gaim_addressbook_source"
+#define GCONF_KEY_GAIM_LAST_SYNC_TIME	  GCONF_ROOT_PATH "/gaim_last_sync_time"
+#define GCONF_KEY_GAIM_LAST_SYNC_MD5	  GCONF_ROOT_PATH "/gaim_last_sync_md5"
+#define GCONF_KEY_GAIM_CHECK_INTERVAL	  GCONF_ROOT_PATH "/gaim_check_interval"
+
+/* How often to poll the buddy list for changes (every two minutes is default) */
+#define BBDB_BLIST_DEFAULT_CHECK_INTERVAL (2 * 60)
 
 #define GAIM_ADDRESSBOOK 1
 #define AUTOMATIC_CONTACTS_ADDRESSBOOK 0
 
 /* bbdb.c */
-EBook *bbdb_open_addressbook (gint type);
+/* creates an EBook for a given type (gaim or contacts), but doesn't open it;
+   this function should be called in a main thread. */
+EBook *bbdb_create_ebook (gint type);
+
+/* opens an EBook. Returns false if it fails, and unrefs the book too;
+   this function can be called in any thread */
+gboolean bbdb_open_ebook (EBook *book);
+
 gboolean bbdb_check_gaim_enabled (void);
 
 /* gaimbuddies.c */
diff --git a/plugins/bbdb/gaimbuddies.c b/plugins/bbdb/gaimbuddies.c
index 8e3117608d..d01950b879 100644
--- a/plugins/bbdb/gaimbuddies.c
+++ b/plugins/bbdb/gaimbuddies.c
@@ -90,24 +90,39 @@ void
 bbdb_sync_buddy_list_check (void)
 {
 	GConfClient *gconf;
+	struct stat statbuf;
+	time_t last_sync_time;
 	gchar *md5;
 	gchar *blist_path;
 	gchar *last_sync_str;
 
 	blist_path = get_buddy_filename ();
-	if (!g_file_test (blist_path, G_FILE_TEST_EXISTS)) {
+	if (stat (blist_path, &statbuf) < 0) {
 		g_free (blist_path);
 		return;
 	}
 
-	md5 = get_md5_as_string (blist_path);
-	g_free (blist_path);
-
 	/* Reprocess the buddy list if it's been updated. */
 	gconf = gconf_client_get_default ();
-	last_sync_str = gconf_client_get_string (gconf, GCONF_KEY_GAIM_LAST_SYNC, NULL);
+	last_sync_str = gconf_client_get_string (gconf, GCONF_KEY_GAIM_LAST_SYNC_TIME, NULL);
+	if (last_sync_str == NULL || ! strcmp ((const gchar *)last_sync_str, ""))
+		last_sync_time = (time_t) 0;
+	else
+		last_sync_time = (time_t) g_ascii_strtoull (last_sync_str, NULL, 10);
+
+	g_free (last_sync_str);
+
+	if (statbuf.st_mtime <= last_sync_time) {
+		g_object_unref (G_OBJECT (gconf));
+		g_free (blist_path);
+		return;
+	}
+
+	last_sync_str = gconf_client_get_string (gconf, GCONF_KEY_GAIM_LAST_SYNC_MD5, NULL);
 	g_object_unref (G_OBJECT (gconf));
 
+	md5 = get_md5_as_string (blist_path);
+
 	if (!last_sync_str || !*last_sync_str || !g_str_equal (md5, last_sync_str)) {
 		fprintf (stderr, "bbdb: Buddy list has changed since last sync.\n");
 
@@ -115,42 +130,83 @@ bbdb_sync_buddy_list_check (void)
 	}
 
 	g_free (last_sync_str);
+	g_free (blist_path);
 	g_free (md5);
 }
 
-void
-bbdb_sync_buddy_list (void)
+static gboolean
+store_last_sync_idle_cb (gpointer data)
 {
-	GList       *blist, *l;
-	EBook       *book = NULL;
+	GConfClient *gconf;
+	gchar *md5;
+	gchar *blist_path = get_buddy_filename ();
+	time_t last_sync;
+	gchar *last_sync_time;
 
-	/* Get the Gaim buddy list */
-	blist = bbdb_get_gaim_buddy_list ();
-	if (blist == NULL)
-		return;
+	time (&last_sync);
+	last_sync_time = g_strdup_printf ("%ld", (glong) last_sync);
 
-	/* Open the addressbook */
-	book = bbdb_open_addressbook (GAIM_ADDRESSBOOK);
-	if (book == NULL) {
-		free_buddy_list (blist);
-		return;
+	md5 = get_md5_as_string (blist_path);
+
+	gconf = gconf_client_get_default ();
+	gconf_client_set_string (gconf, GCONF_KEY_GAIM_LAST_SYNC_TIME, last_sync_time, NULL);
+	gconf_client_set_string (gconf, GCONF_KEY_GAIM_LAST_SYNC_MD5, md5, NULL);
+
+	g_object_unref (G_OBJECT (gconf));
+
+	g_free (last_sync_time);
+	g_free (blist_path);
+	g_free (md5);
+
+	return FALSE;
+}
+
+static gboolean syncing = FALSE;
+G_LOCK_DEFINE_STATIC (syncing);
+
+struct sync_thread_data
+{
+	GList *blist;
+	EBook *book;
+};
+
+static gpointer
+bbdb_sync_buddy_list_in_thread (gpointer data)
+{
+	GList *l;
+	struct sync_thread_data *std = data;
+
+	g_return_val_if_fail (std != NULL, NULL);
+
+	if (!bbdb_open_ebook (std->book)) {
+		/* book got freed in bbdb_open_ebook on a failure */
+		free_buddy_list (std->blist);
+		g_free (std);
+
+		G_LOCK (syncing);
+		syncing = FALSE;
+		G_UNLOCK (syncing);
+
+		return NULL;
 	}
 
 	printf ("bbdb: Synchronizing buddy list to contacts...\n");
 	/* Walk the buddy list */
-	for (l = blist; l != NULL; l = l->next) {
+	for (l = std->blist; l != NULL; l = l->next) {
 		GaimBuddy *b = l->data;
 		EBookQuery *query;
 		GList *contacts;
 		GError *error = NULL;
 		EContact *c;
 
-		if (b->alias == NULL || strlen (b->alias) == 0)
-			b->alias = b->account_name;
+		if (b->alias == NULL || strlen (b->alias) == 0) {
+			g_free (b->alias);
+			b->alias = g_strdup (b->account_name);
+		}
 
 		/* Look for an exact match full name == buddy alias */
 		query = e_book_query_field_test (E_CONTACT_FULL_NAME, E_BOOK_QUERY_IS, b->alias);
-		e_book_get_contacts (book, query, &contacts, NULL);
+		e_book_get_contacts (std->book, query, &contacts, NULL);
 		e_book_query_unref (query);
 		if (contacts != NULL) {
 
@@ -162,11 +218,11 @@ bbdb_sync_buddy_list (void)
 
 			c = E_CONTACT (contacts->data);
 
-			if (! bbdb_merge_buddy_to_contact (book, b, c))
+			if (!bbdb_merge_buddy_to_contact (std->book, b, c))
 				continue;
 
 			/* Write it out to the addressbook */
-			if (! e_book_commit_contact (book, c, &error)) {
+			if (!e_book_commit_contact (std->book, c, &error)) {
 				g_warning ("bbdb: Could not modify contact: %s\n", error->message);
 				g_error_free (error);
 			}
@@ -176,34 +232,83 @@ bbdb_sync_buddy_list (void)
 		/* Otherwise, create a new contact. */
 		c = e_contact_new ();
 		e_contact_set (c, E_CONTACT_FULL_NAME, (gpointer) b->alias);
-		if (! bbdb_merge_buddy_to_contact (book, b, c)) {
+		if (!bbdb_merge_buddy_to_contact (std->book, b, c)) {
 			g_object_unref (G_OBJECT (c));
 			continue;
 		}
 
-		if (! e_book_add_contact (book, c, &error)) {
+		if (! e_book_add_contact (std->book, c, &error)) {
 			g_warning ("bbdb: Failed to add new contact: %s\n", error->message);
 			g_error_free (error);
-			return;
+			goto finish;
 		}
 		g_object_unref (G_OBJECT (c));
 
 	}
 
-	/* Update the last-sync'd time */
-	{
-		GConfClient *gconf;
-		gchar *md5;
-		gchar *blist_path = get_buddy_filename ();
+	g_idle_add (store_last_sync_idle_cb, NULL);
 
-		md5 = get_md5_as_string (blist_path);
-		gconf = gconf_client_get_default ();
-		gconf_client_set_string (gconf, GCONF_KEY_GAIM_LAST_SYNC, md5, NULL);
-		g_object_unref (G_OBJECT (gconf));
-		g_free (md5);
-		g_free (blist_path);
-	}
+ finish:
 	printf ("bbdb: Done syncing buddy list to contacts.\n");
+
+	g_object_unref (std->book);
+	free_buddy_list (std->blist);
+	g_free (std);
+
+	G_LOCK (syncing);
+	syncing = FALSE;
+	G_UNLOCK (syncing);
+
+	return NULL;
+}
+
+void
+bbdb_sync_buddy_list (void)
+{
+	GList *blist;
+	GError *error = NULL;
+	EBook *book = NULL;
+	struct sync_thread_data *std;
+
+	G_LOCK (syncing);
+	if (syncing) {
+		G_UNLOCK (syncing);
+		printf ("bbdb: Already syncing buddy list, skipping this call\n");
+		return;
+	}
+
+	/* Get the Gaim buddy list */
+	blist = bbdb_get_gaim_buddy_list ();
+	if (blist == NULL) {
+		G_UNLOCK (syncing);
+		return;
+	}
+
+	/* Open the addressbook */
+	book = bbdb_create_ebook (GAIM_ADDRESSBOOK);
+	if (book == NULL) {
+		free_buddy_list (blist);
+		G_UNLOCK (syncing);
+		return;
+	}
+
+	std = g_new0 (struct sync_thread_data, 1);
+	std->blist = blist;
+	std->book = book;
+
+	syncing = TRUE;
+
+	g_thread_create (bbdb_sync_buddy_list_in_thread, std, FALSE, &error);
+	if (error) {
+		g_warning ("%s: Creation of the thread failed with error: %s", G_STRFUNC, error->message);
+		g_error_free (error);
+
+		G_UNLOCK (syncing);
+		bbdb_sync_buddy_list_in_thread (std);
+		G_LOCK (syncing);
+	}
+
+	G_UNLOCK (syncing);
 }
 
 static gboolean