[PATCH 2/2] Make our gtk_* decorators safer and more intelligent

Vratislav Podzimek vpodzime at redhat.com
Mon Mar 18 11:16:33 UTC 2013


The gtk_thread_wait and gtk_thread_nowait decorators cannot be used from the
main thread as it would cause a deadlock by queueing actions to the Gtk main
loop and then blocking the loop by waiting for result to appear in the
temporary queue.

However with Thread Manager's ability to tell if we are in the main thread or
not we can make these decorators more universal so that the decorated functions
can be called from anywhere. Since they are now not only for threads, I've
renamed them to gtk_action_wait and gtk_action_nowait.

The question is if we can start decorating all methods and functions somehow
manipulating with Gtk or just methods and functions that are expected to be
called also from threads. That depends on the overhead these decorators bring.

Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
---
 pyanaconda/ui/gui/__init__.py              |  8 +++---
 pyanaconda/ui/gui/hubs/progress.py         |  4 +--
 pyanaconda/ui/gui/spokes/datetime_spoke.py |  6 ++--
 pyanaconda/ui/gui/spokes/software.py       |  4 +--
 pyanaconda/ui/gui/spokes/source.py         |  4 +--
 pyanaconda/ui/gui/spokes/storage.py        |  4 +--
 pyanaconda/ui/gui/utils.py                 | 46 +++++++++++++++++++-----------
 7 files changed, 44 insertions(+), 32 deletions(-)

diff --git a/pyanaconda/ui/gui/__init__.py b/pyanaconda/ui/gui/__init__.py
index 7ace103..7abed97 100644
--- a/pyanaconda/ui/gui/__init__.py
+++ b/pyanaconda/ui/gui/__init__.py
@@ -26,7 +26,7 @@ from gi.repository import Gdk
 from pyanaconda.product import distributionText, isFinal
 
 from pyanaconda.ui import UserInterface, common
-from pyanaconda.ui.gui.utils import enlightbox, gtk_thread_wait
+from pyanaconda.ui.gui.utils import enlightbox, gtk_action_wait
 from pyanaconda.product import isFinal, productName, productVersion
 import os.path
 
@@ -381,7 +381,7 @@ class GraphicalUserInterface(UserInterface):
     ###
     ### MESSAGE HANDLING METHODS
     ###
