Properly handling early exceptions from non-main threads is quite a pain. These three patches make it a bit more straightforward and, what's even better, working.
Vratislav Podzimek (3): Move HW errors processing to the code that runs in the main thread Add a method for waiting for error handling to finish Use threadMgr to wait for exception handling to finish
pyanaconda/exception.py | 149 +++++++++++++++++++++--------------------- pyanaconda/threads.py | 11 ++++ pyanaconda/ui/gui/__init__.py | 6 +- 3 files changed, 89 insertions(+), 77 deletions(-)
Also rename the run_handleException function to main_thread_handleException which better explains its purpose.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- pyanaconda/exception.py | 149 ++++++++++++++++++++++++------------------------ 1 file changed, 75 insertions(+), 74 deletions(-)
diff --git a/pyanaconda/exception.py b/pyanaconda/exception.py index b8b8f4f..9a4e676 100644 --- a/pyanaconda/exception.py +++ b/pyanaconda/exception.py @@ -60,7 +60,7 @@ class AnacondaExceptionHandler(ExceptionHandler): self._gui_lock = gui_lock self._intf_tty_num = tty_num
- def run_handleException(self, dump_info): + def _main_loop_handleException(self, dump_info): """ 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. @@ -69,8 +69,21 @@ class AnacondaExceptionHandler(ExceptionHandler):
"""
- super(AnacondaExceptionHandler, self).handleException(dump_info) - return False + ty = dump_info.exc_info.type + value = dump_info.exc_info.value + + if (issubclass(ty, blivet.errors.StorageError) and value.hardware_fault) \ + or (issubclass(ty, OSError) and value.errno == errno.EIO): + # hardware fault or '[Errno 5] Input/Output error' + hw_error_msg = _("The installation was stopped due to what " + "seems to be a problem with your hardware. " + "The exact error message is:\n\n%s.\n\n " + "The installer will now terminate.") % str(value) + self.intf.messageWindow(_("Hardware error occured"), hw_error_msg) + sys.exit(0) + else: + super(AnacondaExceptionHandler, self).handleException(dump_info) + return False
def handleException(self, dump_info): """ @@ -89,78 +102,66 @@ class AnacondaExceptionHandler(ExceptionHandler): ty = dump_info.exc_info.type value = dump_info.exc_info.value
- if (issubclass(ty, blivet.errors.StorageError) and value.hardware_fault) \ - or (issubclass(ty, OSError) and value.errno == errno.EIO): - # hardware fault or '[Errno 5] Input/Output error' - hw_error_msg = _("The installation was stopped due to what " - "seems to be a problem with your hardware. " - "The exact error message is:\n\n%s.\n\n " - "The installer will now terminate.") % str(value) - self.intf.messageWindow(_("Hardware error occured"), hw_error_msg) - sys.exit(0) - else: - try: - from gi.repository import Gtk - - # XXX: Gtk stopped raising RuntimeError if it fails to - # initialize. Horay! But will it stay like this? Let's be - # cautious and raise the exception on our own to work in both - # cases - initialized = Gtk.init_check(None)[0] - if not initialized: - raise RuntimeError() - - # Attempt to grab the GUI initializing lock, do not block - if not self._gui_lock.acquire(False): - # the graphical interface is running, don't crash it by - # running another one potentially from a different thread - log.debug("Gtk running, queuing exception handler to the " - "main loop") - GLib.idle_add(self.run_handleException, dump_info) - else: - log.debug("Gtk not running, starting Gtk and running " - "exception handler in it") - super(AnacondaExceptionHandler, self).handleException( - dump_info) - - except (RuntimeError, ImportError): - log.debug("Gtk cannot be initialized") - # X not running (Gtk cannot be initialized) - if threadMgr.in_main_thread(): - log.debug("In the main thread, running exception handler") - if (issubclass (ty, CmdlineError)): - - cmdline_error_msg = _("\nThe installation was stopped due to " - "incomplete spokes detected while running " - "in non-interactive cmdline mode. Since there " - "cannot be any questions in cmdline mode, " - "edit your kickstart file and retry " - "installation.\nThe exact error message is: " - "\n\n%s.\n\nThe installer will now terminate.") % str(value) - - # since there is no UI in cmdline mode and it is completely - # non-interactive, we can't show a message window asking the user - # to acknowledge the error; instead, print the error out and sleep - # for a few seconds before exiting the installer - print(cmdline_error_msg) - time.sleep(10) - sys.exit(1) - else: - print("\nAn unknown error has occured, look at the " - "/tmp/anaconda-tb* file(s) for more details") - # in the main thread, run exception handler - super(AnacondaExceptionHandler, self).handleException( - dump_info) + try: + from gi.repository import Gtk + + # XXX: Gtk stopped raising RuntimeError if it fails to + # initialize. Horay! But will it stay like this? Let's be + # cautious and raise the exception on our own to work in both + # cases + initialized = Gtk.init_check(None)[0] + if not initialized: + raise RuntimeError() + + # Attempt to grab the GUI initializing lock, do not block + if not self._gui_lock.acquire(False): + # the graphical interface is running, don't crash it by + # running another one potentially from a different thread + log.debug("Gtk running, queuing exception handler to the " + "main loop") + GLib.idle_add(self._main_loop_handleException, dump_info) + else: + log.debug("Gtk not running, starting Gtk and running " + "exception handler in it") + self._main_loop_handleException(dump_info) + + except (RuntimeError, ImportError): + log.debug("Gtk cannot be initialized") + # X not running (Gtk cannot be initialized) + if threadMgr.in_main_thread(): + log.debug("In the main thread, running exception handler") + if (issubclass (ty, CmdlineError)): + + cmdline_error_msg = _("\nThe installation was stopped due to " + "incomplete spokes detected while running " + "in non-interactive cmdline mode. Since there " + "cannot be any questions in cmdline mode, " + "edit your kickstart file and retry " + "installation.\nThe exact error message is: " + "\n\n%s.\n\nThe installer will now terminate.") % str(value) + + # since there is no UI in cmdline mode and it is completely + # non-interactive, we can't show a message window asking the user + # to acknowledge the error; instead, print the error out and sleep + # for a few seconds before exiting the installer + print(cmdline_error_msg) + time.sleep(10) + sys.exit(1) else: - log.debug("In a non-main thread, sending a message with " - "exception data") - # not in the main thread, just send message with exception - # data and let message handler run the exception handler in - # the main thread - exc_info = dump_info.exc_info - hubQ.send_exception((exc_info.type, - exc_info.value, - exc_info.stack)) + print("\nAn unknown error has occured, look at the " + "/tmp/anaconda-tb* file(s) for more details") + # in the main thread, run exception handler + self._main_loop_handleException(dump_info) + else: + log.debug("In a non-main thread, sending a message with " + "exception data") + # not in the main thread, just send message with exception + # data and let message handler run the exception handler in + # the main thread + exc_info = dump_info.exc_info + hubQ.send_exception((exc_info.type, + exc_info.value, + exc_info.stack))
def postWriteHook(self, dump_info): anaconda = dump_info.object
Also rename the run_handleException function to main_thread_handleException which better explains its purpose.
...
diff --git a/pyanaconda/exception.py b/pyanaconda/exception.py index b8b8f4f..9a4e676 100644 --- a/pyanaconda/exception.py +++ b/pyanaconda/exception.py @@ -60,7 +60,7 @@ class AnacondaExceptionHandler(ExceptionHandler): self._gui_lock = gui_lock self._intf_tty_num = tty_num
- def run_handleException(self, dump_info):
- def _main_loop_handleException(self, dump_info): """ 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.
- Chris
On Mon, 2015-01-12 at 10:00 -0500, Chris Lumens wrote:
Also rename the run_handleException function to main_thread_handleException which better explains its purpose.
...
diff --git a/pyanaconda/exception.py b/pyanaconda/exception.py index b8b8f4f..9a4e676 100644 --- a/pyanaconda/exception.py +++ b/pyanaconda/exception.py @@ -60,7 +60,7 @@ class AnacondaExceptionHandler(ExceptionHandler): self._gui_lock = gui_lock self._intf_tty_num = tty_num
- def run_handleException(self, dump_info):
- def _main_loop_handleException(self, dump_info): """ 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.
Good catch, thanks! Fixing locally.
On Mon, Jan 12, 2015 at 03:04:42PM +0100, Vratislav Podzimek wrote:
Also rename the run_handleException function to main_thread_handleException which better explains its purpose.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com
pyanaconda/exception.py | 149 ++++++++++++++++++++++++------------------------ 1 file changed, 75 insertions(+), 74 deletions(-)
Something I ran into on Friday that this doesn't address is error handling in cmdline mode. In commit 8c5d7c9f you stopped setting up the handler when we're in cmdline mode which fixed that specific bug, but introduced another problem with errors that happen in the tui input thread. If there is an incomplete spoke it raises an error, which passes it to the main loop where the system handles it by printing it. And back in the thread it loops and raises again, and again, etc. nothing ever exits so it's stuck.
I think we need to always have the handler installed, but need to handle problems like 1155979 by telling meh we're not interactive and it should just quit instead of trying to get user input.
except (RuntimeError, ImportError):log.debug("Gtk cannot be initialized")# X not running (Gtk cannot be initialized)if threadMgr.in_main_thread():log.debug("In the main thread, running exception handler")if (issubclass (ty, CmdlineError)):cmdline_error_msg = _("\nThe installation was stopped due to "
May as well remove that blank line while you're at it.
Sometimes we need to wait for error handling in problematic threads finish. The ThreadManager singleton has a good overview of threads and errors they caused or didn't cause so it can easily serve such request.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- pyanaconda/threads.py | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/pyanaconda/threads.py b/pyanaconda/threads.py index eea1da6..f116593 100644 --- a/pyanaconda/threads.py +++ b/pyanaconda/threads.py @@ -182,6 +182,17 @@ class ThreadManager(object): with self._objs_lock: return self._objs.keys()
+ def wait_for_error_threads(self): + """ + Waits for all threads that caused exceptions. In other words, waits for + exception handling (possibly interactive) to be finished. + + """ + + for thread_name in self._errors.keys(): + thread = self._objs[thread_name] + thread.join() + class AnacondaThread(threading.Thread): """A threading.Thread subclass that exists only for a couple purposes:
If an exception appears in a non-main thread prior to GUI being started, python-meh or our own error handling code start a Gtk main loop in such a thread. We need to wait for such main loop to finish instead of running another one in the main thread. But even if python-meh or our error handling code call sys.exit(), the process doesn't exit because the main thread is still running and it is not a daemon thread. Thus we need to wait for the exception handling code to finish and then exit the process from the main thread.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- pyanaconda/ui/gui/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/pyanaconda/ui/gui/__init__.py b/pyanaconda/ui/gui/__init__.py index bd6941d..b189350 100644 --- a/pyanaconda/ui/gui/__init__.py +++ b/pyanaconda/ui/gui/__init__.py @@ -28,6 +28,7 @@ from gi.repository import Gdk, Gtk, AnacondaWidgets, Keybinder, GdkPixbuf, GLib, from pyanaconda.i18n import _ from pyanaconda.constants import IPMI_ABORTED from pyanaconda import product, iutil +from pyanaconda import threads
from pyanaconda.ui import UserInterface, common from pyanaconda.ui.gui.utils import gtk_action_wait, busyCursor, unbusyCursor @@ -648,9 +649,8 @@ class GraphicalUserInterface(UserInterface): log.error("Unhandled exception caught, waiting for python-meh to "\ "exit")
- # Loop forever, meh will call sys.exit() when it's done - while True: - time.sleep(10000) + threads.threadMgr.wait_for_error_threads() + sys.exit(1)
# Apply a widget-scale to hidpi monitors self._widgetScale()
On Mon, Jan 12, 2015 at 03:04:41PM +0100, Vratislav Podzimek wrote:
Properly handling early exceptions from non-main threads is quite a pain. These three patches make it a bit more straightforward and, what's even better, working.
Ack. I fixed the problem with having an interactive error handler in cmdline so this should all be good.
anaconda-patches@lists.fedorahosted.org