GNOME.org
 

IDLE as a thread

This is a first implementation of Push E-mail 'outside of the mainloop'

There are some racy things when expunging a lot of items versus the tree
model (because right now, they are removed in a thread, not in the
mainloop anymore. So this was always wrong, but now it also causes the
race condition to actually occur, whereas when with the mainloop, the
model just didn't yet redraw during the time things where being removed
- because it was blocking -)
 
I need to, therefore, fix some things before I'm applying this.

Flag changes now also work excellent. So removing (without expunging)
and setting a message's status to read from another E-mail client:
works.

The ui block moment that I described on Friday, is of course also gone.
Even with a truly high latency and or slow network, there will be no
delay caused by the IDLE things, on the ui redrawing handling.

I also rewrote most of the locking things. The original got horrible ...

I've let the idle things run without any delay to test the locking. It
seemed to work just fine with both the delays on and with the delays off
(with these same delays off, the current/original support for IDLE just
blocks everything in the ui constantly and made everything not
workable).



-- 
Philip Van Hoof, software developer
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
http://www.pvanhoof.be/blog




Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c	(revision 2109)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c	(working copy)
@@ -164,10 +164,14 @@
 static void 
 let_idle_die (CamelImapStore *imap_store, gboolean connect_buz)
 {
-	if (imap_store->idle_signal > 0) 
-		g_source_remove (imap_store->idle_signal);
 
+	imap_store->idle_cont = FALSE;
+	if (imap_store->idle_thread)
+		g_thread_join (imap_store->idle_thread);
+
 	g_static_rec_mutex_lock (imap_store->idle_prefix_lock);
+	g_static_rec_mutex_lock (imap_store->idle_lock);
+
 	if (imap_store->idle_prefix)
 	{
 		g_free (imap_store->idle_prefix); 
@@ -175,8 +179,11 @@
 		idle_debug ("Sending DONE in let_idle_die\n");
 		camel_stream_printf (imap_store->ostream, "DONE\r\n");
 	}
+
+	g_static_rec_mutex_unlock (imap_store->idle_lock);
 	g_static_rec_mutex_unlock (imap_store->idle_prefix_lock);
 
+	return;
 }
 
 void
@@ -186,6 +193,12 @@
 		camel_imap_folder_stop_idle (store->current_folder);
 	else {
 		g_static_rec_mutex_lock (store->idle_prefix_lock);
+		g_static_rec_mutex_lock (store->idle_lock);
+
+		store->idle_cont = FALSE;
+		if (store->idle_thread)
+			g_thread_join (store->idle_thread);
+
 		if (store->idle_prefix) 
 		{
 			int nwritten=0;
@@ -196,6 +209,8 @@
 			g_free (store->idle_prefix);
 			store->idle_prefix = NULL;
 		}
+
+		g_static_rec_mutex_unlock (store->idle_lock);
 		g_static_rec_mutex_unlock (store->idle_prefix_lock);
 	}
 }
@@ -219,9 +234,9 @@
 		CAMEL_STORE_CLASS (camel_imap_store_class);
 	CamelDiscoStoreClass *camel_disco_store_class =
 		CAMEL_DISCO_STORE_CLASS (camel_imap_store_class);
-	
+
 	parent_class = CAMEL_DISCO_STORE_CLASS (camel_type_get_global_classfuncs (camel_disco_store_get_type ()));
-	
+
 	/* virtual method overload */
 	camel_object_class->setv = imap_setv;
 	camel_object_class->getv = imap_getv;
@@ -280,7 +295,7 @@
 		camel_store_summary_save((CamelStoreSummary *)imap_store->summary);
 		camel_object_unref(imap_store->summary);
 	}
-	
+
 	if (imap_store->base_url)
 		g_free (imap_store->base_url);
 	if (imap_store->storage_path)
@@ -293,6 +308,10 @@
 
 	g_static_rec_mutex_free (imap_store->idle_prefix_lock);
 	imap_store->idle_prefix_lock = NULL;
