rev 314 - in offlineimap/head/offlineimap: . ui

[email protected] Mon, 6 Jan 2003 09:58:30 -0600 (CST)
Newsgroups gmane.mail.imap.offlineimap.subversion
Message-ID <[email protected]>
Author: jgoerzen
Date: 2003-01-06 09:58:29 -0600 (Mon, 06 Jan 2003)
New Revision: 314

Modified:
   offlineimap/head/offlineimap/threadutil.py
   offlineimap/head/offlineimap/ui/Blinkenlights.py
   offlineimap/head/offlineimap/ui/Curses.py
   offlineimap/head/offlineimap/ui/debuglock.py
Log:
More locking updates.

Introduced a new MultiLock to threadutil.  This lock will let a single
thread acquire the same lock more than once, keeping track of how many times
this happens, and will release the actual lock only when the lock's
lock count gets back to zero.

By using MultiLock, various functions in Curses.py and Blinkenlights.py no
longer need to pass around to other functions a parameter indicating whether
or not a lock should be obtained.  This was a large cause of complexity and
errors, which is now eliminated.

Everything seems to be working properly wrt locking at this point.  The
Curses.Blinkenlights interface has achieved basic working functionality.



Modified: offlineimap/head/offlineimap/threadutil.py
==============================================================================
--- offlineimap/head/offlineimap/threadutil.py	(original)
+++ offlineimap/head/offlineimap/threadutil.py	Mon Jan  6 09:58:30 2003
@@ -236,3 +236,54 @@
             instancelimitedsems[self.instancename].release()
         
     
+######################################################################
+# Multi-lock -- capable of handling a single thread requesting a lock
+# multiple times
+######################################################################
+
+class MultiLock:
+    def __init__(self):
+        self.lock = Lock()
+        self.statuslock = Lock()
+        self.locksheld = {}
+
+    def acquire(self):
+        """Obtain a lock.  Provides nice support for a single
+        thread trying to lock it several times -- as may be the case
+        if one I/O-using object calls others, while wanting to make it all
+        an atomic operation.  Keeps a "lock request count" for the current
+        thread, and acquires the lock when it goes above zero, releases when
+        it goes below one.
+
+        This call is always blocking."""
+        
+        # First, check to see if this thread already has a lock.
+        # If so, increment the lock count and just return.
+        self.statuslock.acquire()
+        try:
+            threadid = thread.get_ident()
+
+            if threadid in self.locksheld:
+                self.locksheld[threadid] += 1
+                return
+            else:
+                # This is safe because it is a per-thread structure
+                self.locksheld[threadid] = 1
+        finally:
+            self.statuslock.release()
+        self.lock.acquire()
+
+    def release(self):
+        self.statuslock.acquire()
+        try:
+            threadid = thread.get_ident()
+            if self.locksheld[threadid] > 1:
+                self.locksheld[threadid] -= 1
+                return
+            else:
+                del self.locksheld[threadid]
+                self.lock.release()
+        finally:
+            self.statuslock.release()
+
+        

Modified: offlineimap/head/offlineimap/ui/Blinkenlights.py
==============================================================================
--- offlineimap/head/offlineimap/ui/Blinkenlights.py	(original)
+++ offlineimap/head/offlineimap/ui/Blinkenlights.py	Mon Jan  6 09:58:30 2003
@@ -19,6 +19,7 @@
 from threading import *
 from offlineimap.ui.UIBase import UIBase
 import thread
+from offlineimap.threadutil import MultiLock
 
 from debuglock import DebuggingLock
 
@@ -75,7 +76,7 @@
     def init_banner(s):
         s.availablethreadframes = {}
         s.threadframes = {}
-        s.tflock = DebuggingLock('tflock')
+        s.tflock = MultiLock()
 
     def threadExited(s, thread):
         threadid = thread.threadid
@@ -92,12 +93,11 @@
 
         UIBase.threadExited(s, thread)
 
-    def gettf(s, lock = 1):
+    def gettf(s):
         threadid = thread.get_ident()
         accountname = s.getthreadaccount()
 
-        if lock:
-            s.tflock.acquire()
+        s.tflock.acquire()
 
         try:
             if not accountname in s.threadframes:
@@ -111,14 +111,12 @@
 
             if len(s.availablethreadframes[accountname]):
                 tf = s.availablethreadframes[accountname].pop(0)
-                tf.setthread(currentThread(), lock)
+                tf.setthread(currentThread())
             else:
-                tf = s.getaccountframe().getnewthreadframe(lock)
+                tf = s.getaccountframe().getnewthreadframe()
             s.threadframes[accountname][threadid] = tf
             return tf
-        
         finally:
-            if lock:
-                s.tflock.release()
-        
+            s.tflock.release()
+            
 

Modified: offlineimap/head/offlineimap/ui/debuglock.py
==============================================================================
--- offlineimap/head/offlineimap/ui/debuglock.py	(original)
+++ offlineimap/head/offlineimap/ui/debuglock.py	Mon Jan  6 09:58:30 2003
@@ -19,6 +19,7 @@
 from threading import *
 import traceback
 logfile = open("/tmp/logfile", "wt")
