[PATCH] threadutil: add missing import os

Nicolas Sebrecht nicolas.s-dev at laposte.net
Thu May 12 23:56:03 BST 2011


On Tue, May 10, 2011 at 03:06:55AM +0200, Sebastian Spaeth wrote:
> On Mon,  9 May 2011 22:59:07 +0200, Nicolas Sebrecht <nicolas.s-dev at laposte.net> wrote:

> > Nobody seemed to raise a missing import which make me think this code is either
> > highly broken (we have dead code) or very safe. It would ask a closer review of
> > this area.
> 
> Good catch. The os.* calls are only exited on tread exit when
> 
>     if thread.getExitCause() == 'EXCEPTION':
>     ...
>     elif thread.getExitMessage() == 'SYNC_WITH_TIMER_TERMINATE':
>     ...
> 
> and I cannot remember that we set anywhere ExitCause or ExitMessage :-),
> so that code is probably not exercised at all. Yes, it calls for a review.

ExitCause() and ExitMessage() don't exists but setExitCause() and
setExitMessage() do.

The first is used in threadutil only and the latter is only used at 

	offlineimap/syncmaster.py:33:
		currentThread().setExitMessage('SYNC_WITH_TIMER_TERMINATE')

I still think we could simplify the thread mechanism, though.

-- >8 --
Subject: [PATCH] threadutil: remove broken os.exit()

We are missing the import of 'os' python module since commit d839be3c61888a837b
(Sat Apr 16 20:33:35 2005 +0100) which was when John switched from SVN to Git.

Happily, it help us today: we still had no feedback for this missing import,
6 years later.  So, we can remove the os.exit() call safely.

That beeing said, we still don't know if the above sys.exit() was ever touched.
My guess is that it never was. Keep this (hopefully) commented statement to
ensure the thread terminate and not play too much with the Murphy's law. :-)

Signed-off-by: Nicolas Sebrecht <nicolas.s-dev at laposte.net>
---

I still didn't reviewed the full thread process and may never have to.
I intended to do it but I've been stucked by the complexity behind these
getExitMessage/setExitMessage/semaphore for the meantime.

It may worth to merge this patch for now. It will fix a bug, actually.


 offlineimap/threadutil.py |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/offlineimap/threadutil.py b/offlineimap/threadutil.py
index 65028ee..155a563 100644
--- a/offlineimap/threadutil.py
+++ b/offlineimap/threadutil.py
@@ -122,12 +122,10 @@ def threadexited(thread):
             raise SystemExit
         ui.threadException(thread)      # Expected to terminate
         sys.exit(100)                   # Just in case...
-        os._exit(100)
     elif thread.getExitMessage() == 'SYNC_WITH_TIMER_TERMINATE':
         ui.terminate()
         # Just in case...
         sys.exit(100)
-        os._exit(100)
     else:
         ui.threadExited(thread)
 
-- 
1.7.5.1.354.g761178





More information about the OfflineIMAP-project mailing list