Re: Some IDLE improvements



On Tue, 2007-10-23 at 18:51 +0200, Philip Van Hoof wrote:
> On Tue, 2007-10-23 at 15:13 +0200, Philip Van Hoof wrote:
> 
> And another version :)

And another version. I have done quite a lot of testing with Modest and
the demoui using this version. It also fixes the problem that previous
versions of this patch had: that not all responses in IDLE where handled
and that we where seeing unsolicited responses.

The reason was because non-blocking read in OpenSSL on the device seems
to behave quite strange (there is stuff it can read, as I'm seeing it
being retrieved in wireshark, but SSL_read() is not fetching it).

I first tried to figure out how SSL_read() is supposed to be called when
doing a non-blocking read. But all that seemed alright.

As a solution I tried to just cope with that by reading away what is
still available after the DONE is sent. Surprisingly does OpenSSL on the
device read the content as soon as you SSL_write() something to the
socket. Magic at work, I guess (or .. strange OpenSSL at work).

There was one problem with that: The CamelStreamBuffer didn't cope with
non-blocking reads correctly, so it left a small part of the last read
in its own buffer. This makes camel_imap_store_readline put that last
part in the buffer-pointer that you give it byref.

That buffer-pointer was checked for being a tagged line or not. But if a
last part of a previous buffer was put in that line, then that piece of
string is of course not tagged (as it doesn't start with '*'). So I made
an extra check for the lines that we expect after we send "DONE\r\n" to
the server: I read until I receive a line with "OK", "NO" or "BAD", as
those are the possible untagged responses that DONE (to get out of IDLE)
can cause.

Luckily the CamelStreamBuffer gives the correct piece of buffer if you
call its "gets" again, so in the body of the loop we simply ignore (and
freeup) that corrupt string-piece and handle the next one.

Yeah :-\ Camel is absolutely not designed to cope with non blocking
reading. IDLE more or less requires this, though. If the solution is
handling just one known kludge, when switching between non-blocked and
blocked reading, I guess it's okay (as long as the kludge is controlled
and known).

I'm also not select()ing the socket. In stead I'm simlpy usleep()ing in
some sort of loop. Future plans are indeed to get the select() wrapped
in a CamelStream API, and call that from the code that is now doing an
usleep(). I of course know that this usleep() is not ideal and that a
timeout in select() is preferable.

I'm atm reviewing my code a few times. Will commit later today.


-- 
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/camel-stream.c
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-stream.c	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/camel-stream.c	(working copy)
@@ -98,20 +98,8 @@
 	return (CS_CLASS (stream)->read) (stream, buffer, n);
 }
 
-ssize_t
-camel_stream_read_idle (CamelStream *stream, char *buffer, size_t n)
-{
-	g_return_val_if_fail (CAMEL_IS_STREAM (stream), -1);
-	g_return_val_if_fail (n == 0 || buffer, -1);
 
-	/* Default impl */
-	if (!CS_CLASS (stream)->read_idle)
-		return (CS_CLASS (stream)->read) (stream, buffer, n);
 
-	return (CS_CLASS (stream)->read_idle) (stream, buffer, n);
-}
-
-
 /**
  * camel_stream_write:
  * @stream: a #CamelStream object
Index: libtinymail-camel/camel-lite/camel/camel-stream.h
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-stream.h	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/camel-stream.h	(working copy)
@@ -56,15 +56,12 @@
 	gboolean  (*eos)        (CamelStream *stream);
 	int       (*reset)      (CamelStream *stream);
 
-	ssize_t   (*read_idle)  (CamelStream *stream, char *buffer, size_t n);
-
 } CamelStreamClass;
 
 /* Standard Camel function */
 CamelType camel_stream_get_type (void);
 
 /* public methods */
-ssize_t    camel_stream_read_idle  (CamelStream *stream, char *buffer, size_t n);
 
 ssize_t    camel_stream_read       (CamelStream *stream, char *buffer, size_t n);
 ssize_t    camel_stream_write      (CamelStream *stream, const char *buffer, size_t n);
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c	(working copy)
@@ -195,49 +195,48 @@
 }
 
 static void 