+
+	g_static_rec_mutex_free (imap_store->idle_lock);
+	imap_store->idle_lock = NULL;
+
 }
 
 static void
@@ -303,9 +322,15 @@
 	imap_store->idle_prefix_lock = g_new0 (GStaticRecMutex, 1);
 	g_static_rec_mutex_init (imap_store->idle_prefix_lock);
 
+	imap_store->idle_lock = g_new0 (GStaticRecMutex, 1);
+	g_static_rec_mutex_init (imap_store->idle_lock);
+
 	imap_store->dontdistridlehack = FALSE;
-	imap_store->idle_signal = 0;
+
+	imap_store->idle_cont = FALSE;
+	imap_store->idle_thread = NULL;
 	imap_store->idle_prefix = NULL;
+
 	imap_store->istream = NULL;
 	imap_store->ostream = NULL;
 	imap_store->has_login = FALSE;
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.h
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.h	(revision 2109)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.h	(working copy)
@@ -158,9 +158,11 @@
 	
 	time_t refresh_stamp;
 	gchar *idle_prefix;
-	guint idle_signal;
 	gboolean dontdistridlehack, has_login;
-	GStaticRecMutex *idle_prefix_lock;
+
+	GStaticRecMutex *idle_prefix_lock, *idle_lock;
+	GThread *idle_thread;
+	gboolean idle_cont, in_idle;
 };
 
 typedef struct {
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c	(revision 2109)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c	(working copy)
@@ -657,9 +657,6 @@
 
 	imap_folder->stopping = TRUE;
 
-	if (imap_folder->idle_signal > 0) 
-		g_source_remove (imap_folder->idle_signal);
-
 	if (!imap_folder->in_idle || imap_folder->idle_lock != NULL)
 	{
 		g_static_rec_mutex_free (imap_folder->idle_lock);
@@ -3406,12 +3403,13 @@
 	char *resp;
 	CamelException ex = CAMEL_EXCEPTION_INITIALISER;
 
-	idle_debug ("idle_real_start\n");
-
 	if (store->ostream && store->istream && CAMEL_IS_STREAM (store->ostream))
 	{
 		store->idle_prefix = g_strdup_printf ("%c%.5u", 
 			store->tag_prefix, store->command++);
+
+		idle_debug ("(.., ..) -> %s IDLE | in idle_real_start\n", store->idle_prefix);
+
 		camel_stream_printf (store->ostream, "%s IDLE\r\n",
 			store->idle_prefix);
 	} else {
@@ -3442,7 +3440,7 @@
 			tbreak = TRUE;
 		if (strcasestr (resp, "BAD") != NULL)
 			tbreak = TRUE;
-		/* printf ("-> %s\n", resp); */
+		idle_debug ("(.., ..) <- %s | in idle_real_start\n", resp);
 		g_free (resp); resp=NULL;
 		if (tbreak)
 			break;
@@ -3467,16 +3465,8 @@
 
   idle_debug ("idle_deal_with_stuff\n");
 
-  if (!folder || !CAMEL_IS_IMAP_FOLDER (folder))
-	return NULL;
-  if (!store || !CAMEL_IS_IMAP_STORE (store))
-	return NULL;
   if (!camel_disco_store_check_online ((CamelDiscoStore*)store, &ex))
 	return NULL;
-  if (store->istream == NULL || ((CamelObject *)store->istream)->ref_count <= 0)
-	return NULL;
-  if (store->ostream == NULL || ((CamelObject *)store->istream)->ref_count <= 0)
-	return NULL;
 
   hlock = CAMEL_SERVICE_REC_TRYLOCK (store, connect_lock);
 
@@ -3484,6 +3474,8 @@
   {
 	if (store->current_folder)
 	{
+		/* We read-away everything non-blocking and process it */
+
 		resp = NULL;
 		while (camel_imap_store_readline_nb (store, &resp, &ex) > 0)
 		{
@@ -3496,17 +3488,12 @@
 					idle_resp = idle_response_new (folder);
 				read_idle_response (folder, resp, idle_resp);
 			}
+			idle_debug ("(.., ..) <- %s | in idle_deal_with_stuff at nb\n", resp);
 			g_free (resp); resp=NULL;
 		}
 		if (resp)
 			g_free (resp);
 
-		if (!camel_disco_store_check_online ((CamelDiscoStore*)store, &ex))
-			return NULL;
-		if (store->istream == NULL || ((CamelObject *)store->istream)->ref_count <= 0)
-			return NULL;
-		if (store->ostream == NULL || ((CamelObject *)store->istream)->ref_count <= 0)
-			return NULL;
 
 		/* Here we force the server to tell us about the changes: */
 
@@ -3518,7 +3505,7 @@
 		 * commands. */
 
 		if (store->ostream && CAMEL_IS_STREAM (store->ostream)) {
-			idle_debug ("Sending DONE in idle_deal_with_stuff (nb)\n");
+			idle_debug ("(.., ..) -> DONE | Sending DONE in idle_deal_with_stuff (nb)\n");
 			nwritten = camel_stream_printf (store->ostream, "DONE\r\n");
 		}
 
@@ -3537,6 +3524,7 @@
 					idle_resp = idle_response_new (folder);
 				read_idle_response (folder, resp, idle_resp);
 			}
+			idle_debug ("(.., ..) <- %s | in idle_deal_with_stuff at idle\n", resp);
 			g_free (resp); resp=NULL;
 		}
 
@@ -3547,21 +3535,19 @@
 			g_free (resp);
 
 	} else {
-		if (!camel_disco_store_check_online ((CamelDiscoStore*)store, &ex))
-			return NULL;
-		if (store->istream == NULL || ((CamelObject *)store->istream)->ref_count <= 0)
-			return NULL;
-		if (store->ostream == NULL || ((CamelObject *)store->istream)->ref_count <= 0)
-			return NULL;
+		/* Trying to deal while the current folder is gone: just read away everything */
 		if (store->ostream && CAMEL_IS_STREAM (store->ostream)) {
-			idle_debug ("Sending DONE in idle_deal_with_stuff (b)\n");
+			idle_debug ("(.., ..) -> DONE | Sending DONE in idle_deal_with_stuff (b)\n");
 			nwritten = camel_stream_printf (store->ostream, "DONE\r\n");
 		}
 		if (nwritten == -1) 
 			goto outofhere;
 		resp = NULL;
 		while ((type = camel_imap_command_response_idle (store, &resp, &ex)) == CAMEL_IMAP_RESPONSE_UNTAGGED) 
-			{ g_free (resp); resp=NULL; }
+		{
+			idle_debug ("(.., ..) <- %s | in idle_deal_with_stuff in else\n", resp); 
+			g_free (resp); resp=NULL; 
+		}
 		if (resp)
 			g_free (resp);
 	}
@@ -3586,121 +3572,101 @@
 
 	idle_debug ("camel_imap_folder_stop_idle\n");
 
-	if (!folder || !CAMEL_IS_IMAP_FOLDER (folder))
-		return;
-
 	store = CAMEL_IMAP_STORE (folder->parent_store);
 
-	if (!store || !CAMEL_IS_IMAP_STORE (store))
-		return;
 	if (!camel_disco_store_check_online ((CamelDiscoStore*)store, &ex))
 		return;
-	if (store->istream == NULL || ((CamelObject *)store->istream)->ref_count <= 0)
-		return;
-	if (store->ostream == NULL || ((CamelObject *)store->istream)->ref_count <= 0)
-		return;
 
-	g_static_rec_mutex_lock (((CamelImapFolder *)folder)->idle_lock);
-
+	g_static_rec_mutex_lock (store->idle_lock);
 	g_static_rec_mutex_lock (store->idle_prefix_lock);
+
 	if ((store->capabilities & IMAP_CAPABILITY_IDLE) && store->idle_prefix)
 	{
 		gboolean had_lock = FALSE;
+
+		store->idle_cont = FALSE;
+		if (store->idle_thread)
+			g_thread_join (store->idle_thread);
+
 		g_free (store->idle_prefix);
 		store->idle_prefix = NULL;
 
-		if (store->idle_signal > 0) 
-			g_source_remove (store->idle_signal);
-
 		idle_resp = idle_deal_with_stuff (folder, store, &had_err, &had_lock);
 
 		/* Outside of the lock of course */
 		if (idle_resp && !had_err)
 			process_idle_response (idle_resp);
 
-		/* idle_real_start (store); */
-
 		if (idle_resp) 
 			idle_response_free (idle_resp);
 	}
-	g_static_rec_mutex_unlock (store->idle_prefix_lock);
 
-	g_static_rec_mutex_unlock (((CamelImapFolder *)folder)->idle_lock);
+	g_static_rec_mutex_unlock (store->idle_prefix_lock);
+	g_static_rec_mutex_unlock (store->idle_lock);
 }
 
