[PATCH v2] Simplify exit thread evaluation

[Leif Walsh]

LGTM for now.  I'll let you know if I ever get the same deadlock I was
getting before with this code, but I think this probably solves it.  This
change over v1 definitely solves the signal handler bug (gg python).

The "proper" way to do this probably involves running
exitnotifymonitorloop in a daemon thread and reverting to the non-timeout
version (do we care to find out that threads have exited when we do want
to exit anyway?), but I think this is good enough for now.

My last nit to pick is the choice of 60 seconds.  I suppose it doesn't
really matter, but is there a number we can choose with more rationale
behind it?  Maybe the sync frequency or something a little bigger than it?

On Wed, 1 Dec 2010, Sebastian Spaeth wrote:

> Rather than sleeping for 1 second and poll our exitthread Queue in a
> non-bloking fashion simply call it in a blocking fashion. Works for
> me, and is somewhat faster as we don't finish our 1 second sleep even
> after a thread exited. This version v2 makes sure to work with SIGINT
> again (cf http://bugs.python.org/issue1360).
> 
> Signed-off-by: Sebastian Spaeth <Sebastian at SSpaeth.de>
> ---
> 
>  v1 of this patch actually fell victim to http://bugs.python.org/issue1360 (a Queue.get() call does not react to SIGINT unless it is also called with a timeout. Not very intuitive. Made sure that SIGINT works now with this version. It is still faster than the previous method
>  offlineimap/threadutil.py |   31 ++++++++++++++++++++-----------
>  1 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/offlineimap/threadutil.py b/offlineimap/threadutil.py
> index b516f68..c64734b 100644
> --- a/offlineimap/threadutil.py
> +++ b/offlineimap/threadutil.py
> @@ -101,22 +101,31 @@ def initexitnotify():
>      pass
>  
>  def exitnotifymonitorloop(callback):
> -    """Enter an infinite "monitoring" loop.  The argument, callback,
> -    defines the function to call when an ExitNotifyThread has terminated.
> -    That function is called with a single argument -- the ExitNotifyThread
> -    that has terminated.  The monitor will not continue to monitor for
> -    other threads until the function returns, so if it intends to perform
> -    long calculations, it should start a new thread itself -- but NOT
> -    an ExitNotifyThread, or else an infinite loop may result.  Furthermore,
> -    the monitor will hold the lock all the while the other thread is waiting.
> +    """An infinite "monitoring" loop watching for finished ExitNotifyThread's.
> +
> +    :param callback: the function to call when a thread terminated. That 
> +                     function is called with a single argument -- the 
> +                     ExitNotifyThread that has terminated. The monitor will 
> +                     not continue to monitor for other threads until
> +                     'callback' returns, so if it intends to perform long
> +                     calculations, it should start a new thread itself -- but
> +                     NOT an ExitNotifyThread, or else an infinite loop 
> +                     may result.
> +                     Furthermore, the monitor will hold the lock all the 
> +                     while the other thread is waiting.
> +    :type callback:  a callable function
>      """
>      global exitthreads
> -    while 1:                            # Loop forever.
> +    while 1:                            
> +        # Loop forever and call 'callback' for each thread that exited
>          try:
> -            thrd = exitthreads.get(False)
> +            # we need a timeout in the get() call, so that ctrl-c can throw
> +            # a SIGINT (http://bugs.python.org/issue1360). A timeout with empty
> +            # Queue will raise `Empty`.
> +            thrd = exitthreads.get(True, 60)
>              callback(thrd)
>          except Empty:
> -            time.sleep(1)
> +            pass
>  
>  def threadexited(thread):
>      """Called when a thread exits."""
> 

-- 
Cheers,
Leif