[master] Fix issues with early exceptions in threads

[Vratislav Podzimek]

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(-)

[PATCH 1/3] Move HW errors processing to the code that runs in the main thread

[Vratislav Podzimek]

Also rename the run_handleException function to main_thread_handleException
which better explains its purpose.

Signed-off-by: Vratislav Podzimek 
---
 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

[PATCH 1/3] Move HW errors processing to the code that runs in the main thread

[Chris Lumens]

> 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

[PATCH 1/3] Move HW errors processing to the code that runs in the main thread

[Vratislav Podzimek]

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.

[PATCH 1/3] Move HW errors processing to the code that runs in the main thread

[Brian C. Lane]

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
>
> 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.

[PATCH 2/3] Add a method for waiting for error handling to finish

[Vratislav Podzimek]

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 
---
 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:

[PATCH 3/3] Use threadMgr to wait for exception handling to finish

[Vratislav Podzimek]

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 
---
 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()

[Brian C. Lane]

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.