-static void
-idle_timeout_checker_destroy (gpointer data)
-{
-	idle_debug ("idle_timeout_checker_destroy\n");
 
-	return;
-}
-
-static gboolean 
-idle_timeout_checker (gpointer data)
+static gpointer 
+idle_thread (gpointer data)
 {
 	CamelFolder *folder = (CamelFolder *) data;
 	CamelImapFolder *imap_folder;
 	CamelImapStore *store;
 	gboolean had_err = FALSE, hadlock = FALSE;
-	CamelException ex = CAMEL_EXCEPTION_INITIALISER;
 
-	((CamelImapFolder *)folder)->in_idle = TRUE;
+	idle_debug ("idle_thread\n");
 
-	idle_debug ("idle_timeout_checker\n");
+	store = CAMEL_IMAP_STORE (folder->parent_store);
+	imap_folder = CAMEL_IMAP_FOLDER (folder);
 
-	if ((!folder) || ((((CamelObject *)data)->ref_count <= 0) && (!CAMEL_IS_IMAP_FOLDER (folder))))
-		return FALSE;
+	if (!imap_folder->do_push_email) {
+		idle_debug ("Folder set not to do idle\n");
+		return NULL;
+	}
 
-	store = CAMEL_IMAP_STORE (folder->parent_store);
-	if (!(store->capabilities & IMAP_CAPABILITY_IDLE))
-		return FALSE;
-	if (!camel_disco_store_check_online ((CamelDiscoStore*)store, &ex))
-		return FALSE;
-	if (store->istream == NULL || ((CamelObject *)store->istream)->ref_count <= 0)
-		return FALSE;
-	if (store->ostream == NULL || ((CamelObject *)store->istream)->ref_count <= 0)
-		return FALSE;
+	if (!(store->capabilities & IMAP_CAPABILITY_IDLE)) {
+		idle_debug ("Server has no IDLE capabilities\n");
+		return NULL;
+	}
 
-	g_static_rec_mutex_lock (((CamelImapFolder *)folder)->idle_lock);
+	idle_debug ("idle_thread starting (%s)\n", store->idle_prefix?store->idle_prefix:"(none)");
 
+	store->idle_cont = TRUE;
 
-	imap_folder = CAMEL_IMAP_FOLDER (folder);
-	if (!imap_folder->do_push_email)
+	while (store->idle_cont && store->idle_prefix != NULL)
 	{
-		g_static_rec_mutex_unlock (((CamelImapFolder *)folder)->idle_lock);
-		return FALSE;
-	}
+		g_static_rec_mutex_lock (store->idle_lock);
+		g_static_rec_mutex_lock (store->idle_prefix_lock);
 
-	g_static_rec_mutex_lock (store->idle_prefix_lock);
-	if (store->idle_prefix != NULL)
-	{
-		IdleResponse *idle_resp = idle_deal_with_stuff (folder, store, &had_err,&hadlock);
+		store->in_idle = TRUE;
+		if (store->idle_prefix != NULL)
+		{
+			IdleResponse *idle_resp = idle_deal_with_stuff (folder, store, &had_err,&hadlock);
 
-		if (idle_resp && !had_err)
-			process_idle_response (idle_resp);
+			if (idle_resp && !had_err)
+				process_idle_response (idle_resp);
 
-		if (hadlock)
-			idle_real_start (store);
+			if (hadlock)
+				idle_real_start (store);
 
-		if (idle_resp)
-			idle_response_free (idle_resp);
-	}
-	g_static_rec_mutex_unlock (store->idle_prefix_lock);
+			if (idle_resp)
+				idle_response_free (idle_resp);
+		}
 
-	g_static_rec_mutex_unlock (((CamelImapFolder *)folder)->idle_lock);
+		g_static_rec_mutex_unlock (store->idle_prefix_lock);
+		g_static_rec_mutex_unlock (store->idle_lock);
 
-	if (((CamelImapFolder *)folder)->stopping)
-	{
-		g_static_rec_mutex_free (((CamelImapFolder *)folder)->idle_lock);
-		((CamelImapFolder *)folder)->idle_lock = NULL;
-		((CamelImapFolder *)folder)->in_idle = FALSE;
-		return FALSE;
+		idle_debug ("idle checked in idle_thrad\n");
+
+		usleep (5000000);
 	}
 
-	((CamelImapFolder *)folder)->in_idle = FALSE;
+	store->in_idle = FALSE;
 
-	return TRUE;
+	store->idle_thread = NULL;
+
+	g_thread_exit (NULL);
+	return NULL;
 }
 
 
