fix race in starup/restart logic

I am getting:

2024-04-06 13:38:10,669 ERROR libqtile core.py:_xpoll():L372 Got an exception in poll loop
Traceback (most recent call last):
  File "/home/tycho/.local/lib/python3.11/site-packages/libqtile/backend/x11/core.py", line 347, in _xpoll
    self.handle_event(event)
  File "/home/tycho/.local/lib/python3.11/site-packages/libqtile/backend/x11/core.py", line 314, in handle_event
    ret = target(event)
          ^^^^^^^^^^^^^
  File "/home/tycho/.local/lib/python3.11/site-packages/libqtile/backend/x11/core.py", line 663, in handle_KeyPress
    key, handled = self.qtile.process_key_event(keysym, event.state & self._valid_mask)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tycho/.local/lib/python3.11/site-packages/libqtile/core/manager.py", line 449, in process_key_event
    if cmd.check(self):
       ^^^^^^^^^^^^^^^
  File "/home/tycho/.local/lib/python3.11/site-packages/libqtile/lazy.py", line 148, in check
    cur_win_floating = q.current_window and q.current_window.floating
                       ^^^^^^^^^^^^^^^^
  File "/home/tycho/.local/lib/python3.11/site-packages/libqtile/command/base.py", line 281, in __getattr__
    raise AttributeError(f"{self.__class__} has no attribute {name}")
AttributeError: <class 'libqtile.core.manager.Qtile'> has no attribute current_window

on restart.

current_screen is first set in:

loop
  -> async_loop
    -> load_config()
      -> _process_screens()

but in async_loop(), we setup_listener() before we load the config, i.e.:

loop
  -> async_loop
    -> setup_listener() # whoops
    ...
    -> load_config()
      -> _process_screens()

This means that we could potentially race and start receiving events before
we were completely configured, as above. This is especially likely if we
are restarting, but the race exists as well on startup and could be present
if e.g. people were starting things in their xsessions before exec()ing
qtile.

To fix this, we load the config before we setup the listener, so that
everything will be present. But there's a bit of a circular dependency we
need to break here: setup_listener() is the one that initialize's the
Core's copy of the qtile object. Let's just set that explicitly in
Core's async_loop(), and remove the parameter of setup_listener(). So then
we have:

1. initialize qtile object
2. load_config()
3. setup_listener()

and hopefully we won't see more races like this. Indeed, what clued me in
that it was a race vs just a bug is that I saw other missing attributes
sometimes on restarts e.g.:

    AttributeError: <class 'libqtile.backend.x11.window.Window'> has no attribute has_focus

This is an interesting bug: it seems like it has been around for quite a
long time, but perhaps the new asyncio and python speed improvements are
making it such that now we can actually see it?

Fixes #4755

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>

libqtile/backend/base/core.py

@@ -19,6 +19,7 @@
 class Core(CommandObject, metaclass=ABCMeta):
     painter: Any
     supports_restarting: bool = True
+    qtile: Qtile
 
     @property
     @abstractmethod
@@ -42,7 +43,7 @@ def display_name(self) -> str:
         pass
 
     @abstractmethod
-    def setup_listener(self, qtile: Qtile) -> None:
+    def setup_listener(self) -> None:
         """Setup a listener for the given qtile instance"""
 
     @abstractmethod

libqtile/backend/wayland/core.py

@@ -105,7 +105,6 @@
     from wlroots.wlr_types.data_device_manager import Drag
 
     from libqtile import config
-    from libqtile.core.manager import Qtile
 
 
 class ImplicitGrab(wlrq.HasListeners):
@@ -143,8 +142,6 @@ class Core(base.Core, wlrq.HasListeners):
 
     def __init__(self) -> None:
         """Setup the Wayland core backend"""
-        self.qtile: Qtile | None = None
-
         # This is the window under the pointer
         self._hovered_window: window.WindowType | None = None
         # but this Internal receives keyboard input, e.g. via the Prompt widget.
@@ -393,7 +390,8 @@ def finalize(self) -> None:
         self.seat.destroy()
         self.backend.destroy()
         self.display.destroy()
-        self.qtile = None
+        if hasattr(self, "qtile"):
+            delattr(self, "qtile")
 
     @property
     def display_name(self) -> str:
