#11705 Make gireactor properly work with Gtk+3 and Gtk4 (#11706)

.coveragerc

@@ -4,6 +4,7 @@ parallel = True
 source = twisted
 omit =
     .tox/*/tmp/_trial_temp/*
+    */twisted/internet/gtk2reactor.py
 
 [paths]
 source=

.github/workflows/test.yaml

@@ -160,13 +160,15 @@ jobs:
             trial-target: 'twisted.test.test_compat twisted.test.test_defer twisted.internet.test.test_socket twisted.trial.test.test_tests'
             job-name: 'with-coverage'
 
-          # We run selected test for GTK reactor inside X virtual framebuffer.
+          # We run the full test suite with the GI reactor against an X virtual
+          # framebuffer server.  This covers our integration with the version
+          # of Gtk packaged within our selected version of Linux above.
           - python-version: '3.7'
             tox-env: 'alldeps-gtk-withcov-posix'
-            trial-target: 'twisted.internet.test'
             platform-deps: 'gtk_platform'
             tox-wrapper: 'xvfb-run -a'
             job-name: 'gtk-tests'
+            trial-args: '--reactor=gi -j 4'
 
 
     steps:

src/twisted/internet/_deprecate.py

@@ -0,0 +1,25 @@
+"""
+Support similar deprecation of several reactors.
+"""
+
+import warnings
+
+from incremental import Version, getVersionString
+
+from twisted.python.deprecate import DEPRECATION_WARNING_FORMAT
+
+
+def deprecatedGnomeReactor(name: str, version: Version) -> None:
+    """
+    Emit a deprecation warning about a gnome-related reactor.
+
+    @param name: The name of the reactor.  For example, C{"gtk2reactor"}.
+
+    @param version: The version in which the deprecation was introduced.
+    """
+    stem = DEPRECATION_WARNING_FORMAT % {
+        "fqpn": "twisted.internet." + name,
+        "version": getVersionString(version),
+    }
+    msg = stem + ".  Please use twisted.internet.gireactor instead."
+    warnings.warn(msg, category=DeprecationWarning)

src/twisted/internet/_glibbase.py

@@ -13,11 +13,13 @@
 
 
 import sys
+from typing import Any, Callable, Dict, Set
 
 from zope.interface import implementer
 
 from twisted.internet import base, posixbase, selectreactor
-from twisted.internet.interfaces import IReactorFDSet
+from twisted.internet.abstract import FileDescriptor
+from twisted.internet.interfaces import IReactorFDSet, IReadDescriptor, IWriteDescriptor
 from twisted.python import log
 from ._signals import _UnixWaker
 
@@ -59,6 +61,16 @@ def doRead(self) -> None:
         self.reactor._simulate()
 
 
+def _loopQuitter(
+    idleAdd: Callable[[Callable[[], None]], None], loopQuit: Callable[[], None]
+) -> Callable[[], None]:
+    """
+    Combine the C{glib.idle_add} and C{glib.MainLoop.quit} functions into a
+    function suitable for crashing the reactor.
+    """
+    return lambda: idleAdd(loopQuit)
+
+
 @implementer(IReactorFDSet)
 class GlibReactorBase(posixbase.PosixReactorBase, posixbase._PollLikeMixin):
     """
@@ -96,34 +108,23 @@ class GlibReactorBase(posixbase.PosixReactorBase, posixbase._PollLikeMixin):
     # callbacks queued from a thread:
     _wakerFactory = GlibWaker
 
-    def __init__(self, glib_module, gtk_module, useGtk=False):
+    def __init__(self, glib_module: Any, gtk_module: Any, useGtk: bool = False) -> None:
         self._simtag = None
-        self._reads = set()
-        self._writes = set()
-        self._sources = {}
+        self._reads: Set[IReadDescriptor] = set()
+        self._writes: Set[IWriteDescriptor] = set()
+        self._sources: Dict[FileDescriptor, int] = {}
         self._glib = glib_module
-        self._gtk = gtk_module
         posixbase.PosixReactorBase.__init__(self)
 
         self._source_remove = self._glib.source_remove
         self._timeout_add = self._glib.timeout_add
 
-        def _mainquit():
-            if self._gtk.main_level():
-                self._gtk.main_quit()
-
-        if useGtk:
-            self._pending = self._gtk.events_pending
-            self._iteration = self._gtk.main_iteration_do
-            self._crash = _mainquit
-            self._run = self._gtk.main
-        else:
-            self.context = self._glib.main_context_default()
-            self._pending = self.context.pending
-            self._iteration = self.context.iteration
-            self.loop = self._glib.MainLoop()
-            self._crash = lambda: self._glib.idle_add(self.loop.quit)
-            self._run = self.loop.run
+        self.context = self._glib.main_context_default()
+        self._pending = self.context.pending
+        self._iteration = self.context.iteration
+        self.loop = self._glib.MainLoop()
+        self._crash = _loopQuitter(self._glib.idle_add, self.loop.quit)
+        self._run = self.loop.run
 
     def _handleSignals(self):
         # First, install SIGINT and friends:
@@ -317,26 +318,17 @@ class PortableGlibReactorBase(selectreactor.SelectReactor):
     Sockets aren't supported by GObject's input_add on Win32.
     """
 