@@ -3708,25 +3674,16 @@
 camel_imap_folder_start_idle (CamelFolder *folder)
 {
 	CamelImapStore *store;
-	CamelImapFolder *imap_folder = (CamelImapFolder *) folder;
 	CamelException ex = CAMEL_EXCEPTION_INITIALISER;
 
 	idle_debug ("camel_imap_folder_start_idle\n");
 
-	if (!folder || !CAMEL_IS_IMAP_FOLDER (folder))
-		return;
-
 	store = CAMEL_IMAP_STORE (folder->parent_store);
-	if (!store || !CAMEL_IS_IMAP_STORE (store))
-		return;
+
 	if (!camel_disco_store_check_online ((CamelDiscoStore*)store, &ex))
 		return;
-	if (store->istream == NULL || ((CamelObject *)store->istream)->ref_count <= 0)
-		return;
-	if (store->ostream == NULL || ((CamelObject *)store->istream)->ref_count <= 0)
-		return;
 
-	g_static_rec_mutex_lock (((CamelImapFolder *)folder)->idle_lock);
+	g_static_rec_mutex_lock (store->idle_lock);
 
 	if (store->capabilities & IMAP_CAPABILITY_IDLE)
 	{
@@ -3735,18 +3692,17 @@
 		{
 			folder->folder_flags |= CAMEL_FOLDER_HAS_PUSHEMAIL_CAPABILITY;
 
-			idle_real_start (store);
+			if (!store->idle_thread) {
+				idle_real_start (store);
+				store->idle_thread = g_thread_create (idle_thread, 
+					folder, TRUE, NULL);
+			}
 
-			store->idle_signal = g_timeout_add_full (G_PRIORITY_DEFAULT_IDLE, 5 * 1000,
-				idle_timeout_checker, folder, idle_timeout_checker_destroy);
-
-			imap_folder->idle_signal = store->idle_signal;
 		}
 		g_static_rec_mutex_unlock (store->idle_prefix_lock);
 	}
 
-	g_static_rec_mutex_unlock (((CamelImapFolder *)folder)->idle_lock);
-
+	g_static_rec_mutex_unlock (store->idle_lock);
 }
 
 
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.h
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.h	(revision 2109)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.h	(working copy)
@@ -50,8 +50,9 @@
 	unsigned int need_refresh:1;
 	unsigned int read_only:1;
 	gchar *folder_dir;
+
 	gboolean do_push_email, stopping, in_idle;
-	guint idle_signal, gmsgstore_signal;
+	guint gmsgstore_signal;
 	GStaticRecMutex *idle_lock;
 
 	CamelImapStore *gmsgstore;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]