From cdbef90ec450a129ceecdf02a2f7bc1e7069a7be Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Thu, 17 Jan 2013 12:02:41 +0100 Subject: [PATCH] Fix deadlock for IMAP folder synced in singlethreaded mode The problem lies in the fact that offlineimap.folder.Base's method syncmessagesto_copy() uses threaded code everytime it is suggested by the derived class's suggeststhreads() (currently, only IMAP does this suggestion), but offlineimap/init.py will not spawn the exitnotifymonitorloop() from offlineimap.threadutil. The root cause is that ExitNotifyThread-derived threads need offlineimap.threadutil's exitnotifymonitorloop() to be running the cleaner for the exitthreads Queue(), because it fills the queue via the run() method from this class: it wants to put() itself to the Queue on exit, so when no exitnotifymonitorloop() is running, the queue will fill up. And if this thread is an instance of InstanceLimitedThread that hits the limit on the number of threads, then it will hold the instancelimitedsems[] semaphore will prevent other InstanceLimitedThread()s of the same name to pass its start() method. The fix is to avoid using threaded code if we're running single-threaded. Signed-off-by: Eygene Ryabinkin Obtained-from: X-Ryl669 --- Changelog.rst | 3 ++- offlineimap/accounts.py | 20 +++++++++++--------- offlineimap/folder/Base.py | 2 +- offlineimap/init.py | 3 +++ offlineimap/threadutil.py | 2 ++ 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/Changelog.rst b/Changelog.rst index 7d8992a..6172b34 100644 --- a/Changelog.rst +++ b/Changelog.rst @@ -7,7 +7,8 @@ ChangeLog WIP (add new stuff for the next release) ======================================== -======= +* Avoid lockups for IMAP synchronizations running with the + "-1" command-line switch (X-Ryl669 ) * SIGHUP is now handled as the termination notification rather than the signal to reread the configuration (Dmitrijs Ledkovs) * Honor the timezone of emails (Tobias Thierer) diff --git a/offlineimap/accounts.py b/offlineimap/accounts.py index dd65842..88d62d8 100644 --- a/offlineimap/accounts.py +++ b/offlineimap/accounts.py @@ -321,13 +321,16 @@ class SyncableAccount(Account): self.ui.debug('', "Not syncing filtered folder '%s'" "[%s]" % (localfolder, localfolder.repository)) continue # Ignore filtered folder - thread = InstanceLimitedThread(\ - instancename = 'FOLDER_' + self.remoterepos.getname(), - target = syncfolder, - name = "Folder %s [acc: %s]" % (remotefolder, self), - args = (self, remotefolder, quick)) - thread.start() - folderthreads.append(thread) + if self.config.get('general', 'single-thread') == 'False': + thread = InstanceLimitedThread(\ + instancename = 'FOLDER_' + self.remoterepos.getname(), + target = syncfolder, + name = "Folder %s [acc: %s]" % (remotefolder, self), + args = (self, remotefolder, quick)) + thread.start() + folderthreads.append(thread) + else: + syncfolder(self, remotefolder, quick) # wait for all threads to finish for thr in folderthreads: thr.join() @@ -372,8 +375,7 @@ class SyncableAccount(Account): self.ui.error(e, exc_info()[2], msg = "Calling hook") def syncfolder(account, remotefolder, quick): - """This function is called as target for the - InstanceLimitedThread invokation in SyncableAccount. + """Synchronizes given remote folder for the specified account. Filtered folders on the remote side will not invoke this function.""" remoterepos = account.remoterepos diff --git a/offlineimap/folder/Base.py b/offlineimap/folder/Base.py index c132713..e330086 100644 --- a/offlineimap/folder/Base.py +++ b/offlineimap/folder/Base.py @@ -403,7 +403,7 @@ class BaseFolder(object): break self.ui.copyingmessage(uid, num+1, num_to_copy, self, dstfolder) # exceptions are caught in copymessageto() - if self.suggeststhreads(): + if self.suggeststhreads() and self.config.get('general', 'single-thread') == 'False': self.waitforthread() thread = threadutil.InstanceLimitedThread(\ self.getcopyinstancelimit(), diff --git a/offlineimap/init.py b/offlineimap/init.py index 94a9a18..67b44db 100644 --- a/offlineimap/init.py +++ b/offlineimap/init.py @@ -250,6 +250,9 @@ class OfflineImap: if type.lower() == 'imap': imaplib.Debug = 5 + # XXX: can we avoid introducing fake configuration item? + config.set_if_not_exists('general', 'single-thread', 'True' if options.singlethreading else 'False') + if options.runonce: # FIXME: maybe need a better for section in accounts.getaccountlist(config): diff --git a/offlineimap/threadutil.py b/offlineimap/threadutil.py index a29e68f..33dbd64 100644 --- a/offlineimap/threadutil.py +++ b/offlineimap/threadutil.py @@ -39,6 +39,8 @@ def semaphorereset(semaphore, originalstate): semaphore.release() class threadlist: + """Store the list of all threads in the software so it can be used to find out + what's running and what's not.""" def __init__(self): self.lock = Lock() self.list = [] -- 1.8.1