@@ -795,9 +793,6 @@ def _on_new_idle_inhibitor(
     ) -> None:
         logger.debug("Signal: idle_inhibitor new_inhibitor")
 
-        if self.qtile is None:
-            return
-
         for win in self.qtile.windows_map.values():
             if isinstance(win, (window.Window, window.Static)):
                 win.surface.for_each_surface(win.add_idle_inhibitor, idle_inhibitor)
@@ -1172,10 +1167,9 @@ def _configure_pending_inputs(self) -> None:
                 device.configure(self.qtile.config.wl_input_rules)
         self._pending_input_devices.clear()
 
-    def setup_listener(self, qtile: Qtile) -> None:
+    def setup_listener(self) -> None:
         """Setup a listener for the given qtile instance"""
         logger.debug("Adding io watch")
-        self.qtile = qtile
         self.fd = lib.wl_event_loop_get_fd(self.event_loop._ptr)
         if self.fd:
             asyncio.get_running_loop().add_reader(self.fd, self._poll)

libqtile/backend/wayland/inputs.py

@@ -247,10 +247,7 @@ def _on_modifier(self, _listener: Listener, _data: Any) -> None:
         self.seat.keyboard_notify_modifiers(self.keyboard.modifiers)
 
     def _on_key(self, _listener: Listener, event: KeyboardKeyEvent) -> None:
-        if self.qtile is None:
-            # shushes mypy
-            self.qtile = self.core.qtile
-            assert self.qtile is not None
+        self.qtile = self.core.qtile
 
         self.core.idle.notify_activity(self.seat)
 

libqtile/backend/x11/core.py

@@ -151,7 +151,6 @@ def __init__(self, display_name: str | None = None) -> None:
         # setup the default cursor
         self._root.set_cursor("left_ptr")
 
-        self.qtile = None  # type: Qtile | None
         self._painter = None
         self._xtest = self.conn.conn(xcffib.xtest.key)
 
@@ -181,7 +180,8 @@ def finalize(self) -> None:
                 self._root.wid,
                 self.conn.atoms["_NET_SUPPORTING_WM_CHECK"],
             ).check()
-        self.qtile = None
+        if hasattr(self, "qtile"):
+            delattr(self, "qtile")
         self.conn.finalize()
 
     def get_screen_info(self) -> list[tuple[int, int, int, int]]:
@@ -211,7 +211,10 @@ def wmname(self, wmname):
         self._wmname = wmname
         self._supporting_wm_check_window.set_property("_NET_WM_NAME", wmname)
 
-    def setup_listener(self, qtile: "Qtile") -> None:
+    def set_qtile(self, qtile: "Qtile") -> None:
+        self.qtile
+
+    def setup_listener(self) -> None:
         """Setup a listener for the given qtile instance
 
         :param qtile:
@@ -220,7 +223,6 @@ def setup_listener(self, qtile: "Qtile") -> None:
             The eventloop to use to listen to the file descriptor.
         """
         logger.debug("Adding io watch")
-        self.qtile = qtile
         self.fd = self.conn.conn.get_file_descriptor()
         asyncio.get_running_loop().add_reader(self.fd, self._xpoll)
 

libqtile/core/manager.py

@@ -81,7 +81,7 @@ def __init__(
         state: str | None = None,
         socket_path: str | None = None,
     ) -> None:
-        self.core = kore
+        self.core: base.Core = kore
         self.config = config
         self.no_spawn = no_spawn
         self._state: QtileState | str | None = state
@@ -210,7 +210,9 @@ async def async_loop(self) -> None:
         # Set the event loop policy to facilitate access to main event loop
         asyncio.set_event_loop_policy(QtileEventLoopPolicy(self))
         self._stopped_event = asyncio.Event()
-        self.core.setup_listener(self)
+        self.core.qtile = self
+        self.load_config(initial=True)
+        self.core.setup_listener()
         try:
             async with LoopContext(
                 {
@@ -224,7 +226,6 @@ async def async_loop(self) -> None:
                 self._prepare_socket_path(self.socket_path),
                 self.server.call,
             ):
-                self.load_config(initial=True)
                 await self._stopped_event.wait()
         finally:
             self.finalize()