[PATCH] Make thread manager operations atomic (#1029898)

Martin Kolman mkolman at redhat.com
Thu Nov 14 13:42:09 UTC 2013


The thread manipulation in thread manager needs to
be atomic to get rid of nasty race conditions.
For example, without the lock, it was possible
to get a thread object that was not yet started
(as threads are first added to the tracking dict
and then started) and try to join it,
resulting in an exception.

Signed-off-by: Martin Kolman <mkolman at redhat.com>
---
 pyanaconda/threads.py | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/pyanaconda/threads.py b/pyanaconda/threads.py
index 77cc306..1acde6a 100644
--- a/pyanaconda/threads.py
+++ b/pyanaconda/threads.py
@@ -40,6 +40,7 @@ class ThreadManager(object):
     """
     def __init__(self):
         self._objs = {}
+        self._objs_lock = threading.RLock()
         self._errors = {}
         self._main_thread = threading.current_thread()
 
@@ -50,12 +51,16 @@ class ThreadManager(object):
         """Given a Thread or Process object, add it to the list of known objects
            and start it.  It is assumed that obj.name is unique and descriptive.
         """
-        if obj.name in self._objs:
-            raise KeyError("Cannot add thread '%s', a thread with the same name already running" % obj.name)
 
-        self._objs[obj.name] = obj
-        self._errors[obj.name] = None
-        obj.start()
+        # we need to lock the thread dictionary when adding a new thread,
+        # so that callers can't get & join threads that are not yet started
+        with self._objs_lock:
+            if obj.name in self._objs:
+                raise KeyError("Cannot add thread '%s', a thread with the same name already running" % obj.name)
+
+            self._objs[obj.name] = obj
+            self._errors[obj.name] = None
+            obj.start()
 
         return obj.name
 
@@ -64,27 +69,37 @@ class ThreadManager(object):
            be called when a thread exits, or there will be no way to get a
            handle on it.
         """
-        self._objs.pop(name)
+        with self._objs_lock:
+            self._objs.pop(name)
 
     def exists(self, name):
         """Determine if a thread or process exists with the given name."""
-        return name in self._objs
+
+        # thread in the ThreadManager only oficially exists once started
+        with self._objs_lock:
+            return name in self._objs
 
     def get(self, name):
         """Given an object name, see if it exists and return the object.
            Return None if no such object exists.  Additionally, this method
            will re-raise any uncaught exception in the thread.
         """
-        obj = self._objs.get(name)
-        if obj:
-            self.raise_error(name)
 
-        return obj
+        # without the lockm it would be possible to get & join
+        # a thread that was not yet started
+        with self._objs_lock:
+            obj = self._objs.get(name)
+            if obj:
+                self.raise_error(name)
+
+            return obj
 
     def wait(self, name):
         """Wait for the thread to exit and if the thread exited with an error
            re-raise it here.
         """
+        # we don't need a lock here,
+        # because both exists() & get() are using it
         if self.exists(name):
             self.get(name).join()
 
@@ -136,7 +151,8 @@ class ThreadManager(object):
             :returns: number of running threads
             :rtype:   int
         """
-        return len(self._objs)
+        with self._objs_lock:
+            return len(self._objs)
 
     @property
     def names(self):
@@ -145,7 +161,8 @@ class ThreadManager(object):
             :returns: list of thread names
             :rtype:   list of strings
         """
-        return self._objs.keys()
+        with self._objs_lock:
+            return self._objs.keys()
 
 class AnacondaThread(threading.Thread):
     """A threading.Thread subclass that exists only for a couple purposes:
-- 
1.8.3.1



More information about the anaconda-patches mailing list