CFT: fix OfflineIMAP lockup in single-threaded mode

[Eygene Ryabinkin]

Cyril, good day.

Mon, Jan 28, 2013 at 05:15:53PM +0100, X Ryl wrote:
> Can you elaborate on the singleton approach ?

Yes: in our case, it essentially replaces the global variable with a
class that always has a single instance and a well-defined access
semantics that is encapsulated into the class itself, so callers can't
reliably modify the internal state, since there is no guaranteed
implementation, only the guaranteed semantics (read-only in our case).

Singleton is for a single instance, class is for encapsulation
and semantics.

> 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 ?

My current implementation doesn't singleton'ize the 'config', it
applies singleton to 'options' that have read/write access.  But, as
you can see, 'config' variable from offlineimap/init.py is just a
class that holds the configuration items and they can be modified,
e.g. by using config.set() method.

If we will make, say, 'options' to the global variable, some code can
unintentionally modify it and this will be very hard to trace, since
any piece of code can do it: variable is global.

> However, two point I don't like in your implementation:
> 1) Not sure about the requirement of the Singleton class

We should base our derived classes on some common concept.  If you
have to have depot for options and depot for configuration (with
different, say, access methods), will you repeat yourself and
code these two depot's __new__() method twice?

> (and if required, let's put it in a Singleton.py file, not __init.py
> file)

Anything that is put to offlineimap/utils/Singleton.py will have a
module prefix of offlineimap.utils.Singleton, so if we will put
Singleton class here, it will be named
  offlineimap.utils.Singleton.Singleton
and this is somewhat redundant.

> 2) The Option object should be lowercase

Class names are CamelCase by PEP8,
  http://www.python.org/dev/peps/pep-0008/#class-names
and that's what I see almost consistently across all OfflineIMAP code:
{{{
$ grep -r '^class[[:space:]]\{1,\}[a-z]' .
./offlineimap/threadutil.py:class threadlist:
}}}

> (all the other members are lowercase,

Members of what?

> so it's easy to use a "option" member instead of "Option"

There is no depot.option, so I am not sure what you meant here.

Thanks!
-- 
rea