From c49519008bb5f4aa5399b933e2cccde2f014f081 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 21 May 2026 17:01:55 +0000 Subject: [PATCH] fix: prevent thundering-herd of concurrent Syncthing REST API calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The background-thread refresh introduced in the previous commit had a new problem: _auto_refresh fires every 3s, but connected_devices() can take up to 30s (3 calls × 10s timeout). Without a guard, 10 overlapping refresh threads accumulate, each making 3 requests = 30 simultaneous requests to Syncthing's REST API, saturating it and making timeouts self-reinforcing. - Add _refreshing flag to _DevicesContent and IncomingWindow: a new fetch is skipped if one is already in flight; flag is cleared in _apply_refresh (or immediately if the window is gone). - Increase auto-refresh interval 3s → 10s in both windows to reduce steady-state API call rate. - Cache get_device_id() in SyncthingClient: the local device ID never changes, but was fetched on every connected_devices() call (every refresh cycle). - Make connected_devices() resilient: if get_connections() times out, return devices with unknown connection status instead of raising. - Downgrade PendingDeviceWatcher timeout logs from exception (full traceback) to warning: a ReadTimeout is routine when Syncthing is busy, not an unexpected crash. https://claude.ai/code/session_0128MyVVEu1Dt3jTeaZ1edyL --- clipsync/pairing.py | 3 +++ clipsync/syncthing.py | 13 ++++++++++--- clipsync/ui.py | 18 ++++++++++++++++-- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/clipsync/pairing.py b/clipsync/pairing.py index a35e241..19d2ece 100644 --- a/clipsync/pairing.py +++ b/clipsync/pairing.py @@ -16,6 +16,7 @@ from collections.abc import Callable import qrcode +import requests from PIL import Image from . import config @@ -174,6 +175,8 @@ def _run(self) -> None: while not self._stop.is_set(): try: self._tick() + except requests.RequestException as exc: + log.warning("Pending device watcher: Syncthing not responding (%s)", exc) except Exception: log.exception("Error in pending device watcher") if self._stop.wait(self._interval): diff --git a/clipsync/syncthing.py b/clipsync/syncthing.py index 0e7bd65..a904b4d 100644 --- a/clipsync/syncthing.py +++ b/clipsync/syncthing.py @@ -510,6 +510,7 @@ def __init__(self, api_key: str, base_url: str = config.SYNCTHING_API_URL) -> No self._base = base_url.rstrip("/") self._session = requests.Session() self._session.headers["X-API-Key"] = api_key + self._cached_device_id: str | None = None def _url(self, path: str) -> str: return f"{self._base}{path}" @@ -557,8 +558,10 @@ def wait_until_ready(self, timeout: float = _STARTUP_WAIT) -> bool: return False def get_device_id(self) -> str: - status = self._get("/rest/system/status") - return status["myID"] + if self._cached_device_id is None: + status = self._get("/rest/system/status") + self._cached_device_id = status["myID"] + return self._cached_device_id def get_config(self) -> dict[str, Any]: return self._get("/rest/config") @@ -646,7 +649,11 @@ def connected_devices(self) -> list[dict[str, Any]]: """Return a list of {deviceID, name, connected, address} for paired devices.""" devices = self.get_devices() my_id = self.get_device_id() - connections = self.get_connections().get("connections") or {} + try: + connections = self.get_connections().get("connections") or {} + except requests.RequestException: + # Connection status is best-effort; don't fail the whole call. + connections = {} out: list[dict[str, Any]] = [] for d in devices: did = d.get("deviceID") diff --git a/clipsync/ui.py b/clipsync/ui.py index e31998a..835b5db 100644 --- a/clipsync/ui.py +++ b/clipsync/ui.py @@ -552,6 +552,7 @@ def __init__( ) -> None: self._win = window self._app = app + self._refreshing = False ctk.CTkLabel(container, text="Connected devices", font=ctk.CTkFont(size=18, weight="bold")).pack(pady=(0, 10)) @@ -578,7 +579,7 @@ def _exists(self) -> bool: def _schedule_refresh(self) -> None: if not self._exists(): return - self._win.after(3000, self._auto_refresh) + self._win.after(10_000, self._auto_refresh) def _auto_refresh(self) -> None: if not self._exists(): @@ -587,6 +588,9 @@ def _auto_refresh(self) -> None: self._schedule_refresh() def _refresh(self) -> None: + if self._refreshing: + return + self._refreshing = True for child in self._list_frame.winfo_children(): child.destroy() ctk.CTkLabel(self._list_frame, text="Loading…", text_color=("gray50", "gray60")).pack(pady=20) @@ -604,8 +608,11 @@ def _do_refresh(self) -> None: error = str(exc) if self._exists(): self._win.after(0, self._apply_refresh, devices, error) + else: + self._refreshing = False def _apply_refresh(self, devices: list[dict], error: str | None) -> None: + self._refreshing = False for child in self._list_frame.winfo_children(): child.destroy() if error: @@ -1195,6 +1202,7 @@ def __init__(self, parent: ctk.CTk, app: AppContext, on_close: Callable[[], None super().__init__(parent, f"{config.APP_NAME} — Incoming Requests", (440, 360), on_close) self._app = app self._handled: set[str] = set() + self._refreshing = False container = ctk.CTkFrame(self.window, fg_color="transparent") container.pack(fill="both", expand=True, padx=20, pady=20) @@ -1221,7 +1229,7 @@ def __init__(self, parent: ctk.CTk, app: AppContext, on_close: Callable[[], None def _schedule_refresh(self) -> None: if not self.exists(): return - self.window.after(3000, self._auto_refresh) + self.window.after(10_000, self._auto_refresh) def _auto_refresh(self) -> None: if not self.exists(): @@ -1230,6 +1238,9 @@ def _auto_refresh(self) -> None: self._schedule_refresh() def _refresh(self) -> None: + if self._refreshing: + return + self._refreshing = True for child in self._list_frame.winfo_children(): child.destroy() ctk.CTkLabel(self._list_frame, text="Loading…", text_color=("gray50", "gray60")).pack(pady=20) @@ -1247,8 +1258,11 @@ def _do_refresh(self) -> None: error = str(exc) if self.exists(): self.window.after(0, self._apply_refresh, pending, error) + else: + self._refreshing = False def _apply_refresh(self, pending: dict, error: str | None) -> None: + self._refreshing = False for child in self._list_frame.winfo_children(): child.destroy() if error: