[PATCH 3/3] Make all Gtk calls from inside of it's main loop (and thread)

Martin Sivak msivak at redhat.com
Tue Oct 16 14:36:52 UTC 2012


- Marks all methods containing mostly gtk calls as gtk_thread_wait
  or gtk_thread_nowait
- Uses gtk_call_once instead of GLib.idle_add to make sure the
  method is called only once (returns False)
- Removes some code from the threading locks, because it wasn´t
  touching Gtk at all

This change was discussed in the mailinglist here:

https://www.redhat.com/archives/anaconda-devel-list/2012-October/msg00030.html

The main point was:

According to the Gtk team, the gdk_threads_enter/leave pair should
not be used at all (and they have apparently discouraged usage of
it since early releases of Gtk2). Moreover in the current Gdk docs
(http://developer.gnome.org/gdk3/stable/gdk3-Threads.html)
 those functions are now marked as deprecated.

The preferred way (and now the only way) is to use g_idle_add
(GLib.idle_add) with a callback method to schedule GUI changes.
The callback method will then get called by the Gtk main loop so
no locking is needed (and GLib.idle_add performs none). But that
is also the reason why everything Gtk related must be done from
the mainloop thread either directly or via idle_add.
---
 pyanaconda/ui/gui/__init__.py              | 47 ++++++++++++++-------------
 pyanaconda/ui/gui/hubs/progress.py         | 25 +++++++--------
 pyanaconda/ui/gui/spokes/datetime_spoke.py | 16 ++++++----
 pyanaconda/ui/gui/spokes/keyboard.py       | 15 +++------
 pyanaconda/ui/gui/spokes/software.py       | 34 ++++++++++++--------
 pyanaconda/ui/gui/spokes/source.py         | 51 ++++++++++++++++--------------
 pyanaconda/ui/gui/spokes/storage.py        | 27 +++++++++-------
 7 files changed, 109 insertions(+), 106 deletions(-)

diff --git a/pyanaconda/ui/gui/__init__.py b/pyanaconda/ui/gui/__init__.py
index 638772b..fa80e6d 100644
--- a/pyanaconda/ui/gui/__init__.py
+++ b/pyanaconda/ui/gui/__init__.py
@@ -24,7 +24,7 @@ import meh.ui.gui
 from gi.repository import Gdk
 
 from pyanaconda.ui import UserInterface, common
-from pyanaconda.ui.gui.utils import enlightbox, gdk_threaded
+from pyanaconda.ui.gui.utils import enlightbox, gtk_thread_wait
 
 import gettext
 _ = lambda x: gettext.ldgettext("anaconda", x)
@@ -117,37 +117,36 @@ class GraphicalUserInterface(UserInterface):
     ###
     ### MESSAGE HANDLING METHODS
     ###
+    @gtk_thread_wait
     def showError(self, message):
         from gi.repository import AnacondaWidgets, Gtk
-
-        with gdk_threaded():
-            dlg = Gtk.MessageDialog(flags=Gtk.DialogFlags.MODAL,
-                                    message_type=Gtk.MessageType.ERROR,
-                                    buttons=Gtk.ButtonsType.NONE,
-                                    message_format=message)
-            dlg.add_button(_("_Exit Installer"), 0)
-
-            with enlightbox(self._actions[0].window, dlg):
-                dlg.run()
-                dlg.destroy()
-
+        dlg = Gtk.MessageDialog(flags=Gtk.DialogFlags.MODAL,
+                                message_type=Gtk.MessageType.ERROR,
+                                buttons=Gtk.ButtonsType.NONE,
+                                message_format=message)
+        dlg.add_button(_("_Exit Installer"), 0)
+        
+        with enlightbox(self._actions[0].window, dlg):
+            dlg.run()
+            dlg.destroy()
+
+    @gtk_thread_wait
     def showYesNoQuestion(self, message):
         from gi.repository import AnacondaWidgets, Gtk
+        dlg = Gtk.MessageDialog(flags=Gtk.DialogFlags.MODAL,
+                                message_type=Gtk.MessageType.QUESTION,
+                                buttons=Gtk.ButtonsType.NONE,
+                                message_format=message)
+        dlg.add_buttons(_("_No"), 0, _("_Yes"), 1)
+        dlg.set_default_response(1)
 
-        with gdk_threaded():
-            dlg = Gtk.MessageDialog(flags=Gtk.DialogFlags.MODAL,
-                                    message_type=Gtk.MessageType.QUESTION,
-                                    buttons=Gtk.ButtonsType.NONE,
-                                    message_format=message)
-            dlg.add_buttons(_("_No"), 0, _("_Yes"), 1)
-            dlg.set_default_response(1)
-
-            with enlightbox(self._actions[0].window, dlg):
-                rc = dlg.run()
-                dlg.destroy()
+        with enlightbox(self._actions[0].window, dlg):
+            rc = dlg.run()
+            dlg.destroy()
 
         return bool(rc)
 
+
     def mainExceptionWindow(self, text, exn_file, *args, **kwargs):
         from gi.repository import Gtk, AnacondaWidgets
 
diff --git a/pyanaconda/ui/gui/hubs/progress.py b/pyanaconda/ui/gui/hubs/progress.py
index 9cb95eb..d3b6b41 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 gdk_threaded
+from pyanaconda.ui.gui.utils import gtk_thread_nowait, gtk_call_once
 
 __all__ = ["ProgressHub"]
 
@@ -227,29 +227,26 @@ class ProgressHub(Hub):
         self._totalSteps = steps
         self._currentStep = 0
 
-        with gdk_threaded():
-            self._progressBar.set_fraction(0.0)
+        gtk_call_once(self._progressBar.set_fraction, 0.0)
 
     def _step_progress_bar(self):
         if not self._totalSteps:
             return
 
-        with gdk_threaded():
-            self._currentStep += 1
-            self._progressBar.set_fraction(self._currentStep/self._totalSteps)
+        self._currentStep += 1
+        gtk_call_once(self._progressBar.set_fraction, self._currentStep/self._totalSteps)
 
     def _update_progress_message(self, message):
         if not self._totalSteps:
             return
 
-        with gdk_threaded():
-            self._progressLabel.set_text(message)
+        gtk_call_once(self._progressLabel.set_text, message)
 
+    @gtk_thread_nowait
     def _progress_bar_complete(self):
-        with gdk_threaded():
-            self._progressBar.set_fraction(1.0)
-            self._progressLabel.set_text(_("Complete!"))
+        self._progressBar.set_fraction(1.0)
+        self._progressLabel.set_text(_("Complete!"))
 
-            spinner = self.builder.get_object("progressSpinner")
-            spinner.stop()
-            spinner.hide()
+        spinner = self.builder.get_object("progressSpinner")
+        spinner.stop()
+        spinner.hide()
diff --git a/pyanaconda/ui/gui/spokes/datetime_spoke.py b/pyanaconda/ui/gui/spokes/datetime_spoke.py
index feabeec..f3249bb 100644
--- a/pyanaconda/ui/gui/spokes/datetime_spoke.py
+++ b/pyanaconda/ui/gui/spokes/datetime_spoke.py
@@ -32,7 +32,7 @@ from gi.repository import AnacondaWidgets, GLib, Gtk
 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
+from pyanaconda.ui.gui.utils import enlightbox, gtk_thread_nowait, gtk_call_once
 
 from pyanaconda import timezone
 from pyanaconda import iutil
@@ -174,6 +174,7 @@ class NTPconfigDialog(GUIObject):
 
         """
 
+        @gtk_thread_nowait
         def set_store_value(arg_tuple):
             """
             We need a function for this, because this way it can be added to
@@ -200,13 +201,14 @@ class NTPconfigDialog(GUIObject):
 
             if orig_hostname == actual_hostname:
                 if server_working:
-                    GLib.idle_add(set_store_value, (self._serversStore,
-                                                    itr, 1, SERVER_OK))
+                    set_store_value((self._serversStore,
+                                    itr, 1, SERVER_OK))
                 else:
-                    GLib.idle_add(set_store_value, (self._serversStore,
-                                                    itr, 1, SERVER_NOK))
+                    set_store_value((self._serversStore,
+                                    itr, 1, SERVER_NOK))
         self._epoch_lock.release()
 
+    @gtk_thread_nowait
     def _refresh_server_working(self, itr):
         """ Runs a new thread with _set_server_ok_nok(itr) as a taget. """
 
@@ -229,7 +231,7 @@ class NTPconfigDialog(GUIObject):
             self._poolsNote.set_text(POOL_SERVERS_NOTE)
 
         #do not block UI while starting thread (may take some time)
-        GLib.idle_add(self._refresh_server_working, itr)
+        self._refresh_server_working(itr)
 
     def on_entry_activated(self, entry, *args):
         self._add_server(entry.get_text())
@@ -410,7 +412,7 @@ class DatetimeSpoke(NormalSpoke):
             self._show_no_network_warning()
         else:
             self.window.clear_info()
-            GLib.idle_add(self._config_dialog.refresh_servers_state)
+            gtk_call_once(self._config_dialog.refresh_servers_state)
 
         if flags.can_touch_runtime_system("get NTP service state"):
             ntp_working = has_active_network and iutil.service_running("chronyd")
diff --git a/pyanaconda/ui/gui/spokes/keyboard.py b/pyanaconda/ui/gui/spokes/keyboard.py
index fd9e6ab..1a4ca68 100644
--- a/pyanaconda/ui/gui/spokes/keyboard.py
+++ b/pyanaconda/ui/gui/spokes/keyboard.py
@@ -30,7 +30,7 @@ from gi.repository import GLib, Gkbd, Gtk, Gdk
 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, gdk_threaded
+from pyanaconda.ui.gui.utils import enlightbox, gtk_call_once
 from pyanaconda import keyboard
 from pyanaconda import flags
 
@@ -89,12 +89,12 @@ class AddLayoutDialog(GUIObject):
             return 1
 
     def refresh(self):
-        self._entry = self.builder.get_object("addLayoutEntry")
         self._entry.grab_focus()
 
     def initialize(self):
         # We want to store layouts' names but show layouts as
         # 'language (description)'.
+        self._entry = self.builder.get_object("addLayoutEntry")
         layoutColumn = self.builder.get_object("newLayoutColumn")
         layoutRenderer = self.builder.get_object("newLayoutRenderer")
         layoutColumn.set_cell_data_func(layoutRenderer, _show_layout,
@@ -261,8 +261,8 @@ class KeyboardSpoke(NormalSpoke):
     # Signal handlers.
     def on_add_clicked(self, button):
         dialog = AddLayoutDialog(self.data)
-        dialog.refresh()
         dialog.initialize()
+        dialog.refresh()
 
         with enlightbox(self.window, dialog.window):
             response = dialog.run()
@@ -284,13 +284,6 @@ class KeyboardSpoke(NormalSpoke):
                     self._removeLayout(self._store, itr)
                 self._remove_last_attempt = False
 
-    def _run_add_from_removed(self, button):
-        # Only call gdk_threaded when necessary (when we need to run the add
-        # dialog from the on_remove_clicked path) or we'll introduce a different
-        # deadlock.
-        with gdk_threaded():
-            self.on_add_clicked(button)
-
     def on_remove_clicked(self, button):
         selection = self.builder.get_object("layoutSelection")
         if not selection.count_selected_rows():
@@ -311,7 +304,7 @@ class KeyboardSpoke(NormalSpoke):
             #redrawn
             self._remove_last_attempt = True
             add_button = self.builder.get_object("addLayoutButton")
-            GLib.idle_add(self._run_add_from_removed, add_button)
+            gtk_call_once(self.on_add_clicked, add_button)
             return
 
         #the selected item is not the first, select the previous one
diff --git a/pyanaconda/ui/gui/spokes/software.py b/pyanaconda/ui/gui/spokes/software.py
index 2083b53..e99a1a2 100644
--- a/pyanaconda/ui/gui/spokes/software.py
+++ b/pyanaconda/ui/gui/spokes/software.py
@@ -28,9 +28,10 @@ from pyanaconda.flags import flags
 from pyanaconda.ui.gui 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, gdk_threaded
+from pyanaconda.ui.gui.utils import enlightbox, gtk_thread_wait
 from pyanaconda.ui.gui.categories.software import SoftwareCategory
 from .source import AdditionalReposDialog
+from gi.repository import GLib
 
 from pyanaconda.kickstart import packagesSeen
 from pykickstart.parser import Group
@@ -155,7 +156,6 @@ class SoftwareSelectionSpoke(NormalSpoke):
         threadMgr.add(AnacondaThread(name="AnaSoftwareWatcher", target=self._initialize))
 
     def _initialize(self):
-        from pyanaconda.packaging import MetadataError
         from pyanaconda.threads import threadMgr
 
         communication.send_message(self.__class__.__name__, _("Downloading package metadata..."))
@@ -175,18 +175,8 @@ class SoftwareSelectionSpoke(NormalSpoke):
             if mdGatherThread:
                 mdGatherThread.join()
         else:
-            with gdk_threaded():
-                # Grabbing the list of groups could potentially take a long time the
-                # first time (yum does a lot of magic property stuff, some of which
-                # involves side effects like network access) so go ahead and grab
-                # them once now.
-                try:
-                    self.refresh()
-                except MetadataError:
-                    communication.send_message(self.__class__.__name__,
-                                               _("No installation source available"))
-                    return
-
+            if not self._first_refresh():
+                return
         self.payload.release()
 
         communication.send_ready(self.__class__.__name__)
@@ -195,6 +185,22 @@ class SoftwareSelectionSpoke(NormalSpoke):
         # we should do dependency solving here.
         self.apply()
 
+    @gtk_thread_wait
+    def _first_refresh(self):
+        # Grabbing the list of groups could potentially take a long time the
+        # first time (yum does a lot of magic property stuff, some of which
+        # involves side effects like network access) so go ahead and grab
+        # them once now.
+        from pyanaconda.packaging import MetadataError
+
+        try:
+            self.refresh()
+            return True
+        except MetadataError:
+            communication.send_message(self.__class__.__name__,
+                                       _("No installation source available"))
+            return False
+        
     def refresh(self):
         from pyanaconda.threads import threadMgr
         NormalSpoke.refresh(self)
diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py
index 9d336cc..61ce3ae 100644
--- a/pyanaconda/ui/gui/spokes/source.py
+++ b/pyanaconda/ui/gui/spokes/source.py
@@ -36,7 +36,7 @@ from pyanaconda.image import opticalInstallMedia, potentialHdisoSources
 from pyanaconda.ui.gui import GUIObject, communication
 from pyanaconda.ui.gui.spokes import NormalSpoke
 from pyanaconda.ui.gui.categories.software import SoftwareCategory
-from pyanaconda.ui.gui.utils import enlightbox, gdk_threaded
+from pyanaconda.ui.gui.utils import enlightbox, gtk_thread_wait
 from pyanaconda.iutil import ProxyString, ProxyStringError
 
 __all__ = ["SourceSpoke"]
@@ -623,37 +623,40 @@ class SourceSpoke(NormalSpoke):
         cdrom = None
         chosen = False
 
-        with gdk_threaded():
-            # If we've previously set up to use a CD/DVD method, the media has
-            # already been mounted by payload.setup.  We can't try to mount it
-            # again.  So just use what we already know to create the selector.
-            # Otherwise, check to see if there's anything available.
-            if self.data.method.method == "cdrom":
-                cdrom = self.payload.install_device
-                chosen = True
-            else:
-                cdrom = opticalInstallMedia(self.storage.devicetree, mountpoint=MOUNTPOINT)
+        # If we've previously set up to use a CD/DVD method, the media has
+        # already been mounted by payload.setup.  We can't try to mount it
+        # again.  So just use what we already know to create the selector.
+        # Otherwise, check to see if there's anything available.
+        if self.data.method.method == "cdrom":
+            cdrom = self.payload.install_device
+            chosen = True
+        else:
+            cdrom = opticalInstallMedia(self.storage.devicetree, mountpoint=MOUNTPOINT)
 
-            if cdrom:
+        if cdrom:
+            @gtk_thread_wait
+            def gtk_action_1():
                 selector = AnacondaWidgets.DiskOverview(cdrom.format.label or "", "drive-removable-media", "")
                 selector.path = cdrom.path
                 selector.set_chosen(chosen)
                 self._autodetectMediaBox.pack_start(selector, False, False, 0)
-                added = True
+                
+            gtk_action_1()
+            added = True
 
-            if self.data.method.method == "harddrive":
-                self._currentIsoFile = self.payload.ISOImage
+        if self.data.method.method == "harddrive":
+            self._currentIsoFile = self.payload.ISOImage
 
-            # These UI elements default to not being showable.  If optical install
-            # media were found, mark them to be shown.
-            if added:
-                self._autodetectBox.set_no_show_all(False)
-                self._autodetectButton.set_no_show_all(False)
+        # These UI elements default to not being showable.  If optical install
+        # media were found, mark them to be shown.
+        if added:
+            gtk_call_once(self._autodetectBox.set_no_show_all, False)
+            gtk_call_once(self._autodetectButton.set_no_show_all, False)
 
-            # Add the mirror manager URL in as the default for HTTP and HTTPS.
-            # We'll override this later in the refresh() method, if they've already
-            # provided a URL.
-            # FIXME
+        # Add the mirror manager URL in as the default for HTTP and HTTPS.
+        # We'll override this later in the refresh() method, if they've already
+        # provided a URL.
+        # FIXME
 
         self._ready = True
         communication.send_ready(self.__class__.__name__)
diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py
index bffe1d0..611f4ab 100644
--- a/pyanaconda/ui/gui/spokes/storage.py
+++ b/pyanaconda/ui/gui/spokes/storage.py
@@ -40,7 +40,6 @@
 
 from gi.repository import Gdk, GLib, Gtk
 from gi.repository import AnacondaWidgets
-
 from pyanaconda.ui.gui import GUIObject, communication
 from pyanaconda.ui.gui.spokes import NormalSpoke
 from pyanaconda.ui.gui.spokes.lib.cart import SelectedDisksDialog
@@ -48,7 +47,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, gdk_threaded
+from pyanaconda.ui.gui.utils import enlightbox, gtk_thread_wait
 
 from pyanaconda.kickstart import doKickstartStorage
 from pyanaconda.storage.size import Size
@@ -516,16 +515,18 @@ class StorageSpoke(NormalSpoke, StorageChecker):
         if len(self.disks) == 1 and not self.selected_disks:
             self.data.ignoredisk.onlyuse = [self.disks[0].name]
 
-        with gdk_threaded():
-            # properties: kind, description, capacity, os, popup-info
-            for disk in self.disks:
-                if disk.removable:
-                    kind = "drive-removable-media"
-                else:
-                    kind = "drive-harddisk"
+        # properties: kind, description, capacity, os, popup-info
+        for disk in self.disks:
+            if disk.removable:
+                kind = "drive-removable-media"
+            else:
+                kind = "drive-harddisk"
+
+            size = size_str(disk.size)
+            popup_info = "%s | %s" % (disk.name, disk.serial)
 
-                size = size_str(disk.size)
-                popup_info = "%s | %s" % (disk.name, disk.serial)
+            @gtk_thread_wait
+            def gtk_action():
                 overview = AnacondaWidgets.DiskOverview(disk.description,
                                                         kind,
                                                         size,
@@ -541,8 +542,10 @@ class StorageSpoke(NormalSpoke, StorageChecker):
                 overview.connect("key-release-event", self._on_disk_clicked)
                 overview.show_all()
 
-            self._update_summary()
+                self._update_summary()
 
+            gtk_action()
+                
         self._ready = True
         communication.send_ready(self.__class__.__name__)
 
-- 
1.7.11.4



More information about the anaconda-patches mailing list