issue with exception handling in single-threaded mode (was Re: irreversible silent loss of mail when server account is full)

[Adam Spiers]

On Wed, Mar 27, 2013 at 06:56:07PM +0000, Adam Spiers wrote:

[snipped]

>     https://github.com/OfflineIMAP/offlineimap/pull/29
> 
> However, this led me to notice a separate issue where single-threaded
> mode doesn't properly handle exceptions.  I'll send a separate mail
> about that.

My fix in the above pull request raises an OfflineImapError of type
ERROR.REPO, which causes the sync thread to abort, and *should* cause
the sync runner to abort execution of any further threads for this
repository.  But it only aborts correctly in multi-threaded mode, not
in single-threaded mode.  The explanation is as follows:

ExitNotifyThread.run() catches the OfflineImapError exception and
stores it:

        except Exception as e:
            # Thread exited with Exception, store it
            tb = traceback.format_exc()
            self.set_exit_exception(e, tb)

However, the code which later accesses this stored exception and
propagates it to the UI is in threadutils.threadexited(), which is
only invoked in multi-threaded mode, as seen in this extract of
OfflineIMAP.sync():

        if options.singlethreading:
            #singlethreaded
            self.sync_singlethreaded(syncaccounts)
        else:
            # multithreaded
            t = threadutil.ExitNotifyThread(target=syncmaster.syncitall,
                             name='Sync Runner',
                             kwargs = {'accounts': syncaccounts,
                                       'config': self.config})
            t.start()
            threadutil.exitnotifymonitorloop(threadutil.threadexited)

The thread-handling code is non-trivial so I can't propose a fix
without wading in deeper, which unfortunately I don't have time to do
right now.  The important thing for now is that my pull request above
prevents any further mail loss.  The UI output in single-threaded mode
silently aborts the APPEND without explanation, which is not good, but
significantly better than the previous behaviour.

I would be very grateful if someone could validate my analysis and let
me know whether I should submit a github issue for this.

Cheers!
Adam