On Thu, 2004-07-15 at 15:12 +0800, Not Zed wrote: > > This doesn't look good, shouldn't be poking the other folders > internals, more than it already is anyway. > > I think it would be better to just not call camel_folder_refresh_info > with the connect lock held. Infact since we're only comparing the > current folder which is the only lockable item, it should be safe to > remove the connect locks enitrely. refresh_info will lock in the > right order. ok... if we remove the connect_lock, isn't there a race checking dest != current? or maybe that just doesn't matter...? I've attached a patch with the removed connect_locks but I'd like to know the answer to my above concern before I'd be willing to commit it. Jeff > > On Wed, 2004-07-14 at 15:38 -0400, Jeffrey Stedfast wrote: > > http://bugzilla.ximian.com/show_bug.cgi?id=61551 > > > > deadlock condition where one thread has the connect_lock and tries to > > get the folder_lock and another thread which already has the folder_lock > > tries to get the connect_lock. > > > > Jeff > > > > Plain text document attachment (61551.patch) > > ? 55280.patch > > ? 61538.patch > > ? 61551.patch > > ? ChangeLog.nonximian > > ? body > > ? body.c > > ? body.txt > > ? body2.txt > > ? braindamaged.patch > > ? camel-namespace.patch > > ? cf.c > > ? charset-map.c > > ? charset-map.diff > > ? class.sh > > ? cmsutil.c > > ? date.patch > > ? flags > > ? foo > > ? foo.txt > > ? foo2.txt > > ? gw-body.txt > > ? imap > > ? invalid-content-id.patch > > ? iso > > ? iso.c > > ? namespace.sh > > ? patch > > ? pop3-uidl.patch > > ? smime > > ? uid-cache.patch > > ? providers/tmp > > ? providers/local/camel-mozilla-folder.c > > ? providers/local/camel-mozilla-folder.h > > ? providers/local/camel-mozilla-store.c > > ? providers/local/camel-mozilla-store.h > > ? tests/data/camel-mime-tests.tar.gz > > Index: ChangeLog > > =================================================================== > > RCS file: /cvs/gnome/evolution/camel/ChangeLog,v > > retrieving revision 1.2219 > > diff -u -r1.2219 ChangeLog > > --- ChangeLog 14 Jul 2004 19:00:37 -0000 1.2219 > > +++ ChangeLog 14 Jul 2004 19:25:16 -0000 > > @@ -1,5 +1,11 @@ > > 2004-07-14 Jeffrey Stedfast <fejj novell com> > > > > + * providers/imap/camel-imap-folder.c (imap_transfer_online): Grab > > + the folder lock before grabbing the connect_lock when refreshing > > + the dest folder's info to avoid the deadlock in bug #61551. > > + > > +2004-07-14 Jeffrey Stedfast <fejj novell com> > > + > > Fix for bug #61538 > > > > * camel-process.c (camel_process_fork): Same. > > Index: providers/imap/camel-imap-folder.c > > =================================================================== > > RCS file: /cvs/gnome/evolution/camel/providers/imap/camel-imap-folder.c,v > > retrieving revision 1.336 > > diff -u -r1.336 camel-imap-folder.c > > --- providers/imap/camel-imap-folder.c 20 May 2004 19:16:53 -0000 1.336 > > +++ providers/imap/camel-imap-folder.c 14 Jul 2004 19:25:16 -0000 > > @@ -1481,11 +1481,13 @@ > > return; > > > > /* Make the destination notice its new messages */ > > + CAMEL_FOLDER_LOCK(dest, lock); > > CAMEL_SERVICE_LOCK (store, connect_lock); > > if (store->current_folder != dest || > > camel_folder_summary_count (dest->summary) == count) > > camel_folder_refresh_info (dest, ex); > > CAMEL_SERVICE_UNLOCK (store, connect_lock); > > + CAMEL_FOLDER_UNLOCK(dest, lock); > > > > if (delete_originals) { > > for (i = 0; i < uids->len; i++) > -- > > Michael Zucchi <notzed ximian com> > "born to die, live to work, it's > all downhill from here" > Novell's Evolution and Free > Software Developer -- Jeffrey Stedfast Evolution Hacker - Novell, Inc. fejj ximian com - www.novell.com
? 55280.patch
? 61551.patch
? ChangeLog.nonximian
? body
? body.c
? body.txt
? body2.txt
? braindamaged.patch
? camel-namespace.patch
? cf.c
? charset-map.c
? charset-map.diff
? class.sh
? cmsutil.c
? date.patch
? flags
? foo
? foo.txt
? foo2.txt
? gw-body.txt
? imap
? invalid-content-id.patch
? iso
? iso.c
? namespace.sh
? patch
? pop3-uidl.patch
? smime
? uid-cache.patch
? providers/tmp
? providers/local/camel-mozilla-folder.c
? providers/local/camel-mozilla-folder.h
? providers/local/camel-mozilla-store.c
? providers/local/camel-mozilla-store.h
? tests/data/camel-mime-tests.tar.gz
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/camel/ChangeLog,v
retrieving revision 1.2219
diff -u -r1.2219 ChangeLog
--- ChangeLog 14 Jul 2004 19:00:37 -0000 1.2219
+++ ChangeLog 15 Jul 2004 17:36:48 -0000
@@ -1,3 +1,9 @@
+2004-07-15 Jeffrey Stedfast <fejj novell com>
+
+ * providers/imap/camel-imap-folder.c (imap_transfer_online): Don't
+ grab the connect_lock before calling refresh_info so that we avoid
+ the deadlock in bug #61551.
+
2004-07-14 Jeffrey Stedfast <fejj novell com>
Fix for bug #61538
Index: providers/imap/camel-imap-folder.c
===================================================================
RCS file: /cvs/gnome/evolution/camel/providers/imap/camel-imap-folder.c,v
retrieving revision 1.336
diff -u -r1.336 camel-imap-folder.c
--- providers/imap/camel-imap-folder.c 20 May 2004 19:16:53 -0000 1.336
+++ providers/imap/camel-imap-folder.c 15 Jul 2004 17:36:48 -0000
@@ -1481,11 +1481,9 @@
return;
/* Make the destination notice its new messages */
- CAMEL_SERVICE_LOCK (store, connect_lock);
if (store->current_folder != dest ||
camel_folder_summary_count (dest->summary) == count)
camel_folder_refresh_info (dest, ex);
- CAMEL_SERVICE_UNLOCK (store, connect_lock);
if (delete_originals) {
for (i = 0; i < uids->len; i++)
Attachment:
smime.p7s
Description: S/MIME cryptographic signature