This patch provides a fix for the case when there is an exception in a non-main thread in text mode. It is impossible to run the exception handler in the non-main thread for many reasons. It competes with the main thread for standard input, it cannot run libreport because libreport forks, it cannot quit the installer because the main thread would still be running and probably something else I didn't hit yet. This builds on Martin's patch for processing our message queue in text mode.
The only remaining problem is, that when the main thread runs raw_input another call of raw_input from the exception handler is blocked until the first one is finished. Thus one needs to hit ENTER before entering any input for the exception handler. This can be fixed by telling python-meh's exception handler that it should use our raw_input instead of the built-in one but I am leaving this for an additional patch as it needs some changes in python-meh (and thus new build).
Vratislav Podzimek (1): Run exception handling in the main thread also in TUI
anaconda | 4 ++- pyanaconda/exception.py | 74 ++++++++++++++++++++++++++++++++----------- pyanaconda/ui/tui/__init__.py | 7 ++-- 3 files changed, 64 insertions(+), 21 deletions(-)
We need to get exception handling to the main thread in both graphical and textual user interface. This patch adds the code necessary to get exception handling to the main thread in text mode by using the async handling based on the communication queue. We just need to distinguish between exception in the main thread or in the non-main thread.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- anaconda | 4 ++- pyanaconda/exception.py | 74 ++++++++++++++++++++++++++++++++----------- pyanaconda/ui/tui/__init__.py | 7 ++-- 3 files changed, 64 insertions(+), 21 deletions(-)
diff --git a/anaconda b/anaconda index 0e0fdba..0eb90c4 100755 --- a/anaconda +++ b/anaconda @@ -31,6 +31,7 @@ # This toplevel file is a little messy at the moment...
import atexit, sys, os, re, time, subprocess +import threading from tempfile import mkstemp # keep up with process ID of the window manager if we start it wm_pid = None @@ -963,9 +964,10 @@ if __name__ == "__main__": # sets yum's multilib_policy to "all" (as opposed to "best") ksdata.packages.multiLib = opts.multiLib
+ cur_thread_id = threading.current_thread().ident from pyanaconda import exception # comment out the next line to make exceptions non-fatal - anaconda.mehConfig = exception.initExceptionHandling(anaconda) + anaconda.mehConfig = exception.initExceptionHandling(anaconda, cur_thread_id)
# add our own additional signal handlers signal.signal(signal.SIGUSR1, lambda signum, frame: diff --git a/pyanaconda/exception.py b/pyanaconda/exception.py index f053c0d..6f6d68d 100644 --- a/pyanaconda/exception.py +++ b/pyanaconda/exception.py @@ -30,10 +30,13 @@ import os import shutil import signal import time +import threading from flags import flags import kickstart import blivet.errors +from pyanaconda.ui.communication import HUB_CODE_EXCEPTION, hubQ from pyanaconda.constants import ROOT_PATH +# pylint: disable-msg=E0611 from gi.repository import GLib
import logging @@ -45,33 +48,42 @@ _ = lambda x: gettext.ldgettext("anaconda", x)
class AnacondaExceptionHandler(ExceptionHandler):
- def __init__(self, confObj, intfClass, exnClass, tty_num): + def __init__(self, confObj, intfClass, exnClass, tty_num, main_thread_id): """ @see: python-meh's ExceptionHandler @param tty_num: the number of tty the interface is running on + @param main_thread_id: id of the main thread
"""
ExceptionHandler.__init__(self, confObj, intfClass, exnClass) self._intf_tty_num = tty_num + self._main_thread_id = main_thread_id
- def handleException(self, (ty, value, tb), obj): + def run_handleException(self, args_tuple): + """ + Helper method with one argument only so that it can be registered + with GLib.idle_add() to run on idle or called from a handler.
- def run_handleException_on_idle(args_tuple): - """ - Helper function with one argument only so that it can be registered - with GLib.idle_add() to run on idle. + @param args_tuple: ((ty, value, tb), obj)
- @param args_tuple: ((ty, value, tb), obj) + """
- """ + trace, obj = args_tuple + ty, value, tb = trace
- trace, obj = args_tuple - ty, value, tb = trace + super(AnacondaExceptionHandler, self).handleException((ty, value, tb), + obj) + return False
- super(AnacondaExceptionHandler, self).handleException((ty, value, tb), - obj) - return False + def handleException(self, (ty, value, tb), obj): + """ + Our own handleException method doing some additional stuff before + calling the original python-meh's one. + + @see: python-meh's ExceptionHandler.handleException + + """
if issubclass(ty, blivet.errors.StorageError) and value.hardware_fault: hw_error_msg = _("The installation was stopped due to what " @@ -82,13 +94,14 @@ class AnacondaExceptionHandler(ExceptionHandler): sys.exit(0) else: try: + # pylint: disable-msg=E0611 from gi.repository import Gtk
if Gtk.main_level() > 0: # main loop is running, don't crash it by running another one # potentially from a different thread - GLib.idle_add(run_handleException_on_idle, - ((ty, value, tb), obj)) + GLib.idle_add(self.run_handleException, + ((ty, value, tb), obj)) else: super(AnacondaExceptionHandler, self).handleException( (ty, value, tb), obj) @@ -97,8 +110,15 @@ class AnacondaExceptionHandler(ExceptionHandler): # X not running (Gtk cannot be initialized) print "An unknown error has occured, look at the "\ "/tmp/anaconda-tb* file(s) for more details" - super(AnacondaExceptionHandler, self).handleException( + if threading.current_thread().ident == self._main_thread_id: + # in the main thread, run exception handler + super(AnacondaExceptionHandler, self).handleException( (ty, value, tb), obj) + else: + # not in the main thread, just send message with exception + # data and let message handler run the exception handler + hubQ.put((HUB_CODE_EXCEPTION, + [self, ((ty, value, tb), obj)]))
def postWriteHook(self, (ty, value, tb), anaconda): # See if /mnt/sysimage is present and put exception there as well @@ -151,7 +171,24 @@ class AnacondaExceptionHandler(ExceptionHandler): if self._intf_tty_num != 1: iutil.vtActivate(self._intf_tty_num)
-def initExceptionHandling(anaconda): +def exception_msg_handler(event, data): + """ + Handler for the HUB_CODE_EXCEPTION message in the hubQ. + + @param event: event data + @type event: (event_type, message_data) + @param data: additional data + @type data: any + + """ + + # get data from the event data structure + (event_type, msg_data) = event + (exc_handler, exc_data) = msg_data + + exc_handler.run_handleException(exc_data) + +def initExceptionHandling(anaconda, main_thread_id): fileList = [ "/tmp/anaconda.log", "/tmp/packaging.log", "/tmp/program.log", "/tmp/storage.log", "/tmp/ifcfg.log", "/tmp/yum.log", ROOT_PATH + "/root/install.log", @@ -182,7 +219,8 @@ def initExceptionHandling(anaconda): attchmnt_only=True)
handler = AnacondaExceptionHandler(conf, anaconda.intf.meh_interface, - ReverseExceptionDump, anaconda.intf.tty_num) + ReverseExceptionDump, anaconda.intf.tty_num, + main_thread_id) handler.install(anaconda)
return conf diff --git a/pyanaconda/ui/tui/__init__.py b/pyanaconda/ui/tui/__init__.py index 8da9846..e09b8ef 100644 --- a/pyanaconda/ui/tui/__init__.py +++ b/pyanaconda/ui/tui/__init__.py @@ -21,7 +21,8 @@
from pyanaconda import ui from pyanaconda.ui import common -from pyanaconda.ui import communication +from pyanaconda.exception import exception_msg_handler +from pyanaconda.ui.communication import hubQ, HUB_CODE_EXCEPTION from pyanaconda.flags import flags import simpleline as tui from hubs.summary import SummaryHub @@ -171,7 +172,9 @@ class TextUserInterface(ui.UserInterface): """Construct all the objects required to implement this interface. This method must be provided by all subclasses. """ - self._app = tui.App(u"Anaconda", yes_or_no_question=YesNoDialog, queue=communication.hubQ) + self._app = tui.App(u"Anaconda", yes_or_no_question=YesNoDialog, queue=hubQ) + self._app.register_event_handler(HUB_CODE_EXCEPTION, exception_msg_handler) + _hubs = self._list_hubs()
# First, grab a list of all the standalone spokes.
diff --git a/anaconda b/anaconda index 0e0fdba..0eb90c4 100755 --- a/anaconda +++ b/anaconda @@ -31,6 +31,7 @@ # This toplevel file is a little messy at the moment...
import atexit, sys, os, re, time, subprocess +import threading from tempfile import mkstemp # keep up with process ID of the window manager if we start it wm_pid = None @@ -963,9 +964,10 @@ if __name__ == "__main__": # sets yum's multilib_policy to "all" (as opposed to "best") ksdata.packages.multiLib = opts.multiLib
- cur_thread_id = threading.current_thread().ident from pyanaconda import exception # comment out the next line to make exceptions non-fatal
- anaconda.mehConfig = exception.initExceptionHandling(anaconda)
anaconda.mehConfig = exception.initExceptionHandling(anaconda, cur_thread_id)
# add our own additional signal handlers signal.signal(signal.SIGUSR1, lambda signum, frame:
Relying on thread ID kind of concerns me, given the following from the python docs:
Thread identifiers may be recycled when a thread exits and another thread is created. The identifier is available even after the thread has exited.
However in this one case (the main thread), I think we can get away with it. This was one reason why I added the ThreadManager object, to be able to reference via name instead of ID.
- def handleException(self, (ty, value, tb), obj):
- def run_handleException(self, args_tuple):
"""
Helper method with one argument only so that it can be registered
with GLib.idle_add() to run on idle or called from a handler.
def run_handleException_on_idle(args_tuple):
"""
Helper function with one argument only so that it can be registered
with GLib.idle_add() to run on idle.
@param args_tuple: ((ty, value, tb), obj)
@param args_tuple: ((ty, value, tb), obj)
"""
We've now got so much shipping around of (ty, val, tb) and then sometimes obj that I wonder if we should just go ahead and pack them up into something more handy. A NamedTuple (http://docs.python.org/2/library/collections.html#collections.namedtuple) might be just the thing to use.
- Chris
On Thu, 2013-02-21 at 08:01 -0500, Chris Lumens wrote:
diff --git a/anaconda b/anaconda index 0e0fdba..0eb90c4 100755 --- a/anaconda +++ b/anaconda @@ -31,6 +31,7 @@ # This toplevel file is a little messy at the moment...
import atexit, sys, os, re, time, subprocess +import threading from tempfile import mkstemp # keep up with process ID of the window manager if we start it wm_pid = None @@ -963,9 +964,10 @@ if __name__ == "__main__": # sets yum's multilib_policy to "all" (as opposed to "best") ksdata.packages.multiLib = opts.multiLib
- cur_thread_id = threading.current_thread().ident from pyanaconda import exception # comment out the next line to make exceptions non-fatal
- anaconda.mehConfig = exception.initExceptionHandling(anaconda)
anaconda.mehConfig = exception.initExceptionHandling(anaconda, cur_thread_id)
# add our own additional signal handlers signal.signal(signal.SIGUSR1, lambda signum, frame:
Relying on thread ID kind of concerns me, given the following from the python docs:
Thread identifiers may be recycled when a thread exits and another thread is created. The identifier is available even after the thread has exited.
However in this one case (the main thread), I think we can get away with it. This was one reason why I added the ThreadManager object, to be able to reference via name instead of ID.
Yeah I don't like it much too, but I guess for the main thread it should be okay.
- def handleException(self, (ty, value, tb), obj):
- def run_handleException(self, args_tuple):
"""
Helper method with one argument only so that it can be registered
with GLib.idle_add() to run on idle or called from a handler.
def run_handleException_on_idle(args_tuple):
"""
Helper function with one argument only so that it can be registered
with GLib.idle_add() to run on idle.
@param args_tuple: ((ty, value, tb), obj)
@param args_tuple: ((ty, value, tb), obj)
"""
We've now got so much shipping around of (ty, val, tb) and then sometimes obj that I wonder if we should just go ahead and pack them up into something more handy. A NamedTuple (http://docs.python.org/2/library/collections.html#collections.namedtuple) might be just the thing to use.
That's a neat idea! I'll change this also in python-meh and send patches for both. The anacoda part can be then squashed with this patch.
We've now got so much shipping around of (ty, val, tb) and then sometimes obj that I wonder if we should just go ahead and pack them up into something more handy. A NamedTuple (http://docs.python.org/2/library/collections.html#collections.namedtuple) might be just the thing to use.
That's a neat idea! I'll change this also in python-meh and send patches for both. The anacoda part can be then squashed with this patch.
Yeah, I've been trying to work it into new code where I can. It's just so much more readable than what I've been doing, even if it's not any shorter.
- Chris
anaconda-patches@lists.fedorahosted.org