-    def __init__(self, glib_module, gtk_module, useGtk=False):
+    def __init__(self, glib_module: Any, gtk_module: Any, useGtk: bool = False) -> None:
         self._simtag = None
         self._glib = glib_module
-        self._gtk = gtk_module
         selectreactor.SelectReactor.__init__(self)
 
         self._source_remove = self._glib.source_remove
         self._timeout_add = self._glib.timeout_add
 
-        def _mainquit():
-            if self._gtk.main_level():
-                self._gtk.main_quit()
-
-        if useGtk:
-            self._crash = _mainquit
-            self._run = self._gtk.main
-        else:
-            self.loop = self._glib.MainLoop()
-            self._crash = lambda: self._glib.idle_add(self.loop.quit)
-            self._run = self.loop.run
+        self.loop = self._glib.MainLoop()
+        self._crash = _loopQuitter(self._glib.idle_add, self.loop.quit)
+        self._run = self.loop.run
 
     def crash(self):
         selectreactor.SelectReactor.crash(self)

src/twisted/internet/gireactor.py

@@ -21,24 +21,14 @@
 """
 
 
-import gi.pygtkcompat  # type: ignore[import]
-from gi.repository import GLib  # type: ignore[import]
+from typing import Union
+
+from gi.repository import GLib  # type:ignore[import]
 
 from twisted.internet import _glibbase
 from twisted.internet.error import ReactorAlreadyRunning
 from twisted.python import runtime
 
-# We require a sufficiently new version of pygobject, so always exists:
-_pygtkcompatPresent = True
-
-# Newer version of gi, so we can try to initialize compatibility layer; if
-# real pygtk was already imported we'll get ImportError at this point
-# rather than segfault, so unconditional import is fine.
-gi.pygtkcompat.enable()
-# At this point importing gobject will get you gi version, and importing
-# e.g. gtk will either fail in non-segfaulty way or use gi version if user
-# does gi.pygtkcompat.enable_gtk(). So, no need to prevent imports of
-# old school pygtk modules.
 if getattr(GLib, "threads_init", None) is not None:
     GLib.threads_init()
 
@@ -67,11 +57,7 @@ class GIReactor(_glibbase.GlibReactorBase):
     _gapplication = None
 
     def __init__(self, useGtk=False):
-        _gtk = None
-        if useGtk is True:
-            from gi.repository import Gtk as _gtk
-
-        _glibbase.GlibReactorBase.__init__(self, GLib, _gtk, useGtk=useGtk)
+        _glibbase.GlibReactorBase.__init__(self, GLib, None)
 
     def registerGApplication(self, app):
         """
@@ -111,11 +97,7 @@ class PortableGIReactor(_glibbase.PortableGlibReactorBase):
     """
 
     def __init__(self, useGtk=False):
-        _gtk = None
-        if useGtk is True:
-            from gi.repository import Gtk as _gtk
-
-        _glibbase.PortableGlibReactorBase.__init__(self, GLib, _gtk, useGtk=useGtk)
+        _glibbase.PortableGlibReactorBase.__init__(self, GLib, None, useGtk=useGtk)
 
     def registerGApplication(self, app):
         """
@@ -125,12 +107,12 @@ def registerGApplication(self, app):
         raise NotImplementedError("GApplication is not currently supported on Windows.")
 
 
-def install(useGtk=False):
+def install(useGtk: bool = False) -> Union[GIReactor, PortableGIReactor]:
     """
     Configure the twisted mainloop to be run inside the glib mainloop.
 
-    @param useGtk: should GTK+ rather than glib event loop be
-        used (this will be slightly slower but does support GUI).
+    @param useGtk: A hint that the Gtk GUI will or will not be used.  Currently
+        does not modify any behavior.
     """
     if runtime.platform.getType() == "posix":
         reactor = GIReactor(useGtk=useGtk)

src/twisted/internet/glib2reactor.py

@@ -16,6 +16,12 @@
 intended to be called directly.
 """
 
+from incremental import Version
+
+from ._deprecate import deprecatedGnomeReactor
+
+deprecatedGnomeReactor("glib2reactor", Version("Twisted", "NEXT", 0, 0))
+
 from twisted.internet import gtk2reactor
 
 

src/twisted/internet/gtk2reactor.py

@@ -16,6 +16,12 @@
 intended to be called directly.
 """
 
+from incremental import Version
+
+from ._deprecate import deprecatedGnomeReactor
+
+deprecatedGnomeReactor("gtk2reactor", Version("Twisted", "NEXT", 0, 0))
+
 # System Imports
 import sys
 