-    @gtk_thread_wait
+    @gtk_action_wait
     def showError(self, message):
         from gi.repository import AnacondaWidgets, Gtk
         dlg = Gtk.MessageDialog(flags=Gtk.DialogFlags.MODAL,
@@ -395,7 +395,7 @@ class GraphicalUserInterface(UserInterface):
             dlg.run()
             dlg.destroy()
 
-    @gtk_thread_wait
+    @gtk_action_wait
     def showDetailedError(self, message, details):
         from pyanaconda.ui.gui.spokes.lib.detailederror import DetailedErrorDialog
         dlg = DetailedErrorDialog(None, buttons=[_("_Quit")],
@@ -406,7 +406,7 @@ class GraphicalUserInterface(UserInterface):
             rc = dlg.run()
             dlg.window.destroy()
 
-    @gtk_thread_wait
+    @gtk_action_wait
     def showYesNoQuestion(self, message):
         from gi.repository import AnacondaWidgets, Gtk
         dlg = Gtk.MessageDialog(flags=Gtk.DialogFlags.MODAL,
diff --git a/pyanaconda/ui/gui/hubs/progress.py b/pyanaconda/ui/gui/hubs/progress.py
index 65cd9e4..5fa5079 100644
--- a/pyanaconda/ui/gui/hubs/progress.py
+++ b/pyanaconda/ui/gui/hubs/progress.py
@@ -36,7 +36,7 @@ from pyanaconda.flags import flags
 from pykickstart.constants import KS_WAIT, KS_SHUTDOWN, KS_REBOOT
 
 from pyanaconda.ui.gui.hubs import Hub
-from pyanaconda.ui.gui.utils import gtk_thread_nowait, gtk_call_once
+from pyanaconda.ui.gui.utils import gtk_action_nowait, gtk_call_once
 
 __all__ = ["ProgressHub"]
 
@@ -249,7 +249,7 @@ class ProgressHub(Hub):
 
         gtk_call_once(self._progressLabel.set_text, message)
 
-    @gtk_thread_nowait
+    @gtk_action_nowait
     def _progress_bar_complete(self):
         self._progressBar.set_fraction(1.0)
         self._progressLabel.set_text(_("Complete!"))
diff --git a/pyanaconda/ui/gui/spokes/datetime_spoke.py b/pyanaconda/ui/gui/spokes/datetime_spoke.py
index 1800f02..809c7d1 100644
--- a/pyanaconda/ui/gui/spokes/datetime_spoke.py
+++ b/pyanaconda/ui/gui/spokes/datetime_spoke.py
@@ -33,7 +33,7 @@ from pyanaconda.ui.common import FirstbootSpokeMixIn
 from pyanaconda.ui.gui import GUIObject
 from pyanaconda.ui.gui.spokes import NormalSpoke
 from pyanaconda.ui.gui.categories.localization import LocalizationCategory
-from pyanaconda.ui.gui.utils import enlightbox, gtk_thread_nowait, gtk_call_once
+from pyanaconda.ui.gui.utils import enlightbox, gtk_action_nowait, gtk_call_once
 
 from pyanaconda import timezone
 from pyanaconda.timezone import NTP_SERVICE
@@ -177,7 +177,7 @@ class NTPconfigDialog(GUIObject):
 
         """
 
-        @gtk_thread_nowait
+        @gtk_action_nowait
         def set_store_value(arg_tuple):
             """
             We need a function for this, because this way it can be added to
@@ -211,7 +211,7 @@ class NTPconfigDialog(GUIObject):
                                     itr, 1, SERVER_NOK))
         self._epoch_lock.release()
 
-    @gtk_thread_nowait
+    @gtk_action_nowait
     def _refresh_server_working(self, itr):
         """ Runs a new thread with _set_server_ok_nok(itr) as a taget. """
 
diff --git a/pyanaconda/ui/gui/spokes/software.py b/pyanaconda/ui/gui/spokes/software.py
index 14b15c6..944496c 100644
--- a/pyanaconda/ui/gui/spokes/software.py
+++ b/pyanaconda/ui/gui/spokes/software.py
@@ -30,7 +30,7 @@ from pyanaconda.threads import threadMgr, AnacondaThread
 from pyanaconda.ui import communication
 from pyanaconda.ui.gui.spokes import NormalSpoke
 from pyanaconda.ui.gui.spokes.lib.detailederror import DetailedErrorDialog
-from pyanaconda.ui.gui.utils import enlightbox, gtk_thread_wait
+from pyanaconda.ui.gui.utils import enlightbox, gtk_action_wait
 from pyanaconda.ui.gui.categories.software import SoftwareCategory
 from .source import AdditionalReposDialog
 
@@ -212,7 +212,7 @@ class SoftwareSelectionSpoke(NormalSpoke):
         # we should do dependency solving here.
         self._apply()
 
-    @gtk_thread_wait
+    @gtk_action_wait
     def _first_refresh(self):
         try:
             self.refresh()
diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py
index 5c0aaab..dda385f 100644
--- a/pyanaconda/ui/gui/spokes/source.py
+++ b/pyanaconda/ui/gui/spokes/source.py
@@ -38,7 +38,7 @@ from pyanaconda.ui import communication
 from pyanaconda.ui.gui import GUIObject
 from pyanaconda.ui.gui.spokes import NormalSpoke
 from pyanaconda.ui.gui.categories.software import SoftwareCategory
-from pyanaconda.ui.gui.utils import enlightbox, gtk_thread_wait
+from pyanaconda.ui.gui.utils import enlightbox, gtk_action_wait
 from pyanaconda.iutil import ProxyString, ProxyStringError
 from pyanaconda.ui.gui.utils import gtk_call_once
 from pyanaconda.threads import threadMgr, AnacondaThread
@@ -715,7 +715,7 @@ class SourceSpoke(NormalSpoke):
             cdrom = opticalInstallMedia(self.storage.devicetree)
 
         if cdrom:
-            @gtk_thread_wait
+            @gtk_action_wait
             def gtk_action_1():
                 selector = AnacondaWidgets.DiskOverview(cdrom.format.label or "", "drive-removable-media", "", cdrom.name)
                 selector.set_chosen(chosen)
diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py
index 1b474f0..bdd572b 100644
--- a/pyanaconda/ui/gui/spokes/storage.py
+++ b/pyanaconda/ui/gui/spokes/storage.py
@@ -48,7 +48,7 @@ from pyanaconda.ui.gui.spokes.lib.passphrase import PassphraseDialog
 from pyanaconda.ui.gui.spokes.lib.detailederror import DetailedErrorDialog
 from pyanaconda.ui.gui.spokes.lib.resize import ResizeDialog
 from pyanaconda.ui.gui.categories.storage import StorageCategory
-from pyanaconda.ui.gui.utils import enlightbox, gtk_call_once, gtk_thread_wait
+from pyanaconda.ui.gui.utils import enlightbox, gtk_call_once, gtk_action_wait
 
 from pyanaconda.kickstart import doKickstartStorage
 from blivet import empty_device
@@ -578,7 +578,7 @@ class StorageSpoke(NormalSpoke, StorageChecker):
             size = size_str(disk.size)
             popup_info = "%s" % disk.serial
 
-            @gtk_thread_wait
+            @gtk_action_wait
             def gtk_action():
                 overview = AnacondaWidgets.DiskOverview(disk.description,
                                                         kind,
diff --git a/pyanaconda/ui/gui/utils.py b/pyanaconda/ui/gui/utils.py
index 10ad1df..1baefea 100644
--- a/pyanaconda/ui/gui/utils.py
+++ b/pyanaconda/ui/gui/utils.py
@@ -1,6 +1,6 @@
 # Miscellaneous UI functions
 #
-# Copyright (C) 2012  Red Hat, Inc.
+# Copyright (C) 2012, 2013 Red Hat, Inc.
 #
 # This copyrighted material is made available to anyone wishing to use,
 # modify, copy, or redistribute it subject to the terms and conditions of
@@ -17,7 +17,12 @@
 # Red Hat, Inc.
 #
 # Red Hat Author(s): Chris Lumens <clumens at redhat.com>
+#                    Martin Sivak <msivak at redhat.com>
+#                    Vratislav Podzimek <vpodzime at redhat.com>
 #
+
+from pyanaconda.threads import threadMgr
+
 from contextlib import contextmanager
 from gi.repository import Gdk, Gtk, GLib
 import Queue
@@ -25,21 +30,19 @@ import Queue
 def gtk_call_once(func, *args):
     """Wrapper for GLib.idle_add call that ensures the func is called
        only once.
-    """ 
+    """
     def wrap(args):
         func(*args)
         return False
-    
-    GLib.idle_add(wrap, args)
 
-def gtk_thread_wait(func):
-    """Decorator method which causes every call of the decorated function
-       to be executed in the context of Gtk main loop and returns the ret
-       value after the decorated method finishes.
+    GLib.idle_add(wrap, args)
 
-       Method decorated by this decorator must not be called from inside
-       of the Gtk main loop. It will cause a hang.
+def gtk_action_wait(func):
+    """Decorator method which ensures every call of the decorated function to be
+       executed in the context of Gtk main loop even if called from a non-main
+       thread and returns the ret value after the decorated method finishes.
     """
+
     queue = Queue.Queue()
 
     def _idle_method(q_args):
@@ -51,19 +54,23 @@ def gtk_thread_wait(func):
         return False
 
     def _call_method(*args):
-        """The new body for the decorated method. It uses closure bound
-           queue variable which is valid until the reference to this method
-           is destroyed."""
+        """The new body for the decorated method. If needed, it uses closure
+           bound queue variable which is valid until the reference to this
+           method is destroyed."""
+        if threadMgr.in_main_thread():
+            # nothing special has to be done in the main thread
+            return func(*args)
+
         GLib.idle_add(_idle_method, (queue, args))
         return queue.get()
 
     return _call_method
 
 
-def gtk_thread_nowait(func):
-    """Decorator method which causes every call of the decorated function
-       to be executed in the context of Gtk main loop. The new method does
-       not wait for the callback to finish.
+def gtk_action_nowait(func):
+    """Decorator method which ensures every call of the decorated function to be
+       executed in the context of Gtk main loop even if called from a non-main
+       thread. The new method does not wait for the callback to finish.
     """
 
     def _idle_method(args):
@@ -75,6 +82,11 @@ def gtk_thread_nowait(func):
     def _call_method(*args):
         """The new body for the decorated method.
         """
+        if threadMgr.in_main_thread():
+            # nothing special has to be done in the main thread
+            func(*args)
+            return
+
         GLib.idle_add(_idle_method, args)
 
     return _call_method
-- 
1.7.11.7



More information about the anaconda-patches mailing list