CFT: fix OfflineIMAP lockup in single-threaded mode

X Ryl boite.pour.spam at gmail.com
Mon Jan 28 16:15:53 UTC 2013


Hi,

 Can you elaborate on the singleton approach ?
I thought the config was read-only already, and read before any thread is
spawn, so I don't get why it would require a singleton class ?
That being said, I like the idea of a "depot" for the common, "behavior
related" options.
Having to create a "key" for searching the "general" section of the config
file is painful, and the number of such options is limited anyway, so it
can be moved out the config object.
However, two point I don't like in your implementation:
1) Not sure about the requirement of the Singleton class (and if required,
let's put it in a Singleton.py file, not __init.py file)
2) The Option object should be lowercase (all the other members are
lowercase, so it's easy to use a "option" member instead of "Option"

Cheers,
Cyril

On Sun, Jan 27, 2013 at 11:10 PM, Eygene Ryabinkin <rea at codelabs.ru> wrote:

> Sun, Jan 27, 2013 at 08:39:13PM +0100, Nicolas Sebrecht wrote:
> > Can't say whether the patch really fixes the issue
>
> It should and in my testing -- is, but I am still waiting for Cyril's
> review.
>
> > but it's very pleasant to see work like that and activity on the
> > repository in general (like Dmitrijs did recently).
>
> Thanks!
>
> > > One thing that worries me currently is the way that
> options.singlethreading
> > > is propagated from the main code: adding new pseudo-item to the
> 'general'
> > > configuration section isn't that good.  Something like the singleton
> that
> > > will provide read-only access to the options is a better way, in my
> view.
> >
> > Well, as long as contributors and users are aware of what's going on and
> > as long as whatever is released as stable is not experimental stuff,
> > everything should be ok.
>
> Yes.  I had implemented read-only depot for options, the modified patch
> is at
>
> http://codelabs.ru/patches/offlineimap/2013-fix-deadlock-singlethreaded-IMAP-sync.diff
>
> It additionally teaches folder/IMAP.py not to suggest multithreading while
> OfflineIMAP works in single-threaded mode: it is simple, because now we
> have global-like object that has all options.
>
> > So, about this subtle and critical topic I'd say it's fine to go step by
> > step. This would prevent from non-required headaches and could later
> > help to distinguish newly introduced issues coming from the "core" fix
> > and the "polishing" commit.
>
> Well, I had missed this paragraph while I was polishing my commit.  Will
> split it into two tomorrow.
> --
> rea
>
> _______________________________________________
> OfflineIMAP-project mailing list
> OfflineIMAP-project at lists.alioth.debian.org
> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/offlineimap-project
>
> OfflineIMAP homepage: http://software.complete.org/offlineimap
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/offlineimap-project/attachments/20130128/da55caea/attachment.html>


More information about the OfflineIMAP-project mailing list