@@ -45,6 +51,14 @@
 
 import gobject  # type: ignore[import]
 
+if not hasattr(gobject, "IO_HUP"):
+    # gi.repository's legacy compatibility helper raises an AttributeError with
+    # a custom error message rather than a useful ImportError, so things tend
+    # to fail loudly.  Things that import this module expect an ImportError if,
+    # well, something failed to import, and treat an AttributeError as an
+    # arbitrary application code failure, so we satisfy that expectation here.
+    raise ImportError("pygobject 2.x is not installed. Use the `gi` reactor.")
+
 if hasattr(gobject, "threads_init"):
     # recent versions of python-gtk expose this. python-gtk=2.4.1
     # (wrapping glib-2.4.7) does. python-gtk=2.0.0 (wrapping

src/twisted/internet/gtk3reactor.py

@@ -2,64 +2,21 @@
 # See LICENSE for details.
 
 """
-This module provides support for Twisted to interact with the gtk3 mainloop
-via Gobject introspection. This is like gi, but slightly slower and requires a
-working $DISPLAY.
-
-In order to use this support, simply do the following::
-
-    from twisted.internet import gtk3reactor
-    gtk3reactor.install()
-
-If you wish to use a GApplication, register it with the reactor::
-
-    from twisted.internet import reactor
-    reactor.registerGApplication(app)
-
-Then use twisted.internet APIs as usual.
+This module is a legacy compatibility alias for L{twisted.internet.gireactor}.
+See that module instead.
 """
 
-from twisted.internet import gireactor
-from twisted.python import runtime
-
+from incremental import Version
 
-class Gtk3Reactor(gireactor.GIReactor):
-    """
-    A reactor using the gtk3+ event loop.
-    """
+from ._deprecate import deprecatedGnomeReactor
 
-    def __init__(self):
-        """
-        Override init to set the C{useGtk} flag.
-        """
-        gireactor.GIReactor.__init__(self, useGtk=True)
+deprecatedGnomeReactor("gtk3reactor", Version("Twisted", "NEXT", 0, 0))
 
+from twisted.internet import gireactor
 
-class PortableGtk3Reactor(gireactor.PortableGIReactor):
-    """
-    Portable GTK+ 3.x reactor.
-    """
-
-    def __init__(self):
-        """
-        Override init to set the C{useGtk} flag.
-        """
-        gireactor.PortableGIReactor.__init__(self, useGtk=True)
-
-
-def install():
-    """
-    Configure the Twisted mainloop to be run inside the gtk3+ mainloop.
-    """
-    if runtime.platform.getType() == "posix":
-        reactor = Gtk3Reactor()
-    else:
-        reactor = PortableGtk3Reactor()
-
-    from twisted.internet.main import installReactor
-
-    installReactor(reactor)
-    return reactor
+Gtk3Reactor = gireactor.GIReactor
+PortableGtk3Reactor = gireactor.PortableGIReactor
 
+install = gireactor.install
 
 __all__ = ["install"]

src/twisted/internet/test/reactormixins.py

@@ -18,7 +18,7 @@
 import os
 import signal
 import time
-from typing import TYPE_CHECKING, Dict, Optional, Sequence, Type, Union
+from typing import TYPE_CHECKING, Dict, Optional, Sequence, Type, Union, cast
 
 from zope.interface import Interface
 
@@ -141,20 +141,15 @@ class ReactorBuilder:
         # since no one really wants to use it on other platforms.
         _reactors.extend(
             [
-                "twisted.internet.gtk2reactor.PortableGtkReactor",
                 "twisted.internet.gireactor.PortableGIReactor",
-                "twisted.internet.gtk3reactor.PortableGtk3Reactor",
                 "twisted.internet.win32eventreactor.Win32Reactor",
                 "twisted.internet.iocpreactor.reactor.IOCPReactor",
             ]
         )
     else:
         _reactors.extend(
             [
-                "twisted.internet.glib2reactor.Glib2Reactor",
-                "twisted.internet.gtk2reactor.Gtk2Reactor",
                 "twisted.internet.gireactor.GIReactor",
-                "twisted.internet.gtk3reactor.Gtk3Reactor",
             ]
         )
 
@@ -386,13 +381,20 @@ def asyncioSelectorReactor(self: object) -> "asyncioreactor.AsyncioSelectorReact
     @param self: The L{ReactorBuilder} subclass this is being called on.  We
         don't use this parameter but we get called with it anyway.
     """
-    from asyncio import new_event_loop, set_event_loop
+    from asyncio import get_event_loop, new_event_loop, set_event_loop
 
     from twisted.internet import asyncioreactor
 
+    asTestCase = cast(SynchronousTestCase, self)
+    originalLoop = get_event_loop()
     loop = new_event_loop()
     set_event_loop(loop)
 
+    @asTestCase.addCleanup
+    def cleanUp():
+        loop.close()
+        set_event_loop(originalLoop)
+
     return asyncioreactor.AsyncioSelectorReactor(loop)