-let_idle_die (CamelImapStore *imap_store, gboolean connect_buz)
+let_idle_die (CamelImapStore *store, gboolean connect_buz)
 {
-	imap_store->idle_cont = FALSE;
 
-	g_static_rec_mutex_lock (imap_store->idle_prefix_lock);
-	g_static_rec_mutex_lock (imap_store->idle_lock);
+	idle_debug ("let_idle_die starts\n");
 
-	imap_store->idle_cont = FALSE;
+	g_static_rec_mutex_lock (store->idle_prefix_lock);
+	g_static_rec_mutex_lock (store->idle_lock);
 
-	/* This one can get called from within the thread! This would deadlock
+	store->idle_kill = TRUE;
+	store->idle_cont = FALSE;
 
-	if (imap_store->in_idle && imap_store->idle_thread) {
-		g_thread_join (imap_store->idle_thread);
-		imap_store->idle_thread = NULL;
-	} */
-
-	if (imap_store->idle_prefix)
+	if (store->idle_prefix)
 	{
 		gchar *resp = NULL;
 		int nwritten = 0;
 		CamelException ex = CAMEL_EXCEPTION_INITIALISER;
 
-		g_free (imap_store->idle_prefix); 
-		imap_store->idle_prefix=NULL;
-
 		idle_debug ("Sending DONE in let_idle_die\n");
-		nwritten = camel_stream_printf (imap_store->ostream, "DONE\r\n");
-		if (nwritten != -1) 
-		{
+		CAMEL_SERVICE_REC_LOCK (store, connect_lock);
+		nwritten = camel_stream_printf (store->ostream, "DONE\r\n");
+		store->in_idle = FALSE;
+		if (nwritten != -1) {
 			resp = NULL;
-			while ((camel_imap_command_response_idle (imap_store, &resp, &ex)) == CAMEL_IMAP_RESPONSE_UNTAGGED) 
-			{
-				idle_debug ("(.., ..) <- %s | in idle_deal_with_stuff in let_idle_die\n", resp); 
+			while ((camel_imap_command_response_idle (store, &resp, &ex)) == CAMEL_IMAP_RESPONSE_UNTAGGED) {
+				idle_debug ("(.., ..) <- %s | in let_idle_die\n", resp); 
 				g_free (resp); resp=NULL; 
 			}
-			if (resp)
+			if (resp) {
+				idle_debug ("(.., ..) <- %s\n", resp);
 				g_free (resp);
+			}
 		}
+		CAMEL_SERVICE_REC_UNLOCK (store, connect_lock);
+		g_free (store->idle_prefix); 
+		store->idle_prefix=NULL;
 	}
 
-	g_static_rec_mutex_unlock (imap_store->idle_lock);
-	g_static_rec_mutex_unlock (imap_store->idle_prefix_lock);
+	g_static_rec_mutex_unlock (store->idle_lock);
+	g_static_rec_mutex_unlock (store->idle_prefix_lock);
 
+	idle_debug ("let_idle_die finished\n");
+
 	return;
 }
 
@@ -247,41 +246,33 @@
 	if (store->current_folder && CAMEL_IS_IMAP_FOLDER (store->current_folder))
 		camel_imap_folder_stop_idle (store->current_folder);
 	else {
-		store->idle_cont = FALSE;
-
 		g_static_rec_mutex_lock (store->idle_prefix_lock);
 		g_static_rec_mutex_lock (store->idle_lock);
 
+		store->idle_kill = TRUE;
 		store->idle_cont = FALSE;
-		if (store->in_idle && store->idle_thread) {
-			g_thread_join (store->idle_thread);
-			store->idle_thread = NULL;
-		}
 
-		if (store->idle_prefix) 
-		{
+		if (store->idle_prefix) {
 			gchar *resp = NULL;
 			int nwritten = 0;
 			CamelException ex = CAMEL_EXCEPTION_INITIALISER;
 
-			idle_debug ("Sending DONE in camel_imap_store_stop_idle (no current folder?)\n");
+			idle_debug ("Sending DONE in camel_imap_store_stop_idle\n");
 			CAMEL_SERVICE_REC_LOCK (store, connect_lock);
 			nwritten = camel_stream_printf (store->ostream, "DONE\r\n");
-
-			if (nwritten != -1) 
-			{
+			store->in_idle = FALSE;
+			if (nwritten != -1) {
 				resp = NULL;
-				while ((camel_imap_command_response_idle (store, &resp, &ex)) == CAMEL_IMAP_RESPONSE_UNTAGGED) 
-				{
+				while ((camel_imap_command_response_idle (store, &resp, &ex)) == CAMEL_IMAP_RESPONSE_UNTAGGED) {
 					idle_debug ("(.., ..) <- %s | in idle_deal_with_stuff (no current folder?)\n", resp); 
 					g_free (resp); resp=NULL; 
 				}
-				if (resp)
+				if (resp) {
+					idle_debug ("(.., ..) <- %s\n", resp);
 					g_free (resp);
+				}
 			}
-
 			CAMEL_SERVICE_REC_UNLOCK (store, connect_lock);
-
 			g_free (store->idle_prefix);
 			store->idle_prefix = NULL;
 		}
@@ -312,6 +303,7 @@
 			camel_imap_folder_selected (folder, response2, &nex, TRUE);
 			camel_imap_response_free (imap_store, response2);
 		}
+		camel_imap_store_start_idle (imap_store);
 	}
 
 	return;
@@ -441,7 +433,7 @@
 
 	imap_store->dontdistridlehack = FALSE;
 
-	imap_store->idle_sleep = 20; /* default of 20s */
+	imap_store->idle_sleep = 600; /* default of 10m */
 	imap_store->getsrv_sleep = 100; /* default of 100s */
 
 	imap_store->in_idle = FALSE;
@@ -2588,8 +2580,8 @@
 	}
 	imap_status_item_free (items);
 
-	camel_imap_store_start_idle (imap_store);
 	CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
+	camel_imap_store_start_idle (imap_store);
 
 	return;
 }
@@ -2630,16 +2622,18 @@
 		const char *c;
 		
 		if (camel_exception_get_id(ex) == CAMEL_EXCEPTION_USER_CANCEL) {
-			camel_imap_store_start_idle (imap_store);
 			CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
+			camel_imap_store_start_idle (imap_store);
+
 			return NULL;
 		}
 		
 		camel_exception_clear (ex);
 		
 		if (!(flags & CAMEL_STORE_FOLDER_CREATE)) {
-			camel_imap_store_start_idle (imap_store);
 			CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
+			camel_imap_store_start_idle (imap_store);
+
 			camel_exception_setv (ex, CAMEL_EXCEPTION_STORE_NO_FOLDER,
 					      _("No such folder %s"), folder_name);
 			return NULL;
@@ -2651,11 +2645,11 @@
 			c++;
 		
 		if (*c != '\0') {
-			camel_imap_store_start_idle (imap_store);
 			CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
 			camel_exception_setv (ex, CAMEL_EXCEPTION_FOLDER_INVALID_PATH,
 					      _("The folder name \"%s\" is invalid because it contains the character \"%c\""),
 					      folder_name, *c);
+			camel_imap_store_start_idle (imap_store);
 			return NULL;
 		}
 
@@ -2673,10 +2667,12 @@
 			int i;
 			
 			if (!(response = camel_imap_command (imap_store, NULL, ex, "LIST \"\" %G", parent_real))) {
-				camel_imap_store_start_idle (imap_store);
-				CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
 				g_free (parent_name);
 				g_free (parent_real);
+
+				CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
+				camel_imap_store_start_idle (imap_store);
+
 				return NULL;
 			}
 			
@@ -2717,12 +2713,13 @@
 				imap_status_item_free (items);
 				
 				if (messages > 0) {
-					camel_imap_store_start_idle (imap_store);
 					camel_exception_set (ex, CAMEL_EXCEPTION_FOLDER_INVALID_STATE,
 							     _("The parent folder is not allowed to contain subfolders"));
-					CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
 					g_free (parent_name);
 					g_free (parent_real);
+					CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
+					camel_imap_store_start_idle (imap_store);
+
 					return NULL;
 				}
 				
@@ -2730,11 +2727,11 @@
 				camel_exception_init (&lex);
 				delete_folder (store, parent_name, &lex);
 				if (camel_exception_is_set (&lex)) {
-					camel_imap_store_start_idle (imap_store);
-					CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
-					camel_exception_xfer (ex, &lex);
 					g_free (parent_name);
 					g_free (parent_real);
+					CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
+					camel_imap_store_start_idle (imap_store);
+					camel_exception_xfer (ex, &lex);
 					return NULL;
 				}
 				
@@ -2745,10 +2742,11 @@
 				g_free (name);
 				
 				if (!response) {
-					camel_imap_store_start_idle (imap_store);
-					CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
 					g_free (parent_name);
 					g_free (parent_real);
+					CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
+					camel_imap_store_start_idle (imap_store);
+
 					return NULL;
 				} else
 					camel_imap_response_free (imap_store, response);
@@ -2770,8 +2768,9 @@
 		}
 		g_free(folder_real);
 		if (!response) {
-			camel_imap_store_start_idle (imap_store);
 			CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
+			camel_imap_store_start_idle (imap_store);
+
 			return NULL;
 		}
 	} else if (flags & CAMEL_STORE_FOLDER_EXCL) {
@@ -2780,9 +2779,9 @@
 				      folder_name);
 		
 		camel_imap_response_free_without_processing (imap_store, response);
-		camel_imap_store_start_idle (imap_store);
 		CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
-		
+		camel_imap_store_start_idle (imap_store);
+
 		return NULL;
 	}
 
@@ -2809,9 +2808,9 @@
 	}
 	camel_imap_response_free_without_processing (imap_store, response);
 
-	camel_imap_store_start_idle (imap_store);
 	CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
-	
+	camel_imap_store_start_idle (imap_store);
+
 	return new_folder;
 }
 
@@ -2850,7 +2849,6 @@
 {
 	CamelImapStore *imap_store = CAMEL_IMAP_STORE (store);
 	CamelImapResponse *response;
-	gboolean did_start_idle = FALSE;
 	CamelFolder *old_in_case;
 
 	CAMEL_SERVICE_REC_LOCK (imap_store, connect_lock);
@@ -2881,7 +2879,6 @@
 		if (response2) {
 			camel_imap_folder_selected (old_in_case, response2, &nex, TRUE);
 			camel_imap_response_free (imap_store, response2);
-			did_start_idle = TRUE;
 		}
 	}
 
@@ -2890,9 +2887,10 @@
 		imap_forget_folder (imap_store, folder_name, ex);
 	}
 fail:
-	if (!did_start_idle)
-		camel_imap_store_start_idle (imap_store);
 	CAMEL_SERVICE_REC_UNLOCK(imap_store, connect_lock);
+
+	camel_imap_store_start_idle (imap_store);
+
 }
 
 static void
@@ -3033,7 +3031,6 @@
 	char *oldpath, *newpath, *storage_path;
 	char *tpath, *lslash;
 	CamelFolder *old_in_case = NULL;
-	gboolean did_start_idle = FALSE;
 
 	CAMEL_SERVICE_REC_LOCK (imap_store, connect_lock);
 
@@ -3078,7 +3075,6 @@
 		if (response2) {
 			camel_imap_folder_selected (old_in_case, response2, &nex, TRUE);
 			camel_imap_response_free (imap_store, response2);
-			did_start_idle = TRUE;
 		}
 	}
 
@@ -3126,10 +3122,11 @@
 	g_free (newpath);
 fail:
 	imap_store->renaming = FALSE;
-	if (!did_start_idle)
-		camel_imap_store_start_idle (imap_store);
 	CAMEL_SERVICE_REC_UNLOCK(imap_store, connect_lock);
 	camel_operation_end (NULL);
+
+	camel_imap_store_start_idle (imap_store);
+
 }
 
 static CamelFolderInfo *
@@ -3955,8 +3952,10 @@
 
 	imap_folder_effectively_unsubscribed (imap_store, folder_name, ex);
 done:
-	camel_imap_store_start_idle (imap_store);
 	CAMEL_SERVICE_REC_UNLOCK(store, connect_lock);
+
+	camel_imap_store_start_idle (imap_store);
+
 }
 
 #if 0
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.h
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.h	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.h	(working copy)
@@ -165,7 +165,7 @@
 
 	GStaticRecMutex *idle_prefix_lock, *idle_lock, *sum_lock;
 	GThread *idle_thread;
-	gboolean idle_cont, in_idle;
+	gboolean idle_cont, in_idle, idle_kill;
 	guint idle_sleep, getsrv_sleep;
 	gboolean courier_crap;
 	gboolean going_online, got_online, clean_exit;
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c	(working copy)
@@ -38,6 +38,8 @@
 
 #include <config.h> 
 
+#include <sched.h>
+
 #include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -760,8 +762,6 @@
 		camel_folder_change_info_free(changes);
 	}
 
