By running exception handler every time an exception appears in a thread we block us from using our features provided by the wait method. Some exceptions may be non-critical and the waiting thread may recover from the erroneous state. However, we need to give it a chance to do so.
The wait_all method called before we start the actual installation should make sure there are no abandoned problematic threads that raised exception nobody cared about.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- pyanaconda/threads.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/pyanaconda/threads.py b/pyanaconda/threads.py index 29290df..0e81ded 100644 --- a/pyanaconda/threads.py +++ b/pyanaconda/threads.py @@ -121,6 +121,11 @@ class ThreadManager(object): log.debug("Waiting for thread %s to exit", name) self.wait(name)
+ if self.any_errors: + msg = "Unhandled errors from the following threads detected: %s" %\ + ", ".join(self._errors.iterkeys()) + raise RuntimeError(msg) + def set_error(self, name, *exc_info): """Set the error data for a thread
@@ -199,6 +204,11 @@ class AnacondaThread(threading.Thread): self._prefix_thread_counts[prefix] = thread_num kwargs["name"] = prefix + str(thread_num)
+ if "fatal" in kwargs: + self._fatal = kwargs.pop("fatal") + else: + self._fatal = True + threading.Thread.__init__(self, *args, **kwargs) self.daemon = True
@@ -212,7 +222,8 @@ class AnacondaThread(threading.Thread): # pylint: disable-msg=W0702 except: threadMgr.set_error(self.name, *sys.exc_info()) - sys.excepthook(*sys.exc_info()) + if self._fatal: + sys.excepthook(*sys.exc_info()) finally: threadMgr.remove(self.name) log.info("Thread Done: %s (%s)", self.name, self.ident)
On Mon, 2013-12-09 at 17:50 +0100, Vratislav Podzimek wrote:
By running exception handler every time an exception appears in a thread we block us from using our features provided by the wait method. Some exceptions may be non-critical and the waiting thread may recover from the erroneous state. However, we need to give it a chance to do so.
The wait_all method called before we start the actual installation should make sure there are no abandoned problematic threads that raised exception nobody cared about.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com
pyanaconda/threads.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/pyanaconda/threads.py b/pyanaconda/threads.py index 29290df..0e81ded 100644 --- a/pyanaconda/threads.py +++ b/pyanaconda/threads.py @@ -121,6 +121,11 @@ class ThreadManager(object): log.debug("Waiting for thread %s to exit", name) self.wait(name)
if self.any_errors:
msg = "Unhandled errors from the following threads detected: %s" %\
", ".join(self._errors.iterkeys())
raise RuntimeError(msg)
- def set_error(self, name, *exc_info): """Set the error data for a thread
@@ -199,6 +204,11 @@ class AnacondaThread(threading.Thread): self._prefix_thread_counts[prefix] = thread_num kwargs["name"] = prefix + str(thread_num)
if "fatal" in kwargs:
self._fatal = kwargs.pop("fatal")
else:
self._fatal = True
threading.Thread.__init__(self, *args, **kwargs) self.daemon = True
@@ -212,7 +222,8 @@ class AnacondaThread(threading.Thread): # pylint: disable-msg=W0702 except: threadMgr.set_error(self.name, *sys.exc_info())
sys.excepthook(*sys.exc_info())
if self._fatal:
sys.excepthook(*sys.exc_info()) finally: threadMgr.remove(self.name) log.info("Thread Done: %s (%s)", self.name, self.ident)
Maybe clear the internal error variable before raising the exception when wait is called on a non-fatal thread ? Otherwise the exception will be raised again on every wait.
Other than that looks fine! :)
On Tue, 2013-12-10 at 16:56 +0100, Martin Kolman wrote:
On Mon, 2013-12-09 at 17:50 +0100, Vratislav Podzimek wrote:
By running exception handler every time an exception appears in a thread we block us from using our features provided by the wait method. Some exceptions may be non-critical and the waiting thread may recover from the erroneous state. However, we need to give it a chance to do so.
The wait_all method called before we start the actual installation should make sure there are no abandoned problematic threads that raised exception nobody cared about.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com
pyanaconda/threads.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/pyanaconda/threads.py b/pyanaconda/threads.py index 29290df..0e81ded 100644 --- a/pyanaconda/threads.py +++ b/pyanaconda/threads.py @@ -121,6 +121,11 @@ class ThreadManager(object): log.debug("Waiting for thread %s to exit", name) self.wait(name)
if self.any_errors:
msg = "Unhandled errors from the following threads detected: %s" %\
", ".join(self._errors.iterkeys())
raise RuntimeError(msg)
- def set_error(self, name, *exc_info): """Set the error data for a thread
@@ -199,6 +204,11 @@ class AnacondaThread(threading.Thread): self._prefix_thread_counts[prefix] = thread_num kwargs["name"] = prefix + str(thread_num)
if "fatal" in kwargs:
self._fatal = kwargs.pop("fatal")
else:
self._fatal = True
threading.Thread.__init__(self, *args, **kwargs) self.daemon = True
@@ -212,7 +222,8 @@ class AnacondaThread(threading.Thread): # pylint: disable-msg=W0702 except: threadMgr.set_error(self.name, *sys.exc_info())
sys.excepthook(*sys.exc_info())
if self._fatal:
sys.excepthook(*sys.exc_info()) finally: threadMgr.remove(self.name) log.info("Thread Done: %s (%s)", self.name, self.ident)
Maybe clear the internal error variable before raising the exception when wait is called on a non-fatal thread ? Otherwise the exception will be raised again on every wait.
Yeah, that needs to be done. Fixing locally and waiting for one more ACK on this potentially problematic change.
On Tue, Dec 10, 2013 at 05:26:41PM +0100, Vratislav Podzimek wrote:
On Tue, 2013-12-10 at 16:56 +0100, Martin Kolman wrote:
Maybe clear the internal error variable before raising the exception when wait is called on a non-fatal thread ? Otherwise the exception will be raised again on every wait.
Yeah, that needs to be done. Fixing locally and waiting for one more ACK on this potentially problematic change.
Looks good to me.
anaconda-patches@lists.fedorahosted.org