+loglock = Lock()
 
 class DebuggingLock:
     def __init__(self, name):
@@ -28,16 +29,21 @@
     def acquire(self, blocking = 1):
         self.print_tb("Acquire lock")
         self.lock.acquire(blocking)
-        logfile.write("===== %s: Thread %s acquired lock\n" % (self.name, currentThread().getName()))
+        self.logmsg("===== %s: Thread %s acquired lock\n" % (self.name, currentThread().getName()))
 
     def release(self):
         self.print_tb("Release lock")
         self.lock.release()
 
-    def print_tb(self, msg):
-        logfile.write("==== %s: Thread %s attempting to %s\n" % \
-                           (self.name, currentThread().getName(), msg))
-        logfile.write("\n".join(traceback.format_list(traceback.extract_stack())))
-        logfile.write("\n")
+    def logmsg(self, msg):
+        loglock.acquire()
+        logfile.write(msg + "\n")
         logfile.flush()
+        loglock.release()
+
+    def print_tb(self, msg):
+        self.logmsg(".... %s: Thread %s attempting to %s\n" % \
+                    (self.name, currentThread().getName(), msg) + \
+                    "\n".join(traceback.format_list(traceback.extract_stack())))
+        
 

Modified: offlineimap/head/offlineimap/ui/Curses.py
==============================================================================
--- offlineimap/head/offlineimap/ui/Curses.py	(original)
+++ offlineimap/head/offlineimap/ui/Curses.py	Mon Jan  6 09:58:30 2003
@@ -19,7 +19,9 @@
 from Blinkenlights import BlinkenBase
 from UIBase import UIBase
 from threading import *
+import thread
 from offlineimap import version, threadutil
+from offlineimap.threadutil import MultiLock
 
 import curses, curses.panel, curses.textpad, curses.wrapper
 from debuglock import DebuggingLock
@@ -31,6 +33,27 @@
         self.start()
         self.nextpair = 1
         self.pairlock = Lock()
+        self.iolock = MultiLock()
+
+    def lock(self):
+        self.iolock.acquire()
+
+    def unlock(self):
+        self.iolock.release()
+        
+    def locked(self, target, *args, **kwargs):
+        """Perform an operation with full locking."""
+        self.lock()
+        try:
+            apply(target, args, kwargs)
+        finally:
+            self.unlock()
+
+    def refresh(self):
+        def lockedstuff():
+            curses.panel.update_panels()
+            curses.doupdate()
+        self.locked(lockedstuff)
 
     def isactive(self):
         return hasattr(self, 'stdscr')
@@ -85,31 +108,29 @@
         self.start()
 
 class CursesAccountFrame:
-    def __init__(s, master, accountname, iolock):
-        s.iolock = iolock
+    def __init__(s, master, accountname):
         s.c = master
         s.children = []
         s.accountname = accountname
 
-    def setwindow(s, window, lock = 1):
+    def setwindow(s, window):
         s.window = window
         acctstr = '%15.15s: ' % s.accountname
         s.window.addstr(0, 0, acctstr)
         s.location = len(acctstr)
         for child in s.children:
-            child.update(window, 0, s.location, lock)
+            child.update(window, 0, s.location)
             s.location += 1
 
-    def getnewthreadframe(s, lock = 1):
-        tf = CursesThreadFrame(s.c, s.window, 0, s.location, s.iolock, lock)
+    def getnewthreadframe(s):
+        tf = CursesThreadFrame(s.c, s.window, 0, s.location)
         s.location += 1
         s.children.append(tf)
         return tf
 
 class CursesThreadFrame:
-    def __init__(s, master, window, y, x, iolock, lock = 1):
+    def __init__(s, master, window, y, x):
         """master should be a CursesUtil object."""
-        s.iolock = iolock
         s.c = master
         s.window = window
         s.x = x
@@ -128,34 +149,30 @@
                          'yellow': curses.A_BOLD | s.c.getpair(curses.COLOR_YELLOW, bg),
                          'pink': curses.A_BOLD | s.c.getpair(curses.COLOR_RED, bg)}
         #s.setcolor('gray')
-        s.setcolor('black', lock)
+        s.setcolor('black')
 
-    def setcolor(self, color, lock = 1):
+    def setcolor(self, color):
         self.color = self.colormap[color]
-        self.display(lock)
+        self.display()
 
-    def display(self, lock = 1):
-        if lock:
-            self.iolock.acquire()
-        try:
+    def display(self):
+        def lockedstuff():
             self.window.addstr(self.y, self.x, '.', self.color)
             self.c.stdscr.move(self.c.height - 1, self.c.width - 1)
             self.window.refresh()
-        finally:
-            if lock:
-                self.iolock.release()
+        self.c.locked(lockedstuff)
 
     def getcolor(self):
         return self.color
 