-	if (idle)
-		camel_imap_folder_start_idle (folder);
 }
 
 
@@ -907,18 +907,17 @@
 		 * messages. */
 		imap_rescan (folder, camel_folder_summary_count (folder->summary), ex);
 	} else {
+
 #if 0
 		/* on some servers need to CHECKpoint INBOX to recieve new messages?? */
 		/* rfc2060 suggests this, but havent seen a server that requires it */
 		if (g_ascii_strcasecmp(folder->full_name, "INBOX") == 0) {
 			response = camel_imap_command (imap_store, folder, ex, "CHECK");
 			camel_imap_response_free (imap_store, response);
-			camel_imap_folder_start_idle (folder);
 		}
 #endif
 		response = camel_imap_command (imap_store, folder, ex, "NOOP");
 		camel_imap_response_free (imap_store, response);
-		camel_imap_folder_start_idle (folder);
 	}
 
 	si = camel_store_summary_path((CamelStoreSummary *)((CamelImapStore *)folder->parent_store)->summary, folder->full_name);
@@ -939,8 +938,10 @@
 	CAMEL_FOLDER_REC_UNLOCK(folder, lock);
 
 	camel_folder_summary_save(folder->summary, ex);
+	camel_store_summary_save((CamelStoreSummary *)((CamelImapStore *)folder->parent_store)->summary, ex);
 
-	camel_store_summary_save((CamelStoreSummary *)((CamelImapStore *)folder->parent_store)->summary, ex);
+	camel_imap_folder_start_idle (folder);
+
 }
 
 #if 0
@@ -1522,19 +1523,17 @@
 		}
 		g_ptr_array_free (matches, TRUE);
 
-		if (camel_exception_is_set (&local_ex))
-			camel_imap_folder_start_idle (folder);
-
 		/* We unlock here so that other threads can have a chance to grab the connect_lock */
 		CAMEL_SERVICE_REC_UNLOCK (store, connect_lock);
-		
+
 		/* check for an exception */
 		if (camel_exception_is_set (&local_ex)) {
-			/* Look up */
+			
 			camel_exception_xfer (ex, &local_ex);
+			camel_imap_folder_start_idle (folder);
 			return;
 		}
-		
+
 		/* Re-lock the connect_lock */
 		CAMEL_SERVICE_REC_LOCK (store, connect_lock);
 	}
@@ -1542,9 +1541,9 @@
 	/* Save the summary */
 	imap_sync_offline (folder, ex);
 
+	CAMEL_SERVICE_REC_UNLOCK (store, connect_lock);
 	camel_imap_folder_start_idle (folder);
-	CAMEL_SERVICE_REC_UNLOCK (store, connect_lock);
-	/* camel_imap_folder_start_idle (folder); */
+
 }
 
 static int
@@ -1643,7 +1642,6 @@
 	
 	CAMEL_SERVICE_REC_UNLOCK (store, connect_lock);
 
-
 	camel_imap_folder_start_idle (folder);
 
 }
