Patch: avoid interlocks in let_idle_die



	Hi,

	This patch avoids some interlocks that can happen in the following
situation:
	* In a thread, we wait for joining the idle thread, after locking the
connect_lock (for example when we do a stop_idle in imap_command_start).
	* In the idle thread, let_idle_die stops trying to lock the
connect_lock.

	The cases I saw where this was happening were always when we tried to
stop idle to do a new command after, and then, the let_idle_die is not
that important. Then, I replaced the call to lock connect_lock with a
trylock.

Changelog would be:
* libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c
  (let_idle_die): do a trylock in connect_lock instead of lock, so that
  we can avoid interlocks on stop_idle called from imap_command_start.


-- 
José Dapena Paz <jdapena igalia com>
Igalia
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c	(revision 3762)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c	(working copy)
@@ -232,20 +232,23 @@
 		CamelException ex = CAMEL_EXCEPTION_INITIALISER;
 
 		idle_debug ("Sending DONE in let_idle_die\n");
-		CAMEL_SERVICE_REC_LOCK (store, connect_lock);
-		nwritten = camel_stream_printf (store->ostream, "DONE\r\n");
-		if (nwritten != -1) {
-			resp = NULL;
-			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;
+		/* We replace the lock with a try lock. If some other is retaining the connect lock, then we don't need that someone breaks
+		 * the idle */
+		if (CAMEL_SERVICE_REC_TRYLOCK (store, connect_lock)) {
+			nwritten = camel_stream_printf (store->ostream, "DONE\r\n");
+			if (nwritten != -1) {
+				resp = NULL;
+				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) {
+					idle_debug ("(.., ..) <- %s\n", resp);
+					g_free (resp);
+				}
 			}
-			if (resp) {
-				idle_debug ("(.., ..) <- %s\n", resp);
-				g_free (resp);
-			}
+			CAMEL_SERVICE_REC_UNLOCK (store, connect_lock);
 		}
-		CAMEL_SERVICE_REC_UNLOCK (store, connect_lock);
 		g_free (store->idle_prefix);
 		store->idle_prefix=NULL;
 	}


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