[PATCH v2] Simplify exit thread evaluation
leif.walsh at gmail.com
Wed Dec 1 22:46:34 GMT 2010
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():
> 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
> - 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)
> except Empty:
> - time.sleep(1)
> + pass
> def threadexited(thread):
> """Called when a thread exits."""
More information about the OfflineIMAP-project