@@ -3591,11 +3589,8 @@
 idle_response_new (CamelFolder *folder)
 {
 	IdleResponse *idle_resp = g_slice_new0 (IdleResponse);
-
+	idle_debug ("idle_response_new\n");
 	idle_resp->vanished = NULL;
-
-	idle_debug ("idle_response_new\n");
-
 	idle_resp->folder = folder;
 	camel_object_ref (CAMEL_OBJECT (folder));
 	return idle_resp;
@@ -3604,27 +3599,23 @@
 static void
 idle_response_free (IdleResponse *idle_resp)
 {
-	guint i=0;
-
 	idle_debug ("idle_response_free\n");
-
 	if (idle_resp->expunged)
 		g_array_free (idle_resp->expunged, TRUE);
-
 	if (idle_resp->vanished) {
-		g_list_foreach (idle_resp->vanished, (GFunc)g_free, NULL);
+		g_list_foreach (idle_resp->vanished, (GFunc) g_free, NULL);
 		g_list_free (idle_resp->vanished);
 		idle_resp->vanished = NULL;
 	}
-
-	if (idle_resp->fetch) 
+	if (idle_resp->fetch) {
+		guint i=0;
 		for (i=0 ;i < idle_resp->fetch->len; i++)
 			g_slice_free (FetchIdleResponse, idle_resp->fetch->pdata[i]);
-
+	}
 	if (idle_resp->folder)
 		camel_object_unref (CAMEL_OBJECT (idle_resp->folder));
-
 	g_slice_free (IdleResponse, idle_resp);
+	return;
 }
 
 
@@ -3633,18 +3624,18 @@
 {
 	char *resp;
 	CamelException ex = CAMEL_EXCEPTION_INITIALISER;
-
+	gboolean l = CAMEL_SERVICE_REC_TRYLOCK (store, connect_lock);
 	if (store->ostream && store->istream && CAMEL_IS_STREAM (store->ostream))
 	{
+		int nwritten = 0;
 		if (store->idle_prefix)
 			g_free (store->idle_prefix);
 		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",
+		nwritten = camel_stream_printf (store->ostream, "%s IDLE\r\n",
 			store->idle_prefix);
+		idle_debug ("(%d, %d) -> %s IDLE\n", strlen (store->idle_prefix)+5, 
+			nwritten, store->idle_prefix);
 	} else {
 		idle_debug ("idle_real_start connection lost\n");
 		goto errh;
@@ -3658,7 +3649,6 @@
 	 * active, the server is now free to send untagged EXISTS, EXPUNGE, and
 	 *  other messages at any time. */
 
-
 	/* So according to the RFC, we will wait for the server for its + 
 	 * continuation. If the server doesn't do this, it's an incorrect 
 	 * IDLE implementation at the server. Right? */
@@ -3680,240 +3670,252 @@
 	}
 	if (resp)
 		g_free (resp);
-
 errh:
+	if (l)
+		CAMEL_SERVICE_REC_UNLOCK (store, connect_lock);
+	return;
+}
 
+
+static void
+consume_idle_line (CamelImapStore *store, CamelFolder *folder, char *resp, IdleResponse *idle_resp)
+{
+	if (strchr (resp, '*') != NULL && (camel_strstrcase (resp, "EXISTS") || 
+		camel_strstrcase (resp, "FETCH")|| camel_strstrcase (resp, "EXPUNGE") || 
+		camel_strstrcase (resp, "VANISHED") || camel_strstrcase (resp, "RECENT")))
+	{
+		if (!idle_resp) 
+			idle_resp = idle_response_new (folder);
+		read_idle_response (folder, resp, idle_resp);
+	}
+	idle_debug ("(%d, ..) <- %s\n", 
+		strlen (resp), resp);
 	return;
 }
 
-static IdleResponse*
-idle_deal_with_stuff (CamelFolder *folder, CamelImapStore *store, gboolean *had_err, gboolean *had_lock)
+typedef struct {
+	CamelFolder *folder;
+	GCond *condition;
+	GMutex *mutex;
+	gboolean had_cond;
+} IdleThreadInfo;
+
+static gpointer 
+idle_thread (gpointer data)
 {
-  IdleResponse *idle_resp = NULL;
-  CamelException ex = CAMEL_EXCEPTION_INITIALISER;
-  char *resp = NULL;
-  int nwritten=0;
-  CamelImapResponseType type;
-  gboolean hlock = FALSE;
+	IdleThreadInfo *info = (IdleThreadInfo *) data;
+	CamelFolder *folder = (CamelFolder *) info->folder;
+	CamelImapFolder *imap_folder;
+	CamelImapStore *store;
+	gboolean tfirst = TRUE, first = TRUE, my_cont, had_cond = FALSE;
+	int cnt = 0;
+	int nwritten=0;
+	gpointer retval = NULL;
 
-  idle_debug ("idle_deal_with_stuff\n");
+	idle_debug ("idle_thread starts\n");
 
-  if (!camel_disco_store_check_online ((CamelDiscoStore*)store, &ex))
-	return NULL;
+	if (!folder || !folder->parent_store) { 
+		g_thread_exit (retval); 
+		return retval; 
+	}
 
-  hlock = CAMEL_SERVICE_REC_TRYLOCK (store, connect_lock);
+	if (!CAMEL_IS_FOLDER (folder) || !CAMEL_IS_STORE (folder->parent_store)) { 
+		g_thread_exit (retval); 
+		return retval; 
+	}
 
-  if (hlock)
-  {
-	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)
-		{
-			if (strchr (resp, '*') != NULL && (camel_strstrcase (resp, "EXISTS") || 
-				camel_strstrcase (resp, "FETCH")|| camel_strstrcase (resp, "EXPUNGE") || 
-				camel_strstrcase (resp, "VANISHED") || camel_strstrcase (resp, "RECENT")))
-			{
-				if (!idle_resp) 
-					idle_resp = idle_response_new (folder);
-				read_idle_response (folder, resp, idle_resp);
-			}
-			idle_debug ("(%d, ..) <- %s | in idle_deal_with_stuff at nb\n", 
-				strlen (resp), resp);
-			g_free (resp); resp=NULL;
-		}
-		if (resp)
-			g_free (resp);
+	store = CAMEL_IMAP_STORE (folder->parent_store);
+	imap_folder = CAMEL_IMAP_FOLDER (folder);
 
+	if (!imap_folder->do_push_email) {
+		idle_debug ("Folder set not to do idle\n");
+		g_thread_exit (retval);
+		return retval;
+	}
 
-		/* Here we force the server to tell us about the changes: */
+	if (!(store->capabilities & IMAP_CAPABILITY_IDLE)) {
+		idle_debug ("Server has no IDLE capabilities\n");
+		g_thread_exit (retval);
+		return retval;
+	}
 
-		/* The IDLE command is terminated by the receipt of a "DONE"
-		 * continuation from the client; such response satisfies the server's
-		 * continuation request.  At that point, the server MAY send any
-		 * remaining queued untagged responses and then MUST immediately send
-		 * the tagged response to the IDLE command and prepare to process other
-		 * commands. */
 
-		if (store->ostream && CAMEL_IS_STREAM (store->ostream)) {
-			nwritten = camel_stream_printf (store->ostream, "DONE\r\n");
-			idle_debug ("(%d, 8) -> DONE | Sending DONE in idle_deal_with_stuff (nb)\n",
-				nwritten);
+	camel_object_ref (folder);
+
+	my_cont = store->idle_cont;
+
+	if (my_cont) {
+		idle_debug ("idle_thread starting\n");
+	} else {
+		idle_debug ("idle_thread not starting\n");
+	}
+
+	while (my_cont && !store->idle_kill) 
+	{
+		CamelException ex = CAMEL_EXCEPTION_INITIALISER;
+		char *resp = NULL;
+		IdleResponse *idle_resp = NULL;
+
+		if (store->idle_cont && first) {
+			gboolean l = g_static_rec_mutex_trylock (store->idle_lock);
+			if (!store->idle_kill)
+				idle_real_start (store);
+			if (l)
+				g_static_rec_mutex_unlock (store->idle_lock);
+			first = FALSE;
 		}
 
-		if (nwritten == -1) 
-			goto outofhere;
+		if (tfirst) {
+			if (info->condition) {
+				g_mutex_lock (info->mutex);
+				g_cond_broadcast (info->condition);
+				info->had_cond = TRUE; had_cond = TRUE;
+				g_mutex_unlock (info->mutex);
+			}
+			tfirst = FALSE;
+		}
 
-		resp = NULL;
-		while ((type = camel_imap_command_response_idle (store, &resp, &ex)) == CAMEL_IMAP_RESPONSE_UNTAGGED) 
-		{
-			/* printf ("D resp: %s\n", resp); */
-			if (strchr (resp, '*') != NULL && (camel_strstrcase (resp, "EXISTS") ||
-				camel_strstrcase (resp, "FETCH") || camel_strstrcase (resp, "EXPUNGE") || 
-				camel_strstrcase (resp, "RECENT")))
-			{
+		if (g_static_rec_mutex_trylock (store->idle_lock)) {
+			if (!store->idle_kill)
+			  while (camel_imap_store_readline_nb (store, &resp, &ex) > 0)
+			  {
 				if (!idle_resp)
 					idle_resp = idle_response_new (folder);
-				read_idle_response (folder, resp, idle_resp);
-			}
-			idle_debug ("(%d, ..) <- %s | in idle_deal_with_stuff at idle\n", 
-				strlen (resp), resp);
-			g_free (resp); resp=NULL;
+				consume_idle_line (store, folder, resp, idle_resp);
+				g_free (resp);
+				resp = NULL;
+			  }
+			g_static_rec_mutex_unlock (store->idle_lock);
 		}
 
-		if (type == CAMEL_IMAP_RESPONSE_ERROR)
-			*had_err = TRUE;
-
 		if (resp)
 			g_free (resp);
 
-	} else {
-		/* Trying to deal while the current folder is gone: just read away everything */
-		if (store->ostream && CAMEL_IS_STREAM (store->ostream)) {
-			nwritten = camel_stream_printf (store->ostream, "DONE\r\n");
-			idle_debug ("(%d, 8) -> DONE | Sending DONE in idle_deal_with_stuff (b)\n",
-				nwritten);
+		if (idle_resp) {
+			process_idle_response (idle_resp);
+			idle_response_free (idle_resp);
 		}
-		if (nwritten == -1) 
-			goto outofhere;
-		resp = NULL;
-		while ((type = camel_imap_command_response_idle (store, &resp, &ex)) == CAMEL_IMAP_RESPONSE_UNTAGGED) 
-		{
-			idle_debug ("(.., ..) <- %s | in idle_deal_with_stuff in else\n", resp); 
-			g_free (resp); resp=NULL; 
-		}
-		if (resp)
-			g_free (resp);
-	}
 
-outofhere:
+		idle_resp = NULL;
 
-	CAMEL_SERVICE_REC_UNLOCK (store, connect_lock);
-  } 
+		if ((cnt > store->idle_sleep) || (!store->idle_cont)) 
+		{
+			if (store->idle_prefix) 
+			{
+				CamelImapResponseType type;
+				gboolean l = g_static_rec_mutex_trylock (store->idle_lock);
 
-  *had_lock = hlock;
+				if (!store->idle_kill) {
+				  resp = NULL;
+				  while (camel_imap_store_readline_nb (store, &resp, &ex) > 0)
+				  {
+					if (!idle_resp) {
+						idle_resp = idle_response_new (folder);
+						/* We will free this after the join */
+						retval = idle_resp; 
+					}
+					consume_idle_line (store, folder, resp, idle_resp);
+					g_free (resp);
+					resp = NULL;
+				  }
+				  if (resp)
+					g_free (resp);
+				  resp = NULL;
 
-  return idle_resp;
-}
+				  nwritten = camel_stream_printf (store->ostream, "DONE\r\n");
+				  idle_debug ("(%d, 8) -> DONE\n", nwritten);
+				  while ((type = camel_imap_command_response_idle (store, &resp, &ex)) == CAMEL_IMAP_RESPONSE_UNTAGGED) 
+				  {
+					if (resp && resp[0] == '*') {
+						if (!idle_resp) {
+							idle_resp = idle_response_new (folder);
+							/* We will free this after the join */
+							retval = idle_resp; 
+						}
+						consume_idle_line (store, folder, resp, idle_resp);
+					}
+					if (resp)
+						g_free (resp);
+					resp = NULL;
+				  }
+				}
 
-void
-camel_imap_folder_stop_idle (CamelFolder *folder)
-{
-	CamelImapStore *store;
-	IdleResponse *idle_resp = NULL;
-	gboolean had_err = FALSE;
-	CamelException ex = CAMEL_EXCEPTION_INITIALISER;
+				if (l)
+					g_static_rec_mutex_unlock (store->idle_lock);
+				if (resp)
+					g_free (resp);
+				resp = NULL;
 
-	idle_debug ("camel_imap_folder_stop_idle\n");
+			}
+			if (store->idle_cont)
+				first = TRUE; 
+			else
+				my_cont = FALSE;
 
-	store = CAMEL_IMAP_STORE (folder->parent_store);
-
-	if (!camel_disco_store_check_online ((CamelDiscoStore*)store, &ex))
-		return;
-
-	store->idle_cont = FALSE;
-
-	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->in_idle && store->idle_thread) {
-			g_thread_join (store->idle_thread);
-			store->idle_thread = NULL;
+			cnt = 0;
 		}
 
-		if (store->idle_prefix)
-			g_free (store->idle_prefix);
-		store->idle_prefix = NULL;
+		if (my_cont)
+			usleep (500000);
+		cnt++;
 
-		idle_resp = idle_deal_with_stuff (folder, store, &had_err, &had_lock);
+	}
 
-		if (idle_resp && !had_err)
-			process_idle_response (idle_resp);
-
-		if (idle_resp) 
-			idle_response_free (idle_resp);
+	if (!had_cond && !info->had_cond && info->condition) {
+		g_mutex_lock (info->mutex);
+		g_cond_broadcast (info->condition);
+		info->had_cond = TRUE;
+		g_mutex_unlock (info->mutex);
 	}
 
-	g_static_rec_mutex_unlock (store->idle_prefix_lock);
-	g_static_rec_mutex_unlock (store->idle_lock);
+	camel_object_unref (folder);
 
+	g_thread_exit (retval);
+
+	return retval;
 }
 
 
-static gpointer 
-idle_thread (gpointer data)
+void
+camel_imap_folder_stop_idle (CamelFolder *folder)
 {
-	CamelFolder *folder = (CamelFolder *) data;
-	CamelImapFolder *imap_folder;
 	CamelImapStore *store;
-	gboolean had_err = FALSE, hadlock = FALSE;
+	CamelException ex = CAMEL_EXCEPTION_INITIALISER;
 
-	idle_debug ("idle_thread\n");
+	idle_debug ("camel_imap_folder_stop_idle\n");
 
-	if (!folder || !folder->parent_store)
-		{ g_thread_exit (NULL); return NULL; }
-
-	if (!CAMEL_IS_FOLDER (folder) || !CAMEL_IS_STORE (folder->parent_store))
-		{ g_thread_exit (NULL); return NULL; }
-
 	store = CAMEL_IMAP_STORE (folder->parent_store);
-	imap_folder = CAMEL_IMAP_FOLDER (folder);
 
-	if (!imap_folder->do_push_email) {
-		idle_debug ("Folder set not to do idle\n");
-		return NULL;
-	}
+	store->idle_cont = FALSE;
 
-	if (!(store->capabilities & IMAP_CAPABILITY_IDLE)) {
-		idle_debug ("Server has no IDLE capabilities\n");
-		return NULL;
-	}
+	if (!camel_disco_store_check_online ((CamelDiscoStore*)store, &ex))
+		return;
 
-	idle_debug ("idle_thread starting (%s)\n", store->idle_prefix?store->idle_prefix:"(none)");
-
-	store->idle_cont = TRUE;
-
-	while (store->idle_cont && store->idle_prefix != NULL)
+	if ((store->capabilities & IMAP_CAPABILITY_IDLE))
 	{
-		int x = 0;
+		if (store->in_idle && store->idle_thread) {
+			IdleResponse *idle_resp = NULL;
+			store->idle_cont = FALSE;
+			idle_resp = g_thread_join (store->idle_thread);
+			g_static_rec_mutex_lock (store->idle_prefix_lock);
+			g_static_rec_mutex_lock (store->idle_lock);
+			store->in_idle = FALSE;
+			store->idle_thread = NULL;
+			if (store->idle_prefix)
+				g_free (store->idle_prefix);
+			g_static_rec_mutex_unlock (store->idle_lock);
+			g_static_rec_mutex_unlock (store->idle_prefix_lock);
 
-		g_static_rec_mutex_lock (store->idle_lock);
-		g_static_rec_mutex_lock (store->idle_prefix_lock);
-
-		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)
+			if (idle_resp) {
 				process_idle_response (idle_resp);
+				idle_response_free (idle_resp);
+			}
 
-			if (hadlock)
-				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 (store->idle_lock);
-
-		idle_debug ("idle checked in idle_thread, waiting %ds for new check\n", store->idle_sleep);
-
-		for (x=0; x<1000 && store->idle_cont; x++)
-			usleep (store->idle_sleep * 1000);
+		store->idle_prefix = NULL;
 	}
 
-	store->in_idle = FALSE;
-
-	g_thread_exit (NULL);
-	return NULL;
+	return;
 }
 
 
@@ -3922,6 +3924,7 @@
 {
 	CamelImapStore *store;
 	CamelException ex = CAMEL_EXCEPTION_INITIALISER;
+	CamelImapFolder *imap_folder = (CamelImapFolder *) folder;
 
 	idle_debug ("camel_imap_folder_start_idle\n");
 
@@ -3930,6 +3933,9 @@
 	if (!camel_disco_store_check_online ((CamelDiscoStore*)store, &ex))
 		return;
 
+	if (!imap_folder->do_push_email)
+		return;
+
 	g_static_rec_mutex_lock (store->idle_lock);
 
 	if (store->capabilities & IMAP_CAPABILITY_IDLE)
@@ -3939,17 +3945,44 @@
 		{
 			folder->folder_flags |= CAMEL_FOLDER_HAS_PUSHEMAIL_CAPABILITY;
 
-			if (!store->in_idle && store->idle_thread)
-			{
+			if (!store->in_idle && store->idle_thread) {
+				IdleResponse *idle_resp = NULL;
+
 				store->idle_cont = FALSE;
-				g_thread_join (store->idle_thread);
+				idle_resp = g_thread_join (store->idle_thread);
 				store->idle_thread = NULL;
+
+				if (idle_resp) {
+					process_idle_response (idle_resp);
+					idle_response_free (idle_resp);
+				}
+
 			}
 
 			if (!store->in_idle) {
-				idle_real_start (store);
+				IdleThreadInfo *info = g_slice_new0 (IdleThreadInfo);
+				store->idle_kill = FALSE;
+				store->in_idle = TRUE;
+				store->idle_cont = TRUE;
+
+				info->mutex = g_mutex_new ();
+				info->condition = g_cond_new ();
+				info->had_cond = FALSE;
+				info->folder = folder;
+				camel_object_ref (info->folder);
+
 				store->idle_thread = g_thread_create (idle_thread, 
-					folder, TRUE, NULL);
+					info, TRUE, NULL);
+				g_mutex_lock (info->mutex);
+				if (!info->had_cond)
+					g_cond_wait (info->condition, info->mutex);
+				g_mutex_unlock (info->mutex);
+
+				camel_object_unref (info->folder);
+				g_mutex_free (info->mutex);
+				g_cond_free (info->condition);
+				g_slice_free (IdleThreadInfo, info);
+
 			}
 
 		}
@@ -3977,6 +4010,8 @@
 
 	return;
 }
+
+
 /* Called with the store's connect_lock locked */
 void
 camel_imap_folder_changed (CamelFolder *folder, int exists,
@@ -4018,9 +4053,9 @@
 	len = camel_folder_summary_count (folder->summary);
 
 	if (exists > len) {
+		/* TNY Question: is this stop really necessary? Why? */
 		camel_imap_folder_stop_idle (folder);
 		imap_update_summary (folder, exists, changes, ex);
-		/* camel_imap_folder_start_idle (folder); will happen later? */
 	}
 
 	if (camel_folder_change_info_changed (changes))
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-command.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-command.c	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-command.c	(working copy)
@@ -48,6 +48,8 @@
 #include "camel-imap-utils.h"
 #include "camel-imap-summary.h"
 
+#include "camel-string-utils.h"
+
 extern int camel_verbose_debug;
 
 static gboolean imap_command_start (CamelImapStore *store, CamelFolder *folder,
@@ -61,9 +63,7 @@
 static char *imap_command_strdup_printf (CamelImapStore *store,
 					 const char *fmt, ...);
 
-static char * imap_read_untagged_idle (CamelImapStore *store, char *line, CamelException *ex);
 
-
 static int
 uid_compar (const void *va, const void *vb)
 {
@@ -538,9 +538,11 @@
 	CamelImapResponseType type;
 	char *respbuf;
 
-	if (camel_imap_store_readline_idle (store, &respbuf, ex) < 0)
+	if (camel_imap_store_readline_nl (store, &respbuf, ex) < 0)
 		return CAMEL_IMAP_RESPONSE_ERROR;
 
+	imap_debug ("(.., ..) <- %s (IDLE response)\n", respbuf);
+
 	switch (*respbuf) {
 	case '*':
 		if (!g_ascii_strncasecmp (respbuf, "* BYE", 5)) {
@@ -558,7 +560,7 @@
 		
 		/* Read the rest of the response. */
 		type = CAMEL_IMAP_RESPONSE_UNTAGGED;
-		respbuf = imap_read_untagged_idle (store, respbuf, ex);
+		respbuf = imap_read_untagged (store, respbuf, ex);
 		if (!respbuf)
 			type = CAMEL_IMAP_RESPONSE_ERROR;
 		else if (!g_ascii_strncasecmp (respbuf, "* OK [ALERT]", 12)
@@ -580,7 +582,14 @@
 		type = CAMEL_IMAP_RESPONSE_CONTINUATION;
 		break;
 	default:
-		type = CAMEL_IMAP_RESPONSE_TAGGED;
+		if (camel_strstrcase (respbuf, "OK") != NULL || 
+			camel_strstrcase (respbuf, "NO") != NULL ||
+			camel_strstrcase (respbuf, "BAD") != NULL) {
+
+			type = CAMEL_IMAP_RESPONSE_TAGGED;
+
+		} else 
+			type = CAMEL_IMAP_RESPONSE_UNTAGGED;
 		break;
 	}
 	*response = respbuf;
@@ -817,167 +826,6 @@
 }
 
 
-
-static char *
-imap_read_untagged_idle (CamelImapStore *store, char *line, CamelException *ex)
-{
-	int fulllen, ldigits, nread, n, i, sexp = 0;
-	unsigned int length;
-	GPtrArray *data;
-	GString *str;
-	char *end, *p, *s, *d;
-	
-	p = strrchr (line, '{');
-	if (!p)
-		return line;
-	
-	data = g_ptr_array_new ();
-	fulllen = 0;
-	
-	while (1) {
-		str = g_string_new (line);
-		g_free (line);
-		fulllen += str->len;
-		g_ptr_array_add (data, str);
-		
-		if (!(p = strrchr (str->str, '{')) || p[1] == '-')
-			break;
-		
-		/* HACK ALERT: We scan the non-literal part of the string, looking for possible s expression braces.
-		   This assumes we're getting s-expressions, which we should be.
-		   This is so if we get a blank line after a literal, in an s-expression, we can keep going, since
-		   we do no other parsing at this level.
-		   TODO: handle quoted strings? */
-		for (s=str->str; s<p; s++) {
-			if (*s == '(')
-				sexp++;
-			else if (*s == ')')
-				sexp--;
-		}
-		
-		length = strtoul (p + 1, &end, 10);
-		if (*end != '}' || *(end + 1) || end == p + 1 || length >= UINT_MAX - 2)
-			break;
-		ldigits = end - (p + 1);
-		
-		/* Read the literal */
-		str = g_string_sized_new (length + 2);
-		str->str[0] = '\n';
-		nread = 0;
-		
-		do {
-			if ((n = camel_stream_read_idle (store->istream, str->str + nread + 1, length - nread)) == -1) {
-				if (errno == EINTR) {
-					CamelException mex = CAMEL_EXCEPTION_INITIALISER;
-					camel_exception_set (ex, CAMEL_EXCEPTION_USER_CANCEL,
-							     _("Operation cancelled"));
-					camel_imap_recon (store, &mex);
-					imap_debug ("Recon in untagged idle: %s\n", camel_exception_get_description (&mex));
-				} else {
-					camel_exception_set (ex, CAMEL_EXCEPTION_SERVICE_UNAVAILABLE,
-							     g_strerror (errno));
-					camel_service_disconnect (CAMEL_SERVICE (store), FALSE, NULL);
-				}
-				g_string_free (str, TRUE);
-				goto lose;
-			}
-			
-			nread += n;
-		} while (n > 0 && nread < length);
-		
-		if (nread < length) {
-			if (errno == EINTR) {
-				CamelException mex = CAMEL_EXCEPTION_INITIALISER;
-				camel_exception_set (ex, CAMEL_EXCEPTION_USER_CANCEL,
-						     _("Operation cancelled"));
-				camel_imap_recon (store, &mex);
-				imap_debug ("Recon in untagged idle: %s\n", camel_exception_get_description (&mex));
-			}  else {
-				camel_exception_set (ex, CAMEL_EXCEPTION_SERVICE_UNAVAILABLE,
-					     _("Server response ended too soon."));
-				camel_service_disconnect (CAMEL_SERVICE (store), FALSE, NULL);
-			}
-			g_string_free (str, TRUE);
-			goto lose;
-		}
-		str->str[length + 1] = '\0';
-
-		if (camel_debug("imap")) {
-			printf("Literal: -->");
-			fwrite(str->str+1, 1, length, stdout);
-			printf("<--\n");
-		}
-		
-		/* Fix up the literal, turning CRLFs into LF. Also, if
-		 * we find any embedded NULs, strip them. This is
-		 * dubious, but:
-		 *   - The IMAP grammar says you can't have NULs here
-		 *     anyway, so this will not affect our behavior
-		 *     against any completely correct server.
-		 *   - WU-imapd 12.264 (at least) will cheerily pass
-		 *     NULs along if they are embedded in the message
-		 */
-		
-		s = d = str->str + 1;
-		end = str->str + 1 + length;
-		while (s < end) {
-			while (s < end && *s == '\0') {
-				s++;
-				length--;
-			}
-			if (*s == '\r' && *(s + 1) == '\n') {
-				s++;
-				length--;
-			}
-			*d++ = *s++;
-		}
-		*d = '\0';
-		str->len = length + 1;
-		
-		/* p points to the "{" in the line that starts the
-		 * literal. The length of the CR-less response must be
-		 * less than or equal to the length of the response
-		 * with CRs, therefore overwriting the old value with
-		 * the new value cannot cause an overrun. However, we
-		 * don't want it to be shorter either, because then the
-		 * GString's length would be off...
-		 */
-		sprintf (p, "{%0*u}", ldigits, length);
-		
-		fulllen += str->len;
-		g_ptr_array_add (data, str);
-
-		/* Read the next line. */
-		do {
-			if (camel_imap_store_readline_idle (store, &line, ex) < 0)
-				goto lose;
-
-			/* MAJOR HACK ALERT, gropuwise sometimes sends an extra blank line after literals, check that here
-			   But only do it if we're inside an sexpression */
-			if (line[0] == 0 && sexp > 0)
-				g_warning("Server sent empty line after a literal, assuming in error");
-		} while (line[0] == 0 && sexp > 0);
-	}
-	
-	/* Now reassemble the data. */
-	p = line = g_malloc (fulllen + 1);
-	for (i = 0; i < data->len; i++) {
-		str = data->pdata[i];
-		memcpy (p, str->str, str->len);
-		p += str->len;
-		g_string_free (str, TRUE);
-	}
-	*p = '\0';
-	g_ptr_array_free (data, TRUE);
-	return line;
-	
- lose:
-	for (i = 0; i < data->len; i++)
-		g_string_free (data->pdata[i], TRUE);
-	g_ptr_array_free (data, TRUE);
-	return NULL;
-}
-
 /**
  * camel_imap_response_free:
  * @store: the CamelImapStore the response is from
Index: libtinymail-camel/camel-lite/camel/camel-tcp-stream-openssl.c
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-tcp-stream-openssl.c	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/camel-tcp-stream-openssl.c	(working copy)
@@ -61,7 +61,6 @@
 /* Returns the class for a CamelTcpStreamSSL */
 #define CTSR_CLASS(so) CAMEL_TCP_STREAM_SSL_CLASS (CAMEL_OBJECT_GET_CLASS (so))
 
-static ssize_t stream_read_idle (CamelStream *stream, char *buffer, size_t n);
 static ssize_t stream_read_nb (CamelTcpStream *stream, char *buffer, size_t n);
 static ssize_t stream_read (CamelStream *stream, char *buffer, size_t n);
 static ssize_t stream_write (CamelStream *stream, const char *buffer, size_t n);
@@ -100,9 +99,6 @@
 
 
 	parent_class = CAMEL_TCP_STREAM_CLASS (camel_type_get_global_classfuncs (camel_tcp_stream_get_type ()));
-	
-	/* virtual method overload */
-	camel_stream_class->read_idle = stream_read_idle;
 
 	camel_stream_class->read = stream_read;
 	camel_stream_class->write = stream_write;
@@ -290,58 +286,7 @@
 	return 0;
 }
 
-static ssize_t 
-stream_read_idle (CamelStream *stream, char *buffer, size_t n)
-{
-	CamelTcpStreamSSL *openssl = CAMEL_TCP_STREAM_SSL (stream);
-	SSL *ssl = openssl->priv->ssl;
-	ssize_t nread;
 
-	int error, flags, fdmax;
-	struct timeval timeout;
-	fd_set rdset;
-	int res;
-
-	flags = fcntl (openssl->priv->sockfd, F_GETFL);
-	fcntl (openssl->priv->sockfd, F_SETFL, flags | O_NONBLOCK);
-	
-	fdmax = openssl->priv->sockfd + 1;
-	
-	do {
-		FD_ZERO (&rdset);
-		FD_SET (openssl->priv->sockfd, &rdset);
-		nread = -1;
-		timeout.tv_sec = IDLE_READ_TIMEOUT;
-		timeout.tv_usec = 0;
-		res = select (fdmax, &rdset, 0, 0, &timeout);
-		
-		if (res == -1)
-			;
-		else if (res == 0)
-			errno = ETIMEDOUT;
-		else {
-
-		  do {
-			if (ssl) {
-				nread = SSL_read (ssl, buffer, n);
-				if (nread < 0)
-					errno = ssl_errno (ssl, nread);
-			} else {
-				nread = read (openssl->priv->sockfd, buffer, n);
-			}
-		  } while (0 && (nread < 0 && errno == EINTR));
-		}
-	} while (0 && (nread < 0 && (errno == EAGAIN || errno == EWOULDBLOCK)));
-	
-	error = errno;
-	fcntl (openssl->priv->sockfd, F_SETFL, flags);
-	errno = error;
-
-
-	return nread;
-}
-
-
 static ssize_t
 stream_read_nb (CamelTcpStream *stream, char *buffer, size_t n)
 {
Index: libtinymail-camel/camel-lite/camel/camel-file-utils.c
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-file-utils.c	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/camel-file-utils.c	(working copy)
@@ -703,49 +703,6 @@
 }
 
 
-
-ssize_t
-camel_read_idle (int fd, char *buf, size_t n)
-{
-	ssize_t nread;
-
-	int errnosav, flags, fdmax;
-	fd_set rdset;
-	
-	flags = fcntl (fd, F_GETFL);
-	fcntl (fd, F_SETFL, flags | O_NONBLOCK);
-	
-	do {
-		struct timeval tv;
-		int res;
-
-		FD_ZERO (&rdset);
-		FD_SET (fd, &rdset);
-		fdmax = fd + 1;
-		tv.tv_sec = IDLE_READ_TIMEOUT;
-		tv.tv_usec = 0;
-		nread = -1;
-
-		res = select(fdmax, &rdset, 0, 0, &tv);
-		if (res == -1)
-			;
-		else if (res == 0)
-			errno = ETIMEDOUT;
-		else {
-			do {
-				nread = read (fd, buf, n);
-			} while (0 && (nread == -1 && errno == EINTR));
-		}
-	} while (0 && (nread == -1 && (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK)));
-
-	errnosav = errno;
-	fcntl (fd, F_SETFL, flags);
-	errno = errnosav;
-
-	return nread;
-}
-
-
 /**
  * camel_read_socket:
  * @fd: a socket
@@ -842,86 +799,6 @@
 
 
 ssize_t
-camel_read_socket_idle (int fd, char *buf, size_t n)
-{
-#ifndef G_OS_WIN32
-	return camel_read_idle (fd, buf, n);
-#else
-	ssize_t nread;
-	int cancel_fd;
-	
-	if (camel_operation_cancel_check (NULL)) {
-		errno = EINTR;
-		return -1;
-	}
-	cancel_fd = camel_operation_cancel_fd (NULL);
-
-	if (cancel_fd == -1) {
-
-		int fdmax;
-		fd_set rdset;
-		u_long yes = 1;
-
-		ioctlsocket (fd, FIONBIO, &yes);
-		fdmax = fd + 1;
-		do {
-			struct timeval tv;
-			int res;
-
-			FD_ZERO (&rdset);
-			FD_SET (fd, &rdset);
-			tv.tv_sec = IDLE_READ_TIMEOUT;
-			tv.tv_usec = 0;
-			nread = -1;
-
-			res = select(fdmax, &rdset, 0, 0, &tv);
-			if (res == -1)
-				;
-			else if (res == 0)
-				errno = ETIMEDOUT;
-			} else {				
-				nread = recv (fd, buf, n, 0);
-			}
-		} while ((nread == -1 && WSAGetLastError () == WSAEWOULDBLOCK));
-
-	} else {
-		int fdmax;
-		fd_set rdset;
-		u_long yes = 1;
-
-		ioctlsocket (fd, FIONBIO, &yes);
-		fdmax = MAX (fd, cancel_fd) + 1;
-		do {
-			struct timeval tv;
-			int res;
-
-			FD_ZERO (&rdset);
-			FD_SET (fd, &rdset);
-			FD_SET (cancel_fd, &rdset);
-			tv.tv_sec = IDLE_READ_TIMEOUT;
-			tv.tv_usec = 0;
-			nread = -1;
-
-			res = select(fdmax, &rdset, 0, 0, &tv);
-			if (res == -1)
-				;
-			else if (res == 0)
-				errno = ETIMEDOUT;
-			else if (FD_ISSET (cancel_fd, &rdset)) {
-				errno = EINTR;
-				goto failed;
-			} else {				
-				nread = recv (fd, buf, n, 0);
-			}
-		} while ((nread == -1 && WSAGetLastError () == WSAEWOULDBLOCK));
-	failed:
-		;
-	}
-	
-	return nread;
-#endif
-}
-ssize_t
 camel_read_socket_nb (int fd, char *buf, size_t n)
 {
 #ifndef G_OS_WIN32
Index: libtinymail-camel/camel-lite/camel/camel-file-utils.h
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-file-utils.h	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/camel-file-utils.h	(working copy)
@@ -88,7 +88,6 @@
 ssize_t camel_read (int fd, char *buf, size_t n);
 ssize_t camel_write (int fd, const char *buf, size_t n);
 
-ssize_t camel_read_socket_idle (int fd, char *buf, size_t n);
 ssize_t camel_write_socket (int fd, const char *buf, size_t n);
 
 ssize_t camel_read_socket (int fd, char *buf, size_t n);
Index: libtinymail-camel/camel-lite/camel/camel-tcp-stream-raw.c
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-tcp-stream-raw.c	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/camel-tcp-stream-raw.c	(working copy)
@@ -65,7 +65,6 @@
 static struct sockaddr *stream_get_local_address (CamelTcpStream *stream, socklen_t *len);
 static struct sockaddr *stream_get_remote_address (CamelTcpStream *stream, socklen_t *len);
 static ssize_t stream_read_nb (CamelTcpStream *stream, char *buffer, size_t n);
-static ssize_t stream_read_idle (CamelStream *stream, char *buffer, size_t n);
 
 
 static void
@@ -79,7 +78,6 @@
 	parent_class = CAMEL_TCP_STREAM_CLASS (camel_type_get_global_classfuncs (camel_tcp_stream_get_type ()));
 
 	/* virtual method overload */
-	camel_stream_class->read_idle = stream_read_idle;
 	camel_stream_class->read = stream_read;
 	camel_stream_class->write = stream_write;
 	camel_stream_class->flush = stream_flush;
@@ -258,15 +256,6 @@
 	return camel_read_socket (raw->sockfd, buffer, n);
 }
 
-
-static ssize_t 
-stream_read_idle (CamelStream *stream, char *buffer, size_t n)
-{
-	CamelTcpStreamRaw *raw = CAMEL_TCP_STREAM_RAW (stream);
-	
-	return camel_read_socket_idle (raw->sockfd, buffer, n);
-}
-
 static ssize_t
 stream_read_nb (CamelTcpStream *stream, char *buffer, size_t n)
 {
Index: libtinymail-camel/camel-lite/camel/camel-stream-buffer.c
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-stream-buffer.c	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/camel-stream-buffer.c	(working copy)
@@ -429,51 +429,7 @@
 }
 
 
-
 int
-camel_stream_buffer_gets_idle (CamelStreamBuffer *sbf, char *buf, unsigned int max)
-{
-	register char *outptr, *inptr, *inend, c, *outend;
-	int bytes_read;
-
-	outptr = buf;
-	inptr = (char*)sbf->ptr;
-	inend = (char*)sbf->end;
-	outend = buf+max-1;	/* room for NUL */
-
-	do {
-		while (inptr<inend && outptr<outend) {
-			c = *inptr++;
-			*outptr++ = c;
-			if (c == '\n') {
-				*outptr = 0;
-				sbf->ptr = (unsigned char*) inptr;
-				return outptr-buf;
-			}
-		}
-		if (outptr == outend)
-			break;
-
-		bytes_read = camel_stream_read_idle (sbf->stream, (char*)sbf->buf, sbf->size);
-		if (bytes_read == -1) {
-			if (buf == outptr)
-				return -1;
-			else
-				bytes_read = 0;
-		}
-		sbf->ptr = sbf->buf;
-		sbf->end = sbf->buf + bytes_read;
-		inptr = (char*)sbf->ptr;
-		inend = (char*)sbf->end;
-	} while (bytes_read>0);
-
-	sbf->ptr = (unsigned char*)inptr;
-	*outptr = 0;
-
-	return (int)(outptr - buf);
-}
-
-int
 camel_tcp_stream_buffer_gets_nb (CamelStreamBuffer *sbf, char *buf, unsigned int max)
 {
 	register char *outptr, *inptr, *inend, c, *outend;
Index: libtinymail-camel/camel-lite/camel/camel-stream-buffer.h
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-stream-buffer.h	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/camel-stream-buffer.h	(working copy)
@@ -95,7 +95,6 @@
 
 char *camel_stream_buffer_read_line (CamelStreamBuffer *sbf);
 int camel_tcp_stream_buffer_gets_nb (CamelStreamBuffer *sbf, char *buf, unsigned int max);
-int camel_stream_buffer_gets_idle (CamelStreamBuffer *sbf, char *buf, unsigned int max);
 
 
 G_END_DECLS
Index: libtinymail-camel/camel-lite/camel/camel-tcp-stream-ssl.c
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-tcp-stream-ssl.c	(revision 2872)
+++ libtinymail-camel/camel-lite/camel/camel-tcp-stream-ssl.c	(working copy)
@@ -84,7 +84,6 @@
 static struct sockaddr *stream_get_local_address (CamelTcpStream *stream, socklen_t *len);
 static struct sockaddr *stream_get_remote_address (CamelTcpStream *stream, socklen_t *len);
 static ssize_t stream_read_nb (CamelTcpStream *stream, char *buffer, size_t n);
-static ssize_t stream_read_idle (CamelStream *stream, char *buffer, size_t n);
 
 struct _CamelTcpStreamSSLPrivate {
 	PRFileDesc *sockfd;
@@ -106,9 +105,6 @@
 		CAMEL_STREAM_CLASS (camel_tcp_stream_ssl_class);
 	
 	parent_class = CAMEL_TCP_STREAM_CLASS (camel_type_get_global_classfuncs (camel_tcp_stream_get_type ()));
-	
-	/* virtual method overload */
-	camel_stream_class->read_idle = stream_read_idle;
 
 	camel_stream_class->read = stream_read;
 	camel_stream_class->write = stream_write;
@@ -546,136 +542,6 @@
 }
 
 
-
-static ssize_t 
-stream_read_idle (CamelStream *stream, char *buffer, size_t n)
-{
-	CamelTcpStreamSSL *tcp_stream_ssl = CAMEL_TCP_STREAM_SSL (stream);
-	PRFileDesc *cancel_fd;
-	ssize_t nread;
-	
-	if (camel_operation_cancel_check (NULL)) {
-		errno = EINTR;
-		return -1;
-	}
-	
-	cancel_fd = camel_operation_cancel_prfd (NULL);
-	if (cancel_fd == NULL) {
-
-		PRSocketOptionData sockopts;
-		PRPollDesc pollfds[1];
-		gboolean nonblock;
-		int error;
-		
-		/* get O_NONBLOCK options */
-		sockopts.option = PR_SockOpt_Nonblocking;
-		PR_GetSocketOption (tcp_stream_ssl->priv->sockfd, &sockopts);
-		sockopts.option = PR_SockOpt_Nonblocking;
-		nonblock = sockopts.value.non_blocking;
-		sockopts.value.non_blocking = TRUE;
-		PR_SetSocketOption (tcp_stream_ssl->priv->sockfd, &sockopts);
-
-		pollfds[0].fd = tcp_stream_ssl->priv->sockfd;
-		pollfds[0].in_flags = PR_POLL_READ;
-
-		do {
-			PRInt32 res;
-
-			pollfds[0].out_flags = 0;
-			nread = -1;
-
-			res = PR_Poll(pollfds, 1, PR_TicksPerSecond () * IDLE_READ_TIMEOUT);
-
-			if (res == -1)
-				set_errno(PR_GetError());
-			else if (res == 0) {
-#ifdef ETIMEDOUT
-				errno = ETIMEDOUT;
-#else
-				errno = EIO;
-#endif
-			} else {
-				do {
-					nread = PR_Read (tcp_stream_ssl->priv->sockfd, buffer, n);
-					if (nread == -1)
-						set_errno (PR_GetError ());
-				} while (nread == -1 && PR_GetError () == PR_PENDING_INTERRUPT_ERROR);
-			}
-		} while (nread == -1 && (PR_GetError () == PR_PENDING_INTERRUPT_ERROR ||
-					 PR_GetError () == PR_IO_PENDING_ERROR ||
-					 PR_GetError () == PR_WOULD_BLOCK_ERROR));
-		
-		/* restore O_NONBLOCK options */
-		error = errno;
-		sockopts.option = PR_SockOpt_Nonblocking;
-		sockopts.value.non_blocking = nonblock;
-		PR_SetSocketOption (tcp_stream_ssl->priv->sockfd, &sockopts);
-		errno = error;
-
-
-	} else {
-		PRSocketOptionData sockopts;
-		PRPollDesc pollfds[2];
-		gboolean nonblock;
-		int error;
-		
-		/* get O_NONBLOCK options */
-		sockopts.option = PR_SockOpt_Nonblocking;
-		PR_GetSocketOption (tcp_stream_ssl->priv->sockfd, &sockopts);
-		sockopts.option = PR_SockOpt_Nonblocking;
-		nonblock = sockopts.value.non_blocking;
-		sockopts.value.non_blocking = TRUE;
-		PR_SetSocketOption (tcp_stream_ssl->priv->sockfd, &sockopts);
-
-		pollfds[0].fd = tcp_stream_ssl->priv->sockfd;
-		pollfds[0].in_flags = PR_POLL_READ;
-		pollfds[1].fd = cancel_fd;
-		pollfds[1].in_flags = PR_POLL_READ;
-		
-		do {
-			PRInt32 res;
-
-			pollfds[0].out_flags = 0;
-			pollfds[1].out_flags = 0;
-			nread = -1;
-
-			res = PR_Poll(pollfds, 2, PR_TicksPerSecond () * IDLE_READ_TIMEOUT);
-
-			if (res == -1)
-				set_errno(PR_GetError());
-			else if (res == 0) {
-#ifdef ETIMEDOUT
-				errno = ETIMEDOUT;
-#else
-				errno = EIO;
-#endif
-			} else if (pollfds[1].out_flags == PR_POLL_READ) {
-				errno = EINTR;
-				goto failed;
-			} else {
-				do {
-					nread = PR_Read (tcp_stream_ssl->priv->sockfd, buffer, n);
-					if (nread == -1)
-						set_errno (PR_GetError ());
-				} while (nread == -1 && PR_GetError () == PR_PENDING_INTERRUPT_ERROR);
-			}
-		} while (nread == -1 && (PR_GetError () == PR_PENDING_INTERRUPT_ERROR ||
-					 PR_GetError () == PR_IO_PENDING_ERROR ||
-					 PR_GetError () == PR_WOULD_BLOCK_ERROR));
-		
-		/* restore O_NONBLOCK options */
-	failed:
-		error = errno;
-		sockopts.option = PR_SockOpt_Nonblocking;
-		sockopts.value.non_blocking = nonblock;
-		PR_SetSocketOption (tcp_stream_ssl->priv->sockfd, &sockopts);
-		errno = error;
-	}
-	
-	return nread;
-}
-
-
 static ssize_t
 stream_write (CamelStream *stream, const char *buffer, size_t n)
 {
Index: tests/c-demo/tny-demoui-summary-view.c
===================================================================
--- tests/c-demo/tny-demoui-summary-view.c	(revision 2872)
+++ tests/c-demo/tny-demoui-summary-view.c	(working copy)
@@ -940,11 +940,15 @@
 
 			set_header_view_model (header_view, sortable);
 
+#ifndef NONASYNC_TEST
 			tny_folder_refresh_async (folder, 
 				refresh_current_folder, 
 				status_update, self);
-
-			g_object_unref (G_OBJECT (folder));
+#else
+			tny_folder_refresh (folder, NULL);
+			refresh_current_folder (folder, FALSE, NULL, self);
+#endif
+			g_object_unref (folder);
 		}
 	  }
 	} else {
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 2874)
+++ ChangeLog	(working copy)
@@ -3,6 +3,12 @@
 	* Reference count problem in TnyCamelHeader
 	* priv->folder_name in TnyCamelFolder sometimes is NULL, which doesn't
 	seem right (and is racy).
+	* Improvements for the IDLE support. The Nonblocking read is now
+	actually used correctly, various racy situations should be fixed now
+	and instant event throwing is put in place (during IDLE state).
+	* Removed the (*read_idle) funcptr from CamelTcpStream, as this is no longer
+	required. This to reduce the complexity of the IDLE patch so that we
+	can later, perhaps, more easily bring this feature to upstream Camel.
 
 2007-10-19  Philip Van Hoof  <pvanhoof gnome org>
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]