-    def update(self, window, y, x, lock = 1):
+    def update(self, window, y, x):
         self.window = window
         self.y = y
         self.x = x
-        self.display(lock)
+        self.display()
 
-    def setthread(self, newthread, lock = 1):
-        self.setcolor('black', lock)
+    def setthread(self, newthread):
+        self.setcolor('black')
         #if newthread:
         #    self.setcolor('gray')
         #else:
@@ -234,13 +251,12 @@
         
 class Blinkenlights(BlinkenBase, UIBase):
     def init_banner(s):
-        s.iolock = DebuggingLock('iolock')
         s.af = {}
         s.aflock = DebuggingLock('aflock')
         s.c = CursesUtil()
         s.text = []
         BlinkenBase.init_banner(s)
-        s.setupwindows(dolock = 0)
+        s.setupwindows()
         s.inputhandler = InputHandler(s.c)
         s.gettf().setcolor('red')
         s._msg(version.banner)
@@ -251,22 +267,26 @@
 
     def getpass(s, accountname, config, errmsg = None):
         s.inputhandler.input_acquire()
-        s.iolock.acquire()
+
+        # See comment on _msg for info on why both locks are obtained.
+        
+        s.tflock.acquire()
+        s.c.lock()
         try:
-            s.gettf(lock = 0).setcolor('white', lock = 0)
-            s._addline_unlocked(" *** Input Required", s.gettf().getcolor())
-            s._addline_unlocked(" *** Please enter password for account %s: " % accountname,
-                   s.gettf(lock = 0).getcolor())
+            s.gettf().setcolor('white')
+            s._addline(" *** Input Required", s.gettf().getcolor())
+            s._addline(" *** Please enter password for account %s: " % accountname,
+                   s.gettf().getcolor())
             s.logwindow.refresh()
             password = s.logwindow.getstr()
         finally:
-            s.iolock.release()
+            s.tflock.release()
+            s.c.unlock()
             s.inputhandler.input_release()
         return password
 
-    def setupwindows(s, dolock = 1):
-        if dolock:
-            s.iolock.acquire()
+    def setupwindows(s):
+        s.c.lock()
         try:
             s.bannerwindow = curses.newwin(1, s.c.width, 0, 0)
             s.setupwindow_drawbanner()
@@ -282,13 +302,12 @@
             pos = s.c.height - 1
             for account in accounts:
                 accountwindow = curses.newwin(1, s.c.width, pos, 0)
-                s.af[account].setwindow(accountwindow, lock = 0)
+                s.af[account].setwindow(accountwindow)
                 pos -= 1
 
             curses.doupdate()
         finally:
-            if dolock:
-                s.iolock.release()
+            s.c.unlock()
 
     def setupwindow_drawbanner(s):
         s.bannerwindow.bkgd(' ', curses.A_BOLD | \
@@ -315,11 +334,13 @@
                 return s.af[accountname]
 
             # New one.
-            s.af[accountname] = CursesAccountFrame(s.c, accountname, s.iolock)
-            s.iolock.acquire()
-            s.c.reset()
-            s.setupwindows(dolock = 0)
-            s.iolock.release()
+            s.af[accountname] = CursesAccountFrame(s.c, accountname)
+            s.c.lock()
+            try:
+                s.c.reset()
+                s.setupwindows()
+            finally:
+                s.c.unlock()
         finally:
             s.aflock.release()
         return s.af[accountname]
@@ -330,25 +351,37 @@
             for thisline in msg.split("\n"):
                 s._msg(thisline)
             return
-        s.iolock.acquire()
+
+        # We must acquire both locks.  Otherwise, deadlock can result.
+        # This can happen if one thread calls _msg (locking curses, then
+        # tf) and another tries to set the color (locking tf, then curses)
+        #
+        # By locking both up-front here, in this order, we prevent deadlock.
+        
+        s.tflock.acquire()
+        s.c.lock()
         try:
             if not s.c.isactive():
                 # For dumping out exceptions and stuff.
                 print msg
                 return
             if color:
-                s.gettf(lock = 0).setcolor(color, lock = 0)
-            s._addline_unlocked(msg, s.gettf(lock = 0).getcolor())
+                s.gettf().setcolor(color)
+            s._addline(msg, s.gettf().getcolor())
             s.logwindow.refresh()
         finally:
-            s.iolock.release()
+            s.c.unlock()
+            s.tflock.release()
 
-    def _addline_unlocked(s, msg, color):
-        s.logwindow.addstr(msg + "\n", color)
-        s.text.append((msg, color))
-        while len(s.text) > s.logheight:
-            s.text = s.text[1:]
-        
+    def _addline(s, msg, color):
+        s.c.lock()
+        try:
+            s.logwindow.addstr(msg + "\n", color)
+            s.text.append((msg, color))
+            while len(s.text) > s.logheight:
+                s.text = s.text[1:]
+        finally:
+            s.c.unlock()
 
     def terminate(s, exitstatus = 0):
         s.c.stop()