[evolution-data-server] Bug #666327 - IMAP deadlock when moving message and checking new mail



commit 7a4162dcc372fbca2405056f92eef7bec4c7400f
Author: Alban Browaeys <prahal yahoo com>
Date:   Thu May 31 14:33:03 2012 +0200

    Bug #666327 - IMAP deadlock when moving message and checking new mail
    
    with service lock removal the fix for bug #666327
    went out (commit 74fcab535c0f50a27742c05e94036b8370ea9173).
    Was a good thing as this version is less a hack.
    To summarize:
    imap folder deadlock:
    1. do_copy :
    	.imap command on source lock the imap store
    2. camel_imap_folder_fetch_data on destination:
    	. lock the destination folder
    3. do_copy :
    	. call the hande user tag : wait for lock on the destination
    folder cache	.
    4. camel_imap_folder_fetch_data on destination:
    	. wait for lock on the imap store to be freed.
    
    Fix: get the lock on the destination folder cache before locking the
    imap store (ie before calling the imap command).

 camel/providers/imap/camel-imap-folder.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)
---
diff --git a/camel/providers/imap/camel-imap-folder.c b/camel/providers/imap/camel-imap-folder.c
index 9570fd7..ee9a69f 100644
--- a/camel/providers/imap/camel-imap-folder.c
+++ b/camel/providers/imap/camel-imap-folder.c
@@ -2609,6 +2609,7 @@ imap_transfer_offline (CamelFolder *source,
 	return TRUE;
 }
 
+/* Call with lock held on destination folder cache */
 static void
 handle_copyuid (CamelImapResponse *response,
                 CamelFolder *source,
@@ -2644,7 +2645,6 @@ handle_copyuid (CamelImapResponse *response,
 		 * command lock too, so no one else could be here.
 		 */
 		CAMEL_IMAP_FOLDER_REC_LOCK (source, cache_lock);
-		CAMEL_IMAP_FOLDER_REC_LOCK (destination, cache_lock);
 		for (i = 0; i < src->len; i++) {
 			camel_imap_message_cache_copy (scache, src->pdata[i],
 						       dcache, dest->pdata[i]);
@@ -2652,7 +2652,6 @@ handle_copyuid (CamelImapResponse *response,
 			imap_folder_add_ignore_recent (CAMEL_IMAP_FOLDER (destination), dest->pdata[i]);
 		}
 		CAMEL_IMAP_FOLDER_REC_UNLOCK (source, cache_lock);
-		CAMEL_IMAP_FOLDER_REC_UNLOCK (destination, cache_lock);
 
 		imap_uid_array_free (src);
 		imap_uid_array_free (dest);
@@ -2667,6 +2666,8 @@ handle_copyuid (CamelImapResponse *response,
 	g_warning ("Bad COPYUID response from server");
 }
 
+
+/* Call with lock held on destination folder cache */
 static void
 handle_copyuid_copy_user_tags (CamelImapResponse *response,
                                CamelFolder *source,
@@ -2708,12 +2709,7 @@ handle_copyuid_copy_user_tags (CamelImapResponse *response,
 	dest = imap_uid_set_to_array (destination->summary, destset);
 
 	if (src && dest && src->len == dest->len) {
-		/* We don't have to worry about deadlocking on the
-		 * cache locks here, because we've got the store's
-		 * command lock too, so no one else could be here.
-		 */
 		CAMEL_IMAP_FOLDER_REC_LOCK (source, cache_lock);
-		CAMEL_IMAP_FOLDER_REC_LOCK (destination, cache_lock);
 		for (i = 0; i < src->len; i++) {
 			CamelMessageInfo *mi = camel_folder_get_message_info (source, src->pdata[i]);
 
@@ -2729,7 +2725,6 @@ handle_copyuid_copy_user_tags (CamelImapResponse *response,
 			}
 		}
 		CAMEL_IMAP_FOLDER_REC_UNLOCK (source, cache_lock);
-		CAMEL_IMAP_FOLDER_REC_UNLOCK (destination, cache_lock);
 
 		imap_uid_array_free (src);
 		imap_uid_array_free (dest);
@@ -2823,6 +2818,7 @@ do_copy (CamelFolder *source,
 			/* returns only 'A00012 OK UID XGWMOVE completed' '* 2 XGWMOVE' so nothing useful */
 			camel_imap_response_free (store, response);
 		} else {
+			CAMEL_IMAP_FOLDER_REC_LOCK (destination, cache_lock);
 			response = camel_imap_command (
 				store, source, cancellable, &local_error,
 				"UID COPY %s %F", uidset, full_name);
@@ -2833,6 +2829,7 @@ do_copy (CamelFolder *source,
 					response, source, destination,
 					cancellable);
 			camel_imap_response_free (store, response);
+			CAMEL_IMAP_FOLDER_REC_UNLOCK (destination, cache_lock);
 		}
 
 		if (local_error == NULL && delete_originals && (mark_moved || !trash_path)) {



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