From dc0436136bea5bb00d478be8ece71cd4d962dbbf Mon Sep 17 00:00:00 2001 From: The_miro Date: Thu, 9 Jul 2026 16:48:42 +0200 Subject: [PATCH] fix(astal-menu): fix layout-tab sync race across workspace switches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _sync_layout_controls() fires 3 concurrent hyprctl calls, and refresh() can trigger it twice per open (once from the clients query, once from the activeworkspace one) — each rebuilding a fresh set of layout widgets. Two bugs followed: 1. The 3 callbacks each toggled the shared self._syncing flag independently (True at start, False at end). Whichever resolved first dropped the guard while the others — notably the tab- selection one, which programmatically flips the visible stack child — were still in flight. That let _on_tab_switch/_on_opt_change misfire as if the user had clicked, re-applying stale layout data via layouts.set() to whatever workspace was active by the time the callback landed. 2. A second _sync_layout_controls() call (from the redundant rebuild) could land mid-flight of the first, so a stale callback from sync #1 could decrement sync #2's guard or write sync #1's data into sync #2's freshly built widgets. Fixed with a generation token: any sync superseded by a newer one is invalidated outright, and self._syncing only lifts once the *current* generation's 3 calls have all resolved. This is what "query the current workspace's layout when opened, so it doesn't break switching workspaces" needed — refresh() already re-queried on open, the gap was in trusting stale in-flight results. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TLz6HWsXCwzQ97LrLt2em6 --- desktopenvs/hyprlua/astal-menu/ui/taskbar.py | 63 ++++++++++++++++---- 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/desktopenvs/hyprlua/astal-menu/ui/taskbar.py b/desktopenvs/hyprlua/astal-menu/ui/taskbar.py index fc4efd9..884b469 100644 --- a/desktopenvs/hyprlua/astal-menu/ui/taskbar.py +++ b/desktopenvs/hyprlua/astal-menu/ui/taskbar.py @@ -340,38 +340,81 @@ class Taskbar(Gtk.Box): # -- sync + handlers --------------------------------------------------- def _sync_layout_controls(self) -> None: - # reflect the current per-ws layout / direction / focus_fit in the tabs+options + # Reflect the current per-ws layout / direction / focus_fit in the tabs+options. + # + # This fires 3 concurrent hyprctl subprocess calls with no guaranteed resolution + # order, and refresh() can trigger _rebuild_panel() (hence this) twice in a row + # (once from the clients query, once from the activeworkspace one) — each + # rebuilding a fresh _layout_stack/_dir_dds/_fit_sws. Two failure modes existed: + # 1. The 3 callbacks used to each toggle the shared `self._syncing` flag + # independently (True at start, False at end). Whichever resolved *first* + # dropped the guard while the others — in particular the tab-selection one, + # which programmatically flips the visible stack child — were still in + # flight, letting `_on_tab_switch`/`_on_opt_change` misfire as if the user + # had clicked and re-apply stale data via `layouts.set(...)`. + # 2. A second `_sync_layout_controls()` call (from the redundant rebuild) could + # land while the first's callbacks were still pending, so a stale callback + # from sync #1 could decrement sync #2's pending-count or apply sync #1's + # (possibly wrong-workspace) data into sync #2's freshly built widgets. + # A generation token invalidates any sync superseded by a newer one outright — + # if the user switches workspaces (or a redundant rebuild fires) mid-flight, only + # the latest call's callbacks are allowed to touch anything, and self._syncing + # only lifts once *that* generation's 3 calls have all resolved. + self._sync_generation = getattr(self, "_sync_generation", 0) + 1 + gen = self._sync_generation + ws = self._active_ws self._syncing = True - run_json(["hyprctl", "getoption", "general:layout", "-j"], self._apply_tab_sel) - run_json(["hyprctl", "getoption", "scrolling:direction", "-j"], self._apply_dir_sel) - run_json(["hyprctl", "getoption", "scrolling:focus_fit_method", "-j"], self._apply_fit_sel) + pending = [3] + + def _done() -> None: + if gen != self._sync_generation: + return # superseded — a newer sync owns self._syncing now + pending[0] -= 1 + if pending[0] <= 0: + self._syncing = False + + def _tab_cb(ok, data) -> None: + if gen == self._sync_generation: + self._apply_tab_sel(ws, ok, data) + _done() + + def _dir_cb(ok, data) -> None: + if gen == self._sync_generation: + self._apply_dir_sel(ok, data) + _done() + + def _fit_cb(ok, data) -> None: + if gen == self._sync_generation: + self._apply_fit_sel(ok, data) + _done() + + run_json(["hyprctl", "getoption", "general:layout", "-j"], _tab_cb) + run_json(["hyprctl", "getoption", "scrolling:direction", "-j"], _dir_cb) + run_json(["hyprctl", "getoption", "scrolling:focus_fit_method", "-j"], _fit_cb) # columns' own center-focused state lives in columns-state.json, not a hyprctl # option — read it synchronously rather than round-tripping a subprocess. columns_sw = self._fit_sws.get("columns") if columns_sw is not None: columns_sw.set_active(self._read_center()) - def _apply_tab_sel(self, ok, data) -> None: + def _apply_tab_sel(self, ws, ok, data) -> None: cur = data.get("str") if ok and isinstance(data, dict) else None # prefer the per-workspace layout recorded by hypr/layouts (layouts.set) try: state = json.loads(_LAYOUTS_STATE.read_text()) - cur = state.get(str(self._active_ws), cur) + cur = state.get(str(ws), cur) except (FileNotFoundError, json.JSONDecodeError): pass if cur and any(ly["name"] == cur for ly in self._layouts): self._layout_stack.set_visible_child_name(cur) - self._syncing = False def _apply_dir_sel(self, ok, data) -> None: # scrolling:direction is global; reflect it in every directional layout's # dropdown whose value set contains it. cur = data.get("str") if ok and isinstance(data, dict) else None - self._syncing = True for dd, values in self._dir_dds.values(): if cur in values: dd.set_selected(values.index(cur)) - self._syncing = False def _apply_fit_sel(self, ok, data) -> None: # scrolling:focus_fit_method only describes the scrolling layout's own switch; @@ -380,9 +423,7 @@ class Taskbar(Gtk.Box): val = data.get("int") if ok and isinstance(data, dict) else 0 sw = self._fit_sws.get("scrolling") if sw is not None: - self._syncing = True sw.set_active(val == 1) - self._syncing = False def _cur_dir(self) -> str: entry = self._dir_dds.get(self._layout_stack.get_visible_child_name())