From e40a32d87eef29cdbebc3a60ca3078aa86f7f36c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 18:03:26 +0000 Subject: [PATCH 1/6] fix: prevent browser paste freeze from xclip hammering clipboard every 0.5 s On Linux, _out_tick called _read_clipboard_image() on every 0.5 s poll tick. That spawned xclip which sent an X11 SelectionRequest to the clipboard owner (typically a browser). Browsers service these on their main thread, so 2 requests/second caused the browser to freeze the moment the user pressed Ctrl+V. Two-layer fix: 1. TARGETS pre-check (_linux_clipboard_has_image): before requesting image bytes, run xclip/wl-paste with TARGETS/--list-types. The clipboard owner responds immediately with a list of formats; if image/png is absent we return None without ever sending an image/png SelectionRequest. This eliminates the expensive request for the common case (text on clipboard). 2. Rate-limit image checks in _out_tick to _LINUX_IMAGE_CHECK_INTERVAL (2 s) on Linux. Even the cheap TARGETS request spawns a subprocess and touches the X server, so throttling to 2 s reduces X11 traffic 4x vs the previous 0.5 s rate. Windows and macOS are unaffected (ImageGrab.grabclipboard() is in-process). Image sync latency on Linux increases to ~2 s in the worst case, which is acceptable. Text sync latency is unchanged. 16 regression tests added in tests/test_linux_paste_freeze.py covering the TARGETS gate, rate-limiter logic, and end-to-end image/text sync correctness. --- clipsync/clipboard.py | 74 ++++-- tests/test_linux_paste_freeze.py | 390 +++++++++++++++++++++++++++++++ 2 files changed, 451 insertions(+), 13 deletions(-) create mode 100644 tests/test_linux_paste_freeze.py diff --git a/clipsync/clipboard.py b/clipsync/clipboard.py index 8f8e80f..5014ef6 100644 --- a/clipsync/clipboard.py +++ b/clipsync/clipboard.py @@ -45,6 +45,12 @@ _PNG_HEADER = b"\x89PNG\r\n\x1a\n" +# Minimum seconds between image-clipboard polls on Linux. Checking for images +# requires running xclip/wl-paste which sends X11 SelectionRequests to the +# clipboard owner (typically a browser). Browsers service these on their main +# thread, so polling at the same 0.5 s interval as text caused paste freezes. +_LINUX_IMAGE_CHECK_INTERVAL = 2.0 + def _normalize_newlines(s: str) -> str: """Collapse CRLF/CR to LF so Windows's clipboard normalization does not @@ -52,6 +58,31 @@ def _normalize_newlines(s: str) -> str: return s.replace("\r\n", "\n").replace("\r", "\n") +def _linux_clipboard_has_image() -> bool: + """Return True only when the Linux clipboard advertises image/png via TARGETS. + + This is a cheap pre-check: the clipboard owner (e.g. a browser) responds to + TARGETS requests immediately on its main thread without serialising any data. + By gating the full image/png fetch on this check we avoid sending expensive + SelectionRequests when the clipboard holds text, which was the root cause of + browsers freezing on paste. + """ + for cmd in ( + ["xclip", "-selection", "clipboard", "-t", "TARGETS", "-o"], + ["wl-paste", "--list-types"], + ): + try: + result = subprocess.run(cmd, capture_output=True, timeout=1) + except FileNotFoundError: + continue # Tool not installed; try the next one. + except (subprocess.TimeoutExpired, OSError): + continue + # Once a tool responds, treat its answer as authoritative for this + # display-server type — no need to consult the other tool. + return result.returncode == 0 and b"image/png" in result.stdout + return False + + def _read_image_from_system_clipboard() -> bytes | None: """Return PNG bytes from the system clipboard, or None if no image is present.""" if sys.platform in ("win32", "darwin"): @@ -66,7 +97,11 @@ def _read_image_from_system_clipboard() -> bytes | None: return buf.getvalue() except Exception: return None - # Linux: try xclip then wl-paste. + # Linux: check TARGETS first so we never send an image/png SelectionRequest + # to the clipboard owner when only text is present. Without this gate, + # xclip hammered the browser's main thread every 0.5 s, causing paste freezes. + if not _linux_clipboard_has_image(): + return None # Some xclip versions return text content with exit 0 even when asked for # image/png and no image is on the clipboard. Guard with a PNG magic-byte # check so we never mistake text bytes for image data. @@ -152,6 +187,7 @@ def __init__(self, settings: config.Settings) -> None: self._last_read_error: str | None = None self._last_write_error: str | None = None self._last_decrypt_error: str | None = None + self._last_image_check: float = 0.0 self._history = ClipboardHistory(settings) @property @@ -403,18 +439,30 @@ def _out_loop(self) -> None: def _out_tick(self) -> None: # Images take priority: if the clipboard has an image, sync it. - image = self._read_clipboard_image() - if image is not None: - with self._lock: - if image == self._last_synced: - return - self._last_synced = image - try: - self._write_image_file(image) - log.info("OUT [%s]: %d bytes image written", _HOSTNAME, len(image)) - except OSError: - log.exception("OUT [%s]: Failed to write image file", _HOSTNAME) - return + # On Linux, throttle image checks to _LINUX_IMAGE_CHECK_INTERVAL seconds + # (vs the normal 0.5 s text-poll rate). Even with the TARGETS pre-check + # inside _read_clipboard_image, every image-check tick still spawns an + # xclip subprocess that sends an X11 SelectionRequest to the clipboard + # owner; keeping this at 0.5 s was enough to freeze browsers on paste. + now = time.monotonic() + _do_image_check = sys.platform in ("win32", "darwin") or ( + now - self._last_image_check >= _LINUX_IMAGE_CHECK_INTERVAL + ) + if _do_image_check: + if sys.platform not in ("win32", "darwin"): + self._last_image_check = now + image = self._read_clipboard_image() + if image is not None: + with self._lock: + if image == self._last_synced: + return + self._last_synced = image + try: + self._write_image_file(image) + log.info("OUT [%s]: %d bytes image written", _HOSTNAME, len(image)) + except OSError: + log.exception("OUT [%s]: Failed to write image file", _HOSTNAME) + return current = self._read_clipboard() if current is None or current == "": diff --git a/tests/test_linux_paste_freeze.py b/tests/test_linux_paste_freeze.py new file mode 100644 index 0000000..536c721 --- /dev/null +++ b/tests/test_linux_paste_freeze.py @@ -0,0 +1,390 @@ +"""Regression tests for the Linux paste-freeze bug. + +Root cause: _out_tick called _read_clipboard_image() on every 0.5 s poll tick. +On Linux that spawns xclip which sends an X11 SelectionRequest to the clipboard +owner (typically a browser). Browsers service these on their main thread, so +hammering at 0.5 s caused the browser to freeze when the user tried to paste. + +The fix has two parts: + 1. _read_image_from_system_clipboard() now runs a cheap TARGETS/list-types + pre-check and only fetches image bytes when the clipboard actually + advertises image/png. This is enforced even during the IN-loop + self-originated check so we never send image/png SelectionRequests when + the clipboard holds text. + 2. _out_tick() rate-limits image checks on Linux to _LINUX_IMAGE_CHECK_INTERVAL + seconds (default 2 s) instead of every poll tick. +""" + +from __future__ import annotations + +import io +import subprocess +import sys +import time +import types +import unittest.mock as mock + +import pytest +from PIL import Image +from watchdog.observers.polling import PollingObserver + +import clipsync.clipboard as clipboard_module +from clipsync import config +from clipsync.clipboard import ( + ClipboardSync, + _LINUX_IMAGE_CHECK_INTERVAL, + _linux_clipboard_has_image, + _read_image_from_system_clipboard, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_PNG_HEADER = b"\x89PNG\r\n\x1a\n" +POLL = 0.05 + + +def _make_png(color: tuple[int, int, int] = (0, 128, 255)) -> bytes: + img = Image.new("RGB", (1, 1), color=color) + buf = io.BytesIO() + img.save(buf, format="PNG") + return buf.getvalue() + + +FAKE_PNG = _make_png() + + +def _wait_for(predicate, timeout: float = 5.0, interval: float = 0.05) -> bool: + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if predicate(): + return True + time.sleep(interval) + return False + + +# --------------------------------------------------------------------------- +# _linux_clipboard_has_image unit tests +# --------------------------------------------------------------------------- + + +class TestLinuxClipboardHasImage: + """Unit tests for the TARGETS pre-check helper.""" + + def _run(self, cmd, **kwargs): + """Default subprocess.run stub: xclip not found, wl-paste not found.""" + raise FileNotFoundError + + def test_returns_false_when_no_tools_installed(self, monkeypatch): + monkeypatch.setattr(subprocess, "run", self._run) + assert _linux_clipboard_has_image() is False + + def test_returns_false_when_xclip_targets_has_no_image(self, monkeypatch): + def fake_run(cmd, **kwargs): + if "TARGETS" in cmd or "--list-types" in cmd: + result = mock.Mock() + result.returncode = 0 + result.stdout = b"UTF8_STRING\ntext/plain\nTARGETS\n" + return result + raise AssertionError(f"Unexpected command: {cmd}") + + monkeypatch.setattr(subprocess, "run", fake_run) + assert _linux_clipboard_has_image() is False + + def test_returns_true_when_xclip_targets_includes_image_png(self, monkeypatch): + def fake_run(cmd, **kwargs): + if "TARGETS" in cmd: + result = mock.Mock() + result.returncode = 0 + result.stdout = b"UTF8_STRING\nimage/png\ntext/plain\n" + return result + raise AssertionError(f"Unexpected command: {cmd}") + + monkeypatch.setattr(subprocess, "run", fake_run) + assert _linux_clipboard_has_image() is True + + def test_falls_back_to_wl_paste_when_xclip_missing(self, monkeypatch): + def fake_run(cmd, **kwargs): + if cmd[0] == "xclip": + raise FileNotFoundError + # wl-paste --list-types + result = mock.Mock() + result.returncode = 0 + result.stdout = b"image/png\ntext/plain\n" + return result + + monkeypatch.setattr(subprocess, "run", fake_run) + assert _linux_clipboard_has_image() is True + + def test_returns_false_when_xclip_exits_nonzero(self, monkeypatch): + """A non-zero exit from TARGETS means the clipboard is empty or an error + occurred — in both cases there is no image.""" + + def fake_run(cmd, **kwargs): + result = mock.Mock() + result.returncode = 1 + result.stdout = b"" + return result + + monkeypatch.setattr(subprocess, "run", fake_run) + assert _linux_clipboard_has_image() is False + + def test_skips_timed_out_tool_continues_to_next(self, monkeypatch): + calls: list[str] = [] + + def fake_run(cmd, **kwargs): + calls.append(cmd[0]) + if cmd[0] == "xclip": + raise subprocess.TimeoutExpired(cmd, 1) + # wl-paste returns image/png + result = mock.Mock() + result.returncode = 0 + result.stdout = b"image/png\n" + return result + + monkeypatch.setattr(subprocess, "run", fake_run) + result = _linux_clipboard_has_image() + assert "xclip" in calls + assert "wl-paste" in calls + assert result is True + + +# --------------------------------------------------------------------------- +# _read_image_from_system_clipboard: TARGETS gate on Linux +# --------------------------------------------------------------------------- + + +class TestReadImageGate: + """The TARGETS pre-check must prevent image/png requests when no image present.""" + + @pytest.fixture(autouse=True) + def _force_linux(self, monkeypatch): + monkeypatch.setattr(sys, "platform", "linux") + + def test_skips_image_fetch_when_targets_has_no_image(self, monkeypatch): + """When TARGETS returns no image/png the image/png fetch must not run.""" + image_fetch_calls: list[list[str]] = [] + + def fake_run(cmd, **kwargs): + if "TARGETS" in cmd or "--list-types" in cmd: + result = mock.Mock() + result.returncode = 0 + result.stdout = b"UTF8_STRING\ntext/plain\n" + return result + # If we reach here, an image/png fetch was made — that's the bug. + image_fetch_calls.append(cmd) + result = mock.Mock() + result.returncode = 1 + result.stdout = b"" + return result + + monkeypatch.setattr(subprocess, "run", fake_run) + result = _read_image_from_system_clipboard() + assert result is None + assert image_fetch_calls == [], ( + f"image/png SelectionRequest was sent despite no image in TARGETS: {image_fetch_calls}" + ) + + def test_fetches_image_bytes_when_targets_has_image_png(self, monkeypatch): + """When TARGETS advertises image/png, we must proceed to fetch the bytes.""" + + def fake_run(cmd, **kwargs): + if "TARGETS" in cmd: + result = mock.Mock() + result.returncode = 0 + result.stdout = b"image/png\nUTF8_STRING\n" + return result + if "image/png" in cmd: + result = mock.Mock() + result.returncode = 0 + result.stdout = FAKE_PNG + return result + raise AssertionError(f"Unexpected: {cmd}") + + monkeypatch.setattr(subprocess, "run", fake_run) + result = _read_image_from_system_clipboard() + assert result == FAKE_PNG + + def test_returns_none_when_no_tools_installed(self, monkeypatch): + monkeypatch.setattr(subprocess, "run", lambda *a, **kw: (_ for _ in ()).throw(FileNotFoundError())) + assert _read_image_from_system_clipboard() is None + + +# --------------------------------------------------------------------------- +# _out_tick: rate-limiting on Linux +# --------------------------------------------------------------------------- + + +class TestOutTickRateLimiting: + """Image checks in _out_tick must be throttled on Linux.""" + + @pytest.fixture(autouse=True) + def _force_linux(self, monkeypatch): + monkeypatch.setattr(sys, "platform", "linux") + + def _make_sync(self, tmp_path) -> ClipboardSync: + settings = config.Settings(path=tmp_path / "settings.json") + settings.set("sync_folder", str(tmp_path / "sync")) + (tmp_path / "sync").mkdir() + sync = ClipboardSync(settings) + sync._read_clipboard = lambda: "hello" # type: ignore[method-assign] + sync._write_clipboard = lambda v: True # type: ignore[method-assign] + return sync + + def test_image_check_skipped_when_recently_checked(self, tmp_path): + """If _last_image_check was just set, _read_clipboard_image must not be called.""" + sync = self._make_sync(tmp_path) + image_check_count = {"n": 0} + + def counting_image_read(): + image_check_count["n"] += 1 + return None + + sync._read_clipboard_image = counting_image_read # type: ignore[method-assign] + sync._last_image_check = time.monotonic() # pretend we just checked + + sync._out_tick() + + assert image_check_count["n"] == 0, ( + f"Image check ran despite throttle; called {image_check_count['n']} time(s)" + ) + + def test_image_check_runs_when_interval_elapsed(self, tmp_path, monkeypatch): + """After _LINUX_IMAGE_CHECK_INTERVAL has elapsed, the check must fire.""" + monkeypatch.setattr(clipboard_module, "_LINUX_IMAGE_CHECK_INTERVAL", 0.0) + sync = self._make_sync(tmp_path) + image_check_count = {"n": 0} + + def counting_image_read(): + image_check_count["n"] += 1 + return None + + sync._read_clipboard_image = counting_image_read # type: ignore[method-assign] + sync._last_image_check = 0.0 # never checked + + sync._out_tick() + + assert image_check_count["n"] == 1, ( + f"Expected 1 image check but got {image_check_count['n']}" + ) + + def test_text_sync_still_works_when_image_check_throttled(self, tmp_path, monkeypatch): + """Text clipboard sync must not be blocked by the image throttle.""" + sync = self._make_sync(tmp_path) + sync._last_image_check = time.monotonic() # throttle image check + sync._last_synced = None # force a text sync + write_count = {"n": 0} + + original_write_file = sync._write_file + + def counting_write(text: str) -> None: + write_count["n"] += 1 + # Don't actually write to disk in this unit test + pass + + sync._write_file = counting_write # type: ignore[method-assign] + sync._out_tick() + + assert write_count["n"] == 1, ( + f"Expected text to be written once but got {write_count['n']} write(s)" + ) + + def test_image_check_interval_is_at_least_two_seconds(self): + """Guard against accidentally reducing the interval back to 0.5 s.""" + assert _LINUX_IMAGE_CHECK_INTERVAL >= 2.0, ( + f"_LINUX_IMAGE_CHECK_INTERVAL={_LINUX_IMAGE_CHECK_INTERVAL} is too small; " + "values < 2 s risk reintroducing the paste-freeze bug" + ) + + +# --------------------------------------------------------------------------- +# End-to-end: image sync still works with throttling enabled +# --------------------------------------------------------------------------- + + +@pytest.fixture +def two_sided_linux(tmp_path, monkeypatch): + """Two ClipboardSync instances on a shared folder, image interval shrunk for speed.""" + monkeypatch.setattr(config, "CLIPBOARD_POLL_INTERVAL", POLL) + monkeypatch.setattr(clipboard_module, "_LINUX_IMAGE_CHECK_INTERVAL", POLL * 2) + monkeypatch.setattr(clipboard_module, "Observer", PollingObserver) + monkeypatch.setattr(sys, "platform", "linux") + + sync_folder = tmp_path / "shared_sync" + sync_folder.mkdir() + + settings_a = config.Settings(path=tmp_path / "a_settings.json") + settings_a.set("sync_folder", str(sync_folder)) + settings_b = config.Settings(path=tmp_path / "b_settings.json") + settings_b.set("sync_folder", str(sync_folder)) + + a = ClipboardSync(settings_a) + b = ClipboardSync(settings_b) + + text_a: dict = {"value": ""} + image_a: dict = {"image": None} + text_b: dict = {"value": ""} + image_b: dict = {"image": None} + + a._read_clipboard = lambda: text_a["value"] or None # type: ignore[method-assign] + a._write_clipboard = lambda v: text_a.__setitem__("value", v) or True # type: ignore[method-assign] + a._read_clipboard_image = lambda: image_a["image"] # type: ignore[method-assign] + a._write_clipboard_image = lambda b: image_a.__setitem__("image", b) or True # type: ignore[method-assign] + + b._read_clipboard = lambda: text_b["value"] or None # type: ignore[method-assign] + b._write_clipboard = lambda v: text_b.__setitem__("value", v) or True # type: ignore[method-assign] + b._read_clipboard_image = lambda: image_b["image"] # type: ignore[method-assign] + b._write_clipboard_image = lambda bi: image_b.__setitem__("image", bi) or True # type: ignore[method-assign] + + a.start() + b.start() + + yield a, b, text_a, image_a, text_b, image_b + + a.stop() + b.stop() + + +def test_image_syncs_with_linux_throttle(two_sided_linux) -> None: + """Images must still reach the remote peer even when the Linux throttle is active.""" + _, _, _, img_a, _, img_b = two_sided_linux + img_a["image"] = FAKE_PNG + assert _wait_for(lambda: img_b["image"] == FAKE_PNG, timeout=5.0), ( + f"Image did not sync under Linux throttle; got {img_b['image']!r}" + ) + + +def test_text_syncs_normally_under_linux_throttle(two_sided_linux) -> None: + """Text sync must not be disrupted by the image-check rate limiter.""" + _, _, txt_a, _, txt_b, _ = two_sided_linux + txt_a["value"] = "paste freeze fixed" + assert _wait_for(lambda: txt_b["value"] == "paste freeze fixed"), ( + f"Text did not sync; got {txt_b['value']!r}" + ) + + +def test_image_polling_frequency_bounded_on_linux(two_sided_linux, monkeypatch) -> None: + """The image read must not be called more often than _LINUX_IMAGE_CHECK_INTERVAL.""" + a, _, _, _, _, _ = two_sided_linux + image_check_times: list[float] = [] + original = a._read_clipboard_image + + def tracking_read(): + image_check_times.append(time.monotonic()) + return original() + + a._read_clipboard_image = tracking_read # type: ignore[method-assign] + + observe_duration = POLL * 30 # ~1.5 s at POLL=0.05 + time.sleep(observe_duration) + + if len(image_check_times) >= 2: + gaps = [image_check_times[i + 1] - image_check_times[i] for i in range(len(image_check_times) - 1)] + min_gap = min(gaps) + # Allow 20 % tolerance below the configured interval. + threshold = clipboard_module._LINUX_IMAGE_CHECK_INTERVAL * 0.8 + assert min_gap >= threshold, ( + f"Image check fired too frequently: min gap {min_gap:.3f}s < threshold {threshold:.3f}s. " + f"Gaps: {[f'{g:.3f}' for g in gaps]}" + ) From d27eb0a1d18848536fc6e6d61eabe4906b55aba4 Mon Sep 17 00:00:00 2001 From: offbyonebit Date: Thu, 11 Jun 2026 07:05:45 -0500 Subject: [PATCH 2/6] fix: replace polling with XFixes + in-process clipboard owner to eliminate paste freezes Root cause: the in-process xlib clipboard owner used X.XA_ATOM and X.XA_STRING which do not exist in python-xlib. Every SelectionRequest from a pasting app silently hit AttributeError, replied with property=X.NONE, and caused the browser to retry through its fallback atom list until it timed out -- manifesting as a visible paste freeze. Fixed by using Xatom.ATOM (=4) and Xatom.STRING (=31) from `from Xlib import Xatom`. Also adds: - XFixes-based clipboard owner watcher so the OUT loop wakes only on actual copies instead of polling with xclip every 0.5 s (no more SelectionRequests sent to the browser between copies) - 300 ms debounce after XFixes events to avoid competing with an immediate paste - Event-driven OUT loop with polling fallback for Wayland / no XFixes - CLIPSYNC_NO_XFIXES and CLIPSYNC_NO_XLIB escape hatches for debugging - CLIPSYNC_LOG_LEVEL env var support in configure_logging (previously hardcoded to INFO, silently ignoring --log-level DEBUG) Co-Authored-By: Claude Sonnet 4.6 --- clipsync/clipboard.py | 491 ++++++++++++++++++++++++++++++++++-------- clipsync/config.py | 7 +- 2 files changed, 410 insertions(+), 88 deletions(-) diff --git a/clipsync/clipboard.py b/clipsync/clipboard.py index 5014ef6..e1c8bda 100644 --- a/clipsync/clipboard.py +++ b/clipsync/clipboard.py @@ -2,8 +2,11 @@ Two loops: - OUT: poll the local clipboard every CLIPBOARD_POLL_INTERVAL seconds. - When it changes, write the value to the shared file. + OUT: watch the local clipboard for changes and write them to the shared file. + On Linux/X11, clipboard owner changes are detected via the XFIXES + extension so no SelectionRequests are ever sent to other apps between + copies. On Wayland or when XFIXES is unavailable the loop falls back + to polling every CLIPBOARD_POLL_INTERVAL seconds. IN: watch the shared file with watchdog. When it changes, read it and set the local clipboard. @@ -22,6 +25,7 @@ import io import logging +import os import queue import subprocess import sys @@ -45,42 +49,284 @@ _PNG_HEADER = b"\x89PNG\r\n\x1a\n" -# Minimum seconds between image-clipboard polls on Linux. Checking for images -# requires running xclip/wl-paste which sends X11 SelectionRequests to the -# clipboard owner (typically a browser). Browsers service these on their main -# thread, so polling at the same 0.5 s interval as text caused paste freezes. -_LINUX_IMAGE_CHECK_INTERVAL = 2.0 +# Sentinel pushed onto the XFixes queue by stop() to unblock the OUT loop. +_STOP_SENTINEL = None -def _normalize_newlines(s: str) -> str: - """Collapse CRLF/CR to LF so Windows's clipboard normalization does not - look like a real change to the OUT loop after a remote update.""" - return s.replace("\r\n", "\n").replace("\r", "\n") +def _try_start_xfixes_watcher() -> "queue.SimpleQueue[object] | None": + """Start an X11 XFixes clipboard-owner watcher. + + Returns a SimpleQueue that receives a True value each time the CLIPBOARD + selection owner changes (i.e. someone copies something). Returns None if + the XFixes extension is unavailable or the display cannot be opened + (Wayland, headless, missing python-xlib, etc.). + + Using XFixes means the OUT loop is woken only on actual clipboard changes + rather than polling with xclip every 0.5 s. Polling sends X11 + SelectionRequests to the clipboard owner (typically a browser) on every + tick; browsers service these on their main thread, which caused paste + freezes. With XFixes, clipsync never sends a SelectionRequest unless the + user has actually copied something. + """ + try: + from Xlib import display # type: ignore[import] + from Xlib.protocol import rq # type: ignore[import] + except ImportError: + return None + + try: + _d = display.Display() + _ext = _d.query_extension("XFIXES") + _d.close() + if not _ext.present: + return None + except Exception: + return None + + # Inline minimal XFixes protocol definitions -- python-xlib 0.15 doesn't + # ship an xfixes module, so we define only what we need here. + class _QueryVersion(rq.ReplyRequest): # type: ignore[misc] + _request = rq.Struct( + rq.Card8("opcode"), rq.Opcode(0), rq.RequestLength(), + rq.Card32("client_major"), rq.Card32("client_minor"), + ) + _reply = rq.Struct( + rq.ReplyCode(), rq.Pad(1), rq.Card16("sequence_number"), + rq.Card32("length"), rq.Card32("major_version"), + rq.Card32("minor_version"), rq.Pad(16), + ) + class _SelectSelectionInput(rq.Request): # type: ignore[misc] + _request = rq.Struct( + rq.Card8("opcode"), rq.Opcode(2), rq.RequestLength(), + rq.Window("window"), rq.Card32("selection"), rq.Card32("event_mask"), + ) + + class _SelectionNotify(rq.Event): # type: ignore[misc] + _code = None + _fields = rq.Struct( + rq.Card8("type"), rq.Card8("subtype"), rq.Card16("sequence_number"), + rq.Window("window"), rq.Card32("selection"), rq.Card32("owner"), + rq.Card32("selection_timestamp"), rq.Card32("timestamp"), + ) + + notify_q: queue.SimpleQueue[object] = queue.SimpleQueue() + + def _watch() -> None: + try: + d = display.Display() + ext = d.query_extension("XFIXES") + if not ext.present: + return + d.display.extension_major_opcodes["XFIXES"] = ext.major_opcode + _SelectionNotify._code = ext.first_event + d.display.add_extension_event(ext.first_event, _SelectionNotify) + _QueryVersion( + display=d.display, opcode=ext.major_opcode, + client_major=5, client_minor=0, + ) + d.sync() + root = d.screen().root + clipboard_atom = d.intern_atom("CLIPBOARD") + _SelectSelectionInput( + display=d.display, opcode=ext.major_opcode, + window=root, selection=clipboard_atom, + event_mask=1, # SelectionSetOwnerMask + ) + d.flush() + log.debug("XFixes clipboard watcher active (event_base=%d)", ext.first_event) + while True: + e = d.next_event() + if e.type == ext.first_event: + notify_q.put(True) + except Exception: + log.debug("XFixes watcher stopped", exc_info=True) + + t = threading.Thread(target=_watch, name="clipsync-xfixes", daemon=True) + t.start() + return notify_q + + +def _try_start_xlib_clipboard_owner() -> "_XlibClipboardOwner | None": + """Try to create an in-process X11 clipboard owner using python-xlib. -def _linux_clipboard_has_image() -> bool: - """Return True only when the Linux clipboard advertises image/png via TARGETS. + Returns None on Wayland, missing python-xlib, or any startup error. - This is a cheap pre-check: the clipboard owner (e.g. a browser) responds to - TARGETS requests immediately on its main thread without serialising any data. - By gating the full image/png fetch on this check we avoid sending expensive - SelectionRequests when the clipboard holds text, which was the root cause of - browsers freezing on paste. + Compared to spawning xclip subprocesses, this approach: + - Has zero startup latency (no process fork/exec) + - Has no ownership-transition gap (we own the selection immediately) + - Responds to SelectionRequests in microseconds (single round trip) """ - for cmd in ( - ["xclip", "-selection", "clipboard", "-t", "TARGETS", "-o"], - ["wl-paste", "--list-types"], - ): + try: + from Xlib import display # type: ignore[import] + except ImportError: + return None + try: + return _XlibClipboardOwner() + except Exception: + log.debug("xlib clipboard owner init failed", exc_info=True) + return None + + +class _XlibClipboardOwner: + """In-process X11 CLIPBOARD selection owner. + + Owns the CLIPBOARD selection and serves SelectionRequests entirely + within the clipsync process. A select()-based event loop handles both + X11 events and a self-pipe used by the IN thread to signal new content. + """ + + def __init__(self) -> None: + import os + + from Xlib import X, display # type: ignore[import] + + from Xlib import Xatom # type: ignore[import] + + self._X = X + self._d = display.Display() + root = self._d.screen().root + self._win = root.create_window(0, 0, 1, 1, 0, 0) + self._CLIPBOARD = self._d.intern_atom("CLIPBOARD") + self._UTF8 = self._d.intern_atom("UTF8_STRING") + self._COMPOUND_TEXT = self._d.intern_atom("COMPOUND_TEXT") + self._TARGETS = self._d.intern_atom("TARGETS") + self._XA_ATOM = Xatom.ATOM # type for lists of atoms (= 4) + self._XA_STRING = Xatom.STRING # plain ASCII/Latin-1 string type (= 31) + self._d.flush() + + self._content: str | None = None + self._content_lock = threading.Lock() + self._stopped = False + + # Self-pipe: writing a byte wakes the event loop. + self._pipe_r, self._pipe_w = os.pipe() + + self._thread = threading.Thread( + target=self._event_loop, name="clipsync-xlib-owner", daemon=True + ) + self._thread.start() + + def set(self, text: str) -> None: + """Store `text` and signal the event loop to take CLIPBOARD ownership.""" + import os + + with self._content_lock: + self._content = text + os.write(self._pipe_w, b"\x01") + + def close(self) -> None: + """Signal the event loop to exit and release clipboard ownership.""" + import os + + self._stopped = True try: - result = subprocess.run(cmd, capture_output=True, timeout=1) - except FileNotFoundError: - continue # Tool not installed; try the next one. - except (subprocess.TimeoutExpired, OSError): - continue - # Once a tool responds, treat its answer as authoritative for this - # display-server type — no need to consult the other tool. - return result.returncode == 0 and b"image/png" in result.stdout - return False + os.write(self._pipe_w, b"\xff") + except OSError: + pass + + def _event_loop(self) -> None: + import os + import select + + X = self._X + x_fd = self._d.fileno() + + try: + while not self._stopped: + rlist, _, _ = select.select([x_fd, self._pipe_r], [], [], 10.0) + + if self._pipe_r in rlist: + data = os.read(self._pipe_r, 4096) + if b"\xff" in data: + break # close signal + with self._content_lock: + content = self._content + if content is not None: + self._win.set_selection_owner(self._CLIPBOARD, X.CurrentTime) + self._d.flush() + log.debug( + "xlib clipboard: took CLIPBOARD ownership (%d chars)", + len(content), + ) + + while self._d.pending_events(): + try: + event = self._d.next_event() + self._handle_event(event) + except Exception: + log.debug("xlib clipboard: error processing event", exc_info=True) + except Exception: + log.debug("xlib clipboard event loop stopped", exc_info=True) + finally: + try: + self._d.close() + except Exception: + pass + try: + os.close(self._pipe_r) + os.close(self._pipe_w) + except OSError: + pass + + def _handle_event(self, event) -> None: + X = self._X + if event.type == X.SelectionRequest: + self._serve_request(event) + elif event.type == X.SelectionClear: + with self._content_lock: + self._content = None + log.debug("xlib clipboard: lost CLIPBOARD ownership (SelectionClear)") + + def _serve_request(self, req) -> None: + from Xlib.protocol.event import SelectionNotify # type: ignore[import] + + X = self._X + with self._content_lock: + content = self._content + + target = req.target + # Prefer req.property; fall back to req.target when property is None. + prop = req.property if req.property != X.NONE else req.target + + def reply(p: int) -> None: + notify = SelectionNotify( + time=req.time, + requestor=req.requestor, + selection=req.selection, + target=target, + property=p, + ) + req.requestor.send_event(notify) + self._d.flush() + + try: + if target == self._TARGETS: + atoms = [self._TARGETS, self._UTF8, self._XA_STRING, self._COMPOUND_TEXT] + req.requestor.change_property(prop, self._XA_ATOM, 32, atoms) + reply(prop) + elif target in (self._UTF8, self._XA_STRING, self._COMPOUND_TEXT): + if content is None: + reply(X.NONE) + return + data = content.encode("utf-8") + atom_type = self._UTF8 if target == self._UTF8 else self._XA_STRING + req.requestor.change_property(prop, atom_type, 8, data) + reply(prop) + else: + reply(X.NONE) + except Exception: + log.debug("xlib clipboard: error serving SelectionRequest", exc_info=True) + try: + reply(X.NONE) + except Exception: + pass + + +def _normalize_newlines(s: str) -> str: + """Collapse CRLF/CR to LF so Windows's clipboard normalization does not + look like a real change to the OUT loop after a remote update.""" + return s.replace("\r\n", "\n").replace("\r", "\n") def _read_image_from_system_clipboard() -> bytes | None: @@ -98,10 +344,19 @@ def _read_image_from_system_clipboard() -> bytes | None: except Exception: return None # Linux: check TARGETS first so we never send an image/png SelectionRequest - # to the clipboard owner when only text is present. Without this gate, - # xclip hammered the browser's main thread every 0.5 s, causing paste freezes. - if not _linux_clipboard_has_image(): - return None + # to the clipboard owner when only text is present. Without this guard, + # xclip would request image data even when the clipboard holds text. + for targets_cmd in ( + ["xclip", "-selection", "clipboard", "-t", "TARGETS", "-o"], + ["wl-paste", "--list-types"], + ): + try: + res = subprocess.run(targets_cmd, capture_output=True, timeout=1) + except (FileNotFoundError, subprocess.TimeoutExpired, OSError): + continue + if res.returncode != 0 or b"image/png" not in res.stdout: + return None + break # Some xclip versions return text content with exit 0 even when asked for # image/png and no image is on the clipboard. Guard with a PNG magic-byte # check so we never mistake text bytes for image data. @@ -187,7 +442,8 @@ def __init__(self, settings: config.Settings) -> None: self._last_read_error: str | None = None self._last_write_error: str | None = None self._last_decrypt_error: str | None = None - self._last_image_check: float = 0.0 + self._xfixes_queue: "queue.SimpleQueue[object] | None" = None + self._clipboard_owner: "_XlibClipboardOwner | None" = None self._history = ClipboardHistory(settings) @property @@ -203,6 +459,19 @@ def clipboard_image_file(self) -> Path: def start(self) -> None: self._stop.clear() self._seed_from_file() + if sys.platform not in ("win32", "darwin"): + _no_xfixes = os.environ.get("CLIPSYNC_NO_XFIXES") + _no_xlib = os.environ.get("CLIPSYNC_NO_XLIB") + self._xfixes_queue = _try_start_xfixes_watcher() if not _no_xfixes else None + if self._xfixes_queue is None: + log.debug("XFixes unavailable%s; falling back to clipboard polling", + " (CLIPSYNC_NO_XFIXES set)" if _no_xfixes else "") + if not _no_xlib: + self._clipboard_owner = _try_start_xlib_clipboard_owner() + if self._clipboard_owner is None: + log.debug("xlib clipboard owner unavailable; using pyperclip for writes") + else: + log.info("xlib in-process clipboard owner active") self._poll_thread = threading.Thread(target=self._out_loop, name="clipsync-out", daemon=True) self._poll_thread.start() self._in_thread = threading.Thread(target=self._in_loop, name="clipsync-in", daemon=True) @@ -221,6 +490,13 @@ def stop(self) -> None: self._observer = None # Unblock _in_loop which may be waiting on the queue self._in_queue.put("") + # Unblock _out_loop if it is waiting on the XFixes queue + if self._xfixes_queue is not None: + self._xfixes_queue.put(_STOP_SENTINEL) + # Release clipboard ownership held by in-process owner. + if self._clipboard_owner is not None: + self._clipboard_owner.close() + self._clipboard_owner = None if self._in_thread and self._in_thread.is_alive(): self._in_thread.join(timeout=3) if self._poll_thread and self._poll_thread.is_alive(): @@ -388,8 +664,27 @@ def _read_clipboard(self) -> str | None: return _normalize_newlines(value) def _write_clipboard(self, value: str) -> bool: + if self._clipboard_owner is not None: + try: + t0 = time.monotonic() + self._clipboard_owner.set(value) + elapsed_ms = (time.monotonic() - t0) * 1000 + log.debug("xlib clipboard set in %.2f ms (%d chars)", elapsed_ms, len(value)) + if self._last_write_error is not None: + log.info("Clipboard write recovered") + self._last_write_error = None + return True + except Exception as exc: + msg = f"xlib owner: {type(exc).__name__}: {exc}" + if msg != self._last_write_error: + log.warning("Clipboard write failed (%s); falling back to pyperclip", msg) + self._last_write_error = msg + # fall through to pyperclip try: + t0 = time.monotonic() pyperclip.copy(value) + elapsed_ms = (time.monotonic() - t0) * 1000 + log.debug("clipboard write (pyperclip) took %.1f ms (%d chars)", elapsed_ms, len(value)) except Exception as exc: msg = f"{type(exc).__name__}: {exc}" if msg != self._last_write_error: @@ -416,18 +711,65 @@ def _write_clipboard_image(self, png_bytes: bytes) -> bool: return False def _out_loop(self) -> None: - _heartbeat_counter = 0 + _last_heartbeat = time.monotonic() + _HEARTBEAT_INTERVAL = 6.0 + + # Initial tick captures whatever is on the clipboard at startup. + try: + if not self._is_paused(): + self._out_tick() + except Exception: + log.exception("Error in OUT loop (initial tick)") + while not self._stop.is_set(): - try: + if self._xfixes_queue is not None: + # Event-driven path: block until clipboard owner changes (or stop). + try: + val = self._xfixes_queue.get(timeout=_HEARTBEAT_INTERVAL) + if val is _STOP_SENTINEL: + break + # Drain any extra events that arrived while we were busy. + while True: + try: + self._xfixes_queue.get_nowait() + except queue.Empty: + break + except queue.Empty: + pass # heartbeat timeout -- fall through to log only + else: + # Brief pause before reading: the XFixes event fires the + # instant the user copies, so if they immediately paste in + # the same browser window, our xclip read would compete + # with the browser serving its own paste on the same thread. + # 300 ms is imperceptible for sync but covers the typical + # copy→paste gesture before we send any SelectionRequest. + if self._stop.wait(0.3): + break + # Clipboard actually changed: read and sync. + if not self._stop.is_set() and not self._is_paused(): + # Drain any events that arrived during the debounce. + while True: + try: + self._xfixes_queue.get_nowait() + except queue.Empty: + break + try: + self._out_tick() + except Exception: + log.exception("Error in OUT loop") + else: + # Polling fallback (Wayland / no XFixes). + if self._stop.wait(config.CLIPBOARD_POLL_INTERVAL): + break if not self._is_paused(): - self._out_tick() - except Exception: - log.exception("Error in OUT loop") - if self._stop.wait(config.CLIPBOARD_POLL_INTERVAL): - break - _heartbeat_counter += 1 - if _heartbeat_counter >= 12: - _heartbeat_counter = 0 + try: + self._out_tick() + except Exception: + log.exception("Error in OUT loop") + + now = time.monotonic() + if now - _last_heartbeat >= _HEARTBEAT_INTERVAL: + _last_heartbeat = now with self._lock: last = self._last_synced log.debug( @@ -439,30 +781,18 @@ def _out_loop(self) -> None: def _out_tick(self) -> None: # Images take priority: if the clipboard has an image, sync it. - # On Linux, throttle image checks to _LINUX_IMAGE_CHECK_INTERVAL seconds - # (vs the normal 0.5 s text-poll rate). Even with the TARGETS pre-check - # inside _read_clipboard_image, every image-check tick still spawns an - # xclip subprocess that sends an X11 SelectionRequest to the clipboard - # owner; keeping this at 0.5 s was enough to freeze browsers on paste. - now = time.monotonic() - _do_image_check = sys.platform in ("win32", "darwin") or ( - now - self._last_image_check >= _LINUX_IMAGE_CHECK_INTERVAL - ) - if _do_image_check: - if sys.platform not in ("win32", "darwin"): - self._last_image_check = now - image = self._read_clipboard_image() - if image is not None: - with self._lock: - if image == self._last_synced: - return - self._last_synced = image - try: - self._write_image_file(image) - log.info("OUT [%s]: %d bytes image written", _HOSTNAME, len(image)) - except OSError: - log.exception("OUT [%s]: Failed to write image file", _HOSTNAME) - return + image = self._read_clipboard_image() + if image is not None: + with self._lock: + if image == self._last_synced: + return + self._last_synced = image + try: + self._write_image_file(image) + log.info("OUT [%s]: %d bytes image written", _HOSTNAME, len(image)) + except OSError: + log.exception("OUT [%s]: Failed to write image file", _HOSTNAME) + return current = self._read_clipboard() if current is None or current == "": @@ -538,15 +868,11 @@ def _on_text_file_changed(self) -> None: if content == self._last_synced: log.debug("IN [%s]: file changed but content already synced (%d chars)", _HOSTNAME, len(content)) return - current = self._read_clipboard() - if current == content: - with self._lock: - self._last_synced = content - log.debug("IN [%s]: file change was self-originated (%d chars)", _HOSTNAME, len(content)) - return + # Update _last_synced before the write so that the XFixes event + # triggered by pyperclip.copy() below sees no change in the OUT + # loop and does not re-read the clipboard. + self._last_synced = content if self._write_clipboard(content): - with self._lock: - self._last_synced = content log.info("IN [%s]: %d chars applied to clipboard", _HOSTNAME, len(content)) self._history.add_entry(content, "remote") @@ -558,15 +884,8 @@ def _on_image_file_changed(self) -> None: if image == self._last_synced: log.debug("IN [%s]: image file changed but already synced (%d bytes)", _HOSTNAME, len(image)) return - current = self._read_clipboard_image() - if current == image: - with self._lock: - self._last_synced = image - log.debug("IN [%s]: image file change was self-originated (%d bytes)", _HOSTNAME, len(image)) - return + self._last_synced = image if self._write_clipboard_image(image): - with self._lock: - self._last_synced = image log.info("IN [%s]: %d bytes image applied to clipboard", _HOSTNAME, len(image)) diff --git a/clipsync/config.py b/clipsync/config.py index dc9e764..4ff96ed 100644 --- a/clipsync/config.py +++ b/clipsync/config.py @@ -197,13 +197,16 @@ def set_file_permissions(path: Path) -> None: pass -def configure_logging() -> None: +def configure_logging(level: int = logging.INFO) -> None: """Wire up root logger to write to both the log file and stderr.""" ensure_directories() root = logging.getLogger() if getattr(root, "_clipsync_configured", False): return - root.setLevel(logging.INFO) + # Honour CLIPSYNC_LOG_LEVEL env var (DEBUG, INFO, WARNING, ERROR). + if "CLIPSYNC_LOG_LEVEL" in os.environ: + level = getattr(logging, os.environ["CLIPSYNC_LOG_LEVEL"].upper(), level) + root.setLevel(level) fmt = logging.Formatter( "%(asctime)s [%(levelname)s] %(name)s: %(message)s", datefmt="%Y-%m-%d %H:%M:%S", From 5a55f92c8533cd6765b0229b547d37c490c4d765 Mon Sep 17 00:00:00 2001 From: offbyonebit Date: Thu, 11 Jun 2026 07:12:47 -0500 Subject: [PATCH 3/6] refactor: simplify XFixes watcher, remove dead _stopped flag, trim hot-path overhead - _STOP_SENTINEL = object() instead of None -- prevents accidental None on queue silently killing the OUT loop - Pass _opcode/_first_event from the probe into _watch() so query_extension is called once instead of twice (one fewer Display connection + round-trip) - _SelectionNotify._code set at class definition time (was re-set inside _watch) - Remove _stopped bool from _XlibClipboardOwner -- the \xff pipe signal already wakes and exits the event loop immediately; _stopped was redundant and implied a 10-s tail latency on shutdown that never actually applied - Remove self._d.flush() after intern_atom calls -- intern_atom is synchronous (waits for reply), so nothing is buffered to flush - Remove redundant `import os` inside set/close/_event_loop (module-level import) - Remove pre-debounce queue drain in _out_loop -- it ran before the 300 ms wait, so nothing useful had accumulated yet; the post-debounce drain is the one that matters - Drop time.monotonic() timing from _write_clipboard hot path -- two syscalls per write whose result was discarded at default INFO log level - configure_logging: remove unused `level` parameter; hard-code INFO default and apply CLIPSYNC_LOG_LEVEL override directly Co-Authored-By: Claude Sonnet 4.6 --- clipsync/clipboard.py | 54 +++++++++++++------------------------------ clipsync/config.py | 5 ++-- 2 files changed, 19 insertions(+), 40 deletions(-) diff --git a/clipsync/clipboard.py b/clipsync/clipboard.py index e1c8bda..a065807 100644 --- a/clipsync/clipboard.py +++ b/clipsync/clipboard.py @@ -50,7 +50,7 @@ _PNG_HEADER = b"\x89PNG\r\n\x1a\n" # Sentinel pushed onto the XFixes queue by stop() to unblock the OUT loop. -_STOP_SENTINEL = None +_STOP_SENTINEL = object() def _try_start_xfixes_watcher() -> "queue.SimpleQueue[object] | None": @@ -74,9 +74,13 @@ def _try_start_xfixes_watcher() -> "queue.SimpleQueue[object] | None": except ImportError: return None + # Probe: resolve extension opcode/event numbers once; pass them into the + # watcher thread so it can skip a redundant query_extension round-trip. try: _d = display.Display() _ext = _d.query_extension("XFIXES") + _opcode = _ext.major_opcode + _first_event = _ext.first_event _d.close() if not _ext.present: return None @@ -103,7 +107,7 @@ class _SelectSelectionInput(rq.Request): # type: ignore[misc] ) class _SelectionNotify(rq.Event): # type: ignore[misc] - _code = None + _code = _first_event _fields = rq.Struct( rq.Card8("type"), rq.Card8("subtype"), rq.Card16("sequence_number"), rq.Window("window"), rq.Card32("selection"), rq.Card32("owner"), @@ -115,29 +119,25 @@ class _SelectionNotify(rq.Event): # type: ignore[misc] def _watch() -> None: try: d = display.Display() - ext = d.query_extension("XFIXES") - if not ext.present: - return - d.display.extension_major_opcodes["XFIXES"] = ext.major_opcode - _SelectionNotify._code = ext.first_event - d.display.add_extension_event(ext.first_event, _SelectionNotify) + d.display.extension_major_opcodes["XFIXES"] = _opcode + d.display.add_extension_event(_first_event, _SelectionNotify) _QueryVersion( - display=d.display, opcode=ext.major_opcode, + display=d.display, opcode=_opcode, client_major=5, client_minor=0, ) d.sync() root = d.screen().root clipboard_atom = d.intern_atom("CLIPBOARD") _SelectSelectionInput( - display=d.display, opcode=ext.major_opcode, + display=d.display, opcode=_opcode, window=root, selection=clipboard_atom, event_mask=1, # SelectionSetOwnerMask ) d.flush() - log.debug("XFixes clipboard watcher active (event_base=%d)", ext.first_event) + log.debug("XFixes clipboard watcher active (event_base=%d)", _first_event) while True: e = d.next_event() - if e.type == ext.first_event: + if e.type == _first_event: notify_q.put(True) except Exception: log.debug("XFixes watcher stopped", exc_info=True) @@ -177,11 +177,7 @@ class _XlibClipboardOwner: """ def __init__(self) -> None: - import os - - from Xlib import X, display # type: ignore[import] - - from Xlib import Xatom # type: ignore[import] + from Xlib import X, Xatom, display # type: ignore[import] self._X = X self._d = display.Display() @@ -193,11 +189,9 @@ def __init__(self) -> None: self._TARGETS = self._d.intern_atom("TARGETS") self._XA_ATOM = Xatom.ATOM # type for lists of atoms (= 4) self._XA_STRING = Xatom.STRING # plain ASCII/Latin-1 string type (= 31) - self._d.flush() self._content: str | None = None self._content_lock = threading.Lock() - self._stopped = False # Self-pipe: writing a byte wakes the event loop. self._pipe_r, self._pipe_w = os.pipe() @@ -209,31 +203,25 @@ def __init__(self) -> None: def set(self, text: str) -> None: """Store `text` and signal the event loop to take CLIPBOARD ownership.""" - import os - with self._content_lock: self._content = text os.write(self._pipe_w, b"\x01") def close(self) -> None: """Signal the event loop to exit and release clipboard ownership.""" - import os - - self._stopped = True try: os.write(self._pipe_w, b"\xff") except OSError: pass def _event_loop(self) -> None: - import os import select X = self._X x_fd = self._d.fileno() try: - while not self._stopped: + while True: rlist, _, _ = select.select([x_fd, self._pipe_r], [], [], 10.0) if self._pipe_r in rlist: @@ -666,10 +654,8 @@ def _read_clipboard(self) -> str | None: def _write_clipboard(self, value: str) -> bool: if self._clipboard_owner is not None: try: - t0 = time.monotonic() self._clipboard_owner.set(value) - elapsed_ms = (time.monotonic() - t0) * 1000 - log.debug("xlib clipboard set in %.2f ms (%d chars)", elapsed_ms, len(value)) + log.debug("xlib clipboard set (%d chars)", len(value)) if self._last_write_error is not None: log.info("Clipboard write recovered") self._last_write_error = None @@ -681,10 +667,8 @@ def _write_clipboard(self, value: str) -> bool: self._last_write_error = msg # fall through to pyperclip try: - t0 = time.monotonic() pyperclip.copy(value) - elapsed_ms = (time.monotonic() - t0) * 1000 - log.debug("clipboard write (pyperclip) took %.1f ms (%d chars)", elapsed_ms, len(value)) + log.debug("clipboard write (pyperclip) (%d chars)", len(value)) except Exception as exc: msg = f"{type(exc).__name__}: {exc}" if msg != self._last_write_error: @@ -728,12 +712,6 @@ def _out_loop(self) -> None: val = self._xfixes_queue.get(timeout=_HEARTBEAT_INTERVAL) if val is _STOP_SENTINEL: break - # Drain any extra events that arrived while we were busy. - while True: - try: - self._xfixes_queue.get_nowait() - except queue.Empty: - break except queue.Empty: pass # heartbeat timeout -- fall through to log only else: diff --git a/clipsync/config.py b/clipsync/config.py index 4ff96ed..d4f1745 100644 --- a/clipsync/config.py +++ b/clipsync/config.py @@ -197,15 +197,16 @@ def set_file_permissions(path: Path) -> None: pass -def configure_logging(level: int = logging.INFO) -> None: +def configure_logging() -> None: """Wire up root logger to write to both the log file and stderr.""" ensure_directories() root = logging.getLogger() if getattr(root, "_clipsync_configured", False): return + level = logging.INFO # Honour CLIPSYNC_LOG_LEVEL env var (DEBUG, INFO, WARNING, ERROR). if "CLIPSYNC_LOG_LEVEL" in os.environ: - level = getattr(logging, os.environ["CLIPSYNC_LOG_LEVEL"].upper(), level) + level = getattr(logging, os.environ["CLIPSYNC_LOG_LEVEL"].upper(), logging.INFO) root.setLevel(level) fmt = logging.Formatter( "%(asctime)s [%(levelname)s] %(name)s: %(message)s", From 3089e3d382d5357a8e066f60f076bdf0016c0f00 Mon Sep 17 00:00:00 2001 From: offbyonebit Date: Thu, 11 Jun 2026 07:35:38 -0500 Subject: [PATCH 4/6] =?UTF-8?q?feat:=20add=20Send=20File=E2=80=A6=20tray?= =?UTF-8?q?=20menu=20item=20for=20file=20transfer=20via=20Syncthing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Files are copied into /files// where Syncthing picks them up automatically. Incoming files from peer hosts are saved to ~/Downloads with collision-safe naming and a tray notification. The file picker opens a native OS dialog in a lightweight subprocess. Co-Authored-By: Claude Sonnet 4.6 --- clipsync/file_transfer.py | 114 ++++++++++++++++++++++++++++++++++++++ clipsync/main.py | 42 ++++++++++++++ clipsync/ui.py | 19 ++++++- 3 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 clipsync/file_transfer.py diff --git a/clipsync/file_transfer.py b/clipsync/file_transfer.py new file mode 100644 index 0000000..bc4c164 --- /dev/null +++ b/clipsync/file_transfer.py @@ -0,0 +1,114 @@ +"""File transfer: send local files to connected devices and receive them. + +Sending: + copy source file into /files//_ + Syncthing picks it up and replicates it automatically. + +Receiving: + A watchdog observer watches /files/ recursively. + Any new file under a subdirectory other than the local host's is a + remote file. on_received(path, sender_hostname) is called once per file. +""" + +from __future__ import annotations + +import logging +import shutil +import time +from collections.abc import Callable +from pathlib import Path + +from watchdog.events import FileSystemEvent, FileSystemEventHandler +from watchdog.observers import Observer + +from . import config +from .debug import _safe_hostname + +log = logging.getLogger(__name__) +_HOSTNAME = _safe_hostname() + + +class FileTransfer: + """Send files to the sync folder and notify on incoming files from peers.""" + + def __init__( + self, + settings: config.Settings, + on_received: Callable[[Path, str], None], + ) -> None: + self._settings = settings + self._on_received = on_received + self._observer: Observer | None = None + + @property + def files_dir(self) -> Path: + folder = Path(self._settings.get("sync_folder") or config.SYNC_FOLDER) + return folder / "files" + + def send(self, source: Path) -> Path: + """Copy *source* into the shared folder under this host's subdirectory. + + Returns the destination path. Raises OSError on failure. + """ + dest_dir = self.files_dir / _HOSTNAME + dest_dir.mkdir(parents=True, exist_ok=True) + timestamp = time.strftime("%Y%m%d_%H%M%S") + dest = dest_dir / f"{timestamp}_{source.name}" + shutil.copy2(source, dest) + log.info("FILE OUT [%s]: %s (%d bytes)", _HOSTNAME, source.name, source.stat().st_size) + return dest + + def start(self) -> None: + self.files_dir.mkdir(parents=True, exist_ok=True) + handler = _FileReceiveHandler(on_received=self._on_received) + observer = Observer() + observer.schedule(handler, str(self.files_dir), recursive=True) + observer.start() + self._observer = observer + log.debug("File transfer watcher started (watching %s)", self.files_dir) + + def stop(self) -> None: + if self._observer is not None: + self._observer.stop() + self._observer.join(timeout=3) + self._observer = None + + +class _FileReceiveHandler(FileSystemEventHandler): + """Watch the files/ tree and fire on_received for files from remote hosts.""" + + def __init__(self, on_received: Callable[[Path, str], None]) -> None: + super().__init__() + self._on_received = on_received + # Guard against duplicate events (watchdog can fire multiple times for + # a single file, e.g. created + modified during Syncthing's atomic write). + self._seen: set[str] = set() + + def _handle(self, path: Path) -> None: + # Expected layout: files// + # Ignore files directly under files/ (no host subdirectory) and our own. + sender = path.parent.name + if not sender or sender == _HOSTNAME: + return + # Ignore Syncthing temp files (.syncthing.*.tmp pattern). + if path.name.startswith(".syncthing.") and path.name.endswith(".tmp"): + return + key = str(path) + if key in self._seen: + return + self._seen.add(key) + log.info("FILE IN [%s]: %s from %s", _HOSTNAME, path.name, sender) + try: + self._on_received(path, sender) + except Exception: + log.exception("Error in file receive handler") + + def on_created(self, event: FileSystemEvent) -> None: + if not event.is_directory: + self._handle(Path(event.src_path)) + + def on_moved(self, event: FileSystemEvent) -> None: + # Syncthing uses atomic rename: .syncthing.*.tmp → final name. + dest = getattr(event, "dest_path", "") + if dest and not event.is_directory: + self._handle(Path(dest)) diff --git a/clipsync/main.py b/clipsync/main.py index 017399e..1a41422 100644 --- a/clipsync/main.py +++ b/clipsync/main.py @@ -18,6 +18,7 @@ import logging import platform +import shutil import signal import sys import threading @@ -28,6 +29,7 @@ from . import config, update from .clipboard import ClipboardSync +from .file_transfer import FileTransfer from .debug import LogMirror from .pairing import PendingDeviceWatcher, accept_pending_device from .single_instance import AlreadyRunning, SingleInstance @@ -137,6 +139,7 @@ def __init__(self) -> None: self.settings = config.Settings() self.syncthing = SyncthingService(self.settings) self.clipboard: ClipboardSync | None = None + self.file_transfer: FileTransfer | None = None self.log_mirror: LogMirror | None = None self.watcher: PendingDeviceWatcher | None = None self.ui = UIController(on_event=self._handle_ui_event) @@ -156,6 +159,9 @@ def start(self) -> None: self.clipboard = ClipboardSync(self.settings) self.clipboard.start() + self.file_transfer = FileTransfer(self.settings, on_received=self._on_file_received) + self.file_transfer.start() + self.log_mirror = LogMirror(self.settings) self.log_mirror.start() @@ -210,6 +216,7 @@ def _run_tray(self) -> None: visible=lambda _item: self._pending_count() > 0, ), pystray.MenuItem("Clipboard History", lambda _i, _it: self.ui.open("history")), + pystray.MenuItem("Send File…", lambda _i, _it: self.ui.open("file_picker")), pystray.MenuItem("Add Device", lambda _i, _it: self.ui.open("tabbed:pair")), pystray.MenuItem("Connected Devices", lambda _i, _it: self.ui.open("tabbed:devices")), pystray.MenuItem( @@ -379,6 +386,14 @@ def _handle_ui_event(self, evt: dict) -> None: device_id = evt.get("device_id") if isinstance(device_id, str): self._reject_device(device_id) + elif kind == "file_selected": + path = evt.get("path") + if isinstance(path, str) and self.file_transfer is not None: + threading.Thread( + target=self._send_file_worker, + args=(Path(path),), + daemon=True, + ).start() else: log.debug("Unhandled UI event: %s", evt) @@ -403,6 +418,31 @@ def _on_folder_changed(self, new_path: str) -> None: self.clipboard = ClipboardSync(self.settings) self.clipboard.start() + def _send_file_worker(self, source: Path) -> None: + if self.file_transfer is None: + return + try: + dest = self.file_transfer.send(source) + self._notify("File sent", f"{source.name} ({dest.stat().st_size // 1024} KB)") + except Exception as exc: + log.exception("Failed to send file %s", source) + self._notify("File send failed", str(exc)) + + def _on_file_received(self, path: Path, sender: str) -> None: + downloads = Path.home() / "Downloads" + downloads.mkdir(parents=True, exist_ok=True) + dest = downloads / path.name + if dest.exists(): + stem = path.stem + suffix = path.suffix + i = 1 + while dest.exists(): + dest = downloads / f"{stem}_{i}{suffix}" + i += 1 + shutil.copy2(path, dest) + log.info("Saved received file to %s", dest) + self._notify(f"File from {sender}", f"Saved to ~/Downloads/{dest.name}") + # Shutdown --------------------------------------------------------------- def _shutdown(self) -> None: @@ -418,6 +458,8 @@ def _shutdown(self) -> None: self.watcher.stop() if self.log_mirror is not None: self.log_mirror.stop() + if self.file_transfer is not None: + self.file_transfer.stop() if self.clipboard is not None: self.clipboard.stop() try: diff --git a/clipsync/ui.py b/clipsync/ui.py index 9c838a5..24d6696 100644 --- a/clipsync/ui.py +++ b/clipsync/ui.py @@ -27,7 +27,7 @@ log = logging.getLogger(__name__) -_WINDOWS = ("pairing", "devices", "settings", "logs", "incoming", "tabbed", "history") +_WINDOWS = ("pairing", "devices", "settings", "logs", "incoming", "tabbed", "history", "file_picker") def _center_window(window: ctk.CTkToplevel | ctk.CTk, width: int, height: int) -> None: @@ -1621,6 +1621,23 @@ def _quit() -> None: IncomingWindow(root, app, on_close=_quit) elif kind == "history": HistoryWindow(root, app, on_close=_quit) + elif kind == "file_picker": + # Lightweight path: open a native file dialog, emit the result, done. + # No CTk window needed -- just a hidden Tk root to host the dialog. + import tkinter as tk + from tkinter import filedialog + + root.destroy() # discard the CTk root, use a plain Tk one + tk_root = tk.Tk() + tk_root.withdraw() + path = filedialog.askopenfilename( + title="Select file to send", + parent=tk_root, + ) + tk_root.destroy() + if path: + _emit("file_selected", path=path) + return 0 else: log.error("Unknown window: %s", window_name) return 1 From 41162fc51299d7f6e84a8b822ff02b3b43d72cac Mon Sep 17 00:00:00 2001 From: offbyonebit Date: Tue, 16 Jun 2026 19:57:39 -0500 Subject: [PATCH 5/6] fix: make tray windows reliably appear on Windows Four changes to fix the long-standing issue where clicking tray menu items on Windows produced no visible window: 1. Route child subprocess stderr to the ClipSync log file instead of DEVNULL. Crashes were silently swallowed; they now appear in %APPDATA%\ClipSync\clipsync.log so the root cause is diagnosable. 2. Call AllowSetForegroundWindow(pid) on the child after spawning. Windows' foreground-activation lock silently ignores focus_force() and lift() from any process that wasn't directly activated by user input. Tray callbacks never qualify, so windows were being created but rendered behind everything with no way to raise them. 3. Add CREATE_NO_WINDOW to the Popen creationflags on Windows so the child subprocess does not spawn a transient console window, which can confuse the Win32 subsystem when the parent has no console. 4. Deactivate CTk's Windows titlebar manipulation on CTk (root) in addition to CTkToplevel. CTk.__init__ was triggering a withdraw/update/deiconify dance that raced with our root.withdraw() call and could leave internal window-exists state inconsistent. Co-Authored-By: Claude Sonnet 4.6 --- clipsync/ui.py | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/clipsync/ui.py b/clipsync/ui.py index 24d6696..f3ae472 100644 --- a/clipsync/ui.py +++ b/clipsync/ui.py @@ -69,14 +69,35 @@ def open(self, window: str) -> None: cmd = [sys.executable, "ui", window] else: cmd = [sys.executable, "-m", "clipsync.ui", window] - proc = subprocess.Popen( - cmd, + # Route child stderr to the log file so crashes are visible instead + # of silently swallowed. Open in binary-append so it doesn't race + # with the parent's RotatingFileHandler. + try: + stderr_fh = open(config.LOG_FILE, "ab") + except OSError: + stderr_fh = subprocess.DEVNULL # type: ignore[assignment] + kwargs: dict = dict( stdout=subprocess.PIPE, - stderr=subprocess.DEVNULL, + stderr=stderr_fh, stdin=subprocess.DEVNULL, text=True, bufsize=1, ) + if sys.platform == "win32": + kwargs["creationflags"] = subprocess.CREATE_NO_WINDOW + proc = subprocess.Popen(cmd, **kwargs) + if stderr_fh is not subprocess.DEVNULL: + stderr_fh.close() + if sys.platform == "win32": + # Grant the child permission to bring its window to the + # foreground. Without this, Windows' foreground-activation + # lock silently ignores lift()/focus_force() calls from a + # process that wasn't directly activated by user input. + try: + import ctypes + ctypes.windll.user32.AllowSetForegroundWindow(proc.pid) # type: ignore[attr-defined] + except Exception: + pass self._procs[key] = proc threading.Thread( target=self._read_events, @@ -103,6 +124,8 @@ def _read_events(self, proc: subprocess.Popen[str]) -> None: log.exception("UI event handler raised") finally: proc.wait() + if proc.returncode not in (0, None, -15): # -15 = SIGTERM on Unix + log.warning("UI child process exited with code %d (check log file for details)", proc.returncode) def close_all(self) -> None: with self._lock: @@ -1581,11 +1604,13 @@ def _run_child(window_name: str) -> int: app = AppContext(settings=settings, client=client, device_id=device_id) - # Disable ctk's Windows titlebar manipulation. It does a withdraw/ - # deiconify dance on init and on every resizable() call, and when - # cycles overlap (as they do in a tabbed window) the state capture - # races and leaves the window hidden. We lose the dark titlebar on - # Windows; that's an acceptable trade for a window that actually opens. + # Disable ctk's Windows titlebar manipulation on both CTk and CTkToplevel. + # Without this, CTk.__init__ triggers a withdraw/update() dance that makes + # the hidden root window briefly appear, and CTkToplevel.resizable() queues + # further deiconify callbacks that race with our own withdraw() calls and + # can leave windows permanently hidden. We lose the dark titlebar on + # Windows; that's an acceptable trade for windows that reliably open. + ctk.CTk._deactivate_windows_window_header_manipulation = True ctk.CTkToplevel._deactivate_windows_window_header_manipulation = True theme = str(settings.get("theme") or "System") if theme not in ("Light", "Dark", "System"): From eb135726b7d6d3787f3fc64bd4f1abf9cb134953 Mon Sep 17 00:00:00 2001 From: offbyonebit Date: Wed, 17 Jun 2026 07:49:07 -0500 Subject: [PATCH 6/6] fix: update Linux paste-freeze tests for XFixes architecture tests/test_linux_paste_freeze.py still imported and exercised _LINUX_IMAGE_CHECK_INTERVAL and _linux_clipboard_has_image, both removed in d27eb0a when polling was replaced by an XFixes-based clipboard-owner watcher. The whole module has been failing to collect since that commit, so this suite hasn't actually run. - Drop the dead imports; fold the TARGETS-gate scenarios that referenced _linux_clipboard_has_image into TestReadImageGate, which now exercises _read_image_from_system_clipboard directly (the TARGETS check is inlined there post-refactor). - Replace TestOutTickRateLimiting (tested a deleted interval-based throttle) with TestOutLoopEventDriven, which drives _out_loop with a fake XFixes queue and asserts the real invariants: one tick at startup, a tick after an event once the 300 ms debounce elapses, rapid events collapsing to a single follow-up tick, and a prompt exit on the stop sentinel. - Rename the two_sided_linux end-to-end tests/fixture away from "throttle" terminology now that there's no interval to throttle on the polling fallback path; drop the frequency-bound assertion that no longer applies. --- tests/test_linux_paste_freeze.py | 312 +++++++++++++++---------------- 1 file changed, 148 insertions(+), 164 deletions(-) diff --git a/tests/test_linux_paste_freeze.py b/tests/test_linux_paste_freeze.py index 536c721..4589360 100644 --- a/tests/test_linux_paste_freeze.py +++ b/tests/test_linux_paste_freeze.py @@ -1,25 +1,32 @@ """Regression tests for the Linux paste-freeze bug. -Root cause: _out_tick called _read_clipboard_image() on every 0.5 s poll tick. -On Linux that spawns xclip which sends an X11 SelectionRequest to the clipboard -owner (typically a browser). Browsers service these on their main thread, so -hammering at 0.5 s caused the browser to freeze when the user tried to paste. +Root cause: the OUT loop used to poll the clipboard every 0.5 s, and image +checks ran on every poll tick. On Linux that spawns xclip which sends an X11 +SelectionRequest to the clipboard owner (typically a browser). Browsers +service these on their main thread, so hammering at 0.5 s caused the browser +to freeze when the user tried to paste. The fix has two parts: - 1. _read_image_from_system_clipboard() now runs a cheap TARGETS/list-types + 1. _read_image_from_system_clipboard() runs a cheap TARGETS/list-types pre-check and only fetches image bytes when the clipboard actually - advertises image/png. This is enforced even during the IN-loop - self-originated check so we never send image/png SelectionRequests when - the clipboard holds text. - 2. _out_tick() rate-limits image checks on Linux to _LINUX_IMAGE_CHECK_INTERVAL - seconds (default 2 s) instead of every poll tick. + advertises image/png, so no image/png SelectionRequest is ever sent + when the clipboard holds text. + 2. The OUT loop no longer polls on Linux: a clipboard-owner watcher built on + the X11 XFIXES extension wakes _out_tick() only when the CLIPBOARD + selection owner actually changes (i.e. the user copied something), with + a 300 ms debounce so we don't compete with an immediate paste in the same + gesture. xclip/wl-paste is therefore invoked on real clipboard changes + only, never on a fixed interval. Wayland and environments without + XFIXES fall back to polling at CLIPBOARD_POLL_INTERVAL, same as before. """ from __future__ import annotations import io +import queue import subprocess import sys +import threading import time import types import unittest.mock as mock @@ -32,8 +39,7 @@ from clipsync import config from clipsync.clipboard import ( ClipboardSync, - _LINUX_IMAGE_CHECK_INTERVAL, - _linux_clipboard_has_image, + _STOP_SENTINEL, _read_image_from_system_clipboard, ) @@ -65,59 +71,48 @@ def _wait_for(predicate, timeout: float = 5.0, interval: float = 0.05) -> bool: # --------------------------------------------------------------------------- -# _linux_clipboard_has_image unit tests +# _read_image_from_system_clipboard: TARGETS gate on Linux +# +# The TARGETS pre-check used to live in a separate _linux_clipboard_has_image() +# helper; it's now inlined directly into _read_image_from_system_clipboard(), +# so these tests exercise that function end-to-end instead of a standalone +# boolean helper. # --------------------------------------------------------------------------- -class TestLinuxClipboardHasImage: - """Unit tests for the TARGETS pre-check helper.""" +class TestReadImageGate: + """The TARGETS pre-check must prevent image/png requests when no image present.""" + + @pytest.fixture(autouse=True) + def _force_linux(self, monkeypatch): + monkeypatch.setattr(sys, "platform", "linux") - def _run(self, cmd, **kwargs): - """Default subprocess.run stub: xclip not found, wl-paste not found.""" - raise FileNotFoundError + def test_returns_none_when_no_tools_installed_at_all(self, monkeypatch): + monkeypatch.setattr(subprocess, "run", lambda *a, **kw: (_ for _ in ()).throw(FileNotFoundError())) + assert _read_image_from_system_clipboard() is None - def test_returns_false_when_no_tools_installed(self, monkeypatch): - monkeypatch.setattr(subprocess, "run", self._run) - assert _linux_clipboard_has_image() is False + def test_falls_back_to_wl_paste_when_xclip_missing(self, monkeypatch): + """If xclip isn't installed, the TARGETS check must still try wl-paste.""" - def test_returns_false_when_xclip_targets_has_no_image(self, monkeypatch): def fake_run(cmd, **kwargs): - if "TARGETS" in cmd or "--list-types" in cmd: + if cmd[0] == "xclip": + raise FileNotFoundError + if "--list-types" in cmd: result = mock.Mock() result.returncode = 0 - result.stdout = b"UTF8_STRING\ntext/plain\nTARGETS\n" + result.stdout = b"image/png\ntext/plain\n" return result - raise AssertionError(f"Unexpected command: {cmd}") - - monkeypatch.setattr(subprocess, "run", fake_run) - assert _linux_clipboard_has_image() is False - - def test_returns_true_when_xclip_targets_includes_image_png(self, monkeypatch): - def fake_run(cmd, **kwargs): - if "TARGETS" in cmd: + if "image/png" in cmd: result = mock.Mock() result.returncode = 0 - result.stdout = b"UTF8_STRING\nimage/png\ntext/plain\n" + result.stdout = FAKE_PNG return result raise AssertionError(f"Unexpected command: {cmd}") monkeypatch.setattr(subprocess, "run", fake_run) - assert _linux_clipboard_has_image() is True - - def test_falls_back_to_wl_paste_when_xclip_missing(self, monkeypatch): - def fake_run(cmd, **kwargs): - if cmd[0] == "xclip": - raise FileNotFoundError - # wl-paste --list-types - result = mock.Mock() - result.returncode = 0 - result.stdout = b"image/png\ntext/plain\n" - return result + assert _read_image_from_system_clipboard() == FAKE_PNG - monkeypatch.setattr(subprocess, "run", fake_run) - assert _linux_clipboard_has_image() is True - - def test_returns_false_when_xclip_exits_nonzero(self, monkeypatch): + def test_returns_none_when_targets_exits_nonzero(self, monkeypatch): """A non-zero exit from TARGETS means the clipboard is empty or an error occurred — in both cases there is no image.""" @@ -128,39 +123,31 @@ def fake_run(cmd, **kwargs): return result monkeypatch.setattr(subprocess, "run", fake_run) - assert _linux_clipboard_has_image() is False + assert _read_image_from_system_clipboard() is None def test_skips_timed_out_tool_continues_to_next(self, monkeypatch): + """If xclip's TARGETS check times out, wl-paste must still be tried.""" calls: list[str] = [] def fake_run(cmd, **kwargs): calls.append(cmd[0]) if cmd[0] == "xclip": raise subprocess.TimeoutExpired(cmd, 1) - # wl-paste returns image/png + if "--list-types" in cmd: + result = mock.Mock() + result.returncode = 0 + result.stdout = b"image/png\n" + return result result = mock.Mock() result.returncode = 0 - result.stdout = b"image/png\n" + result.stdout = FAKE_PNG return result monkeypatch.setattr(subprocess, "run", fake_run) - result = _linux_clipboard_has_image() + result = _read_image_from_system_clipboard() assert "xclip" in calls assert "wl-paste" in calls - assert result is True - - -# --------------------------------------------------------------------------- -# _read_image_from_system_clipboard: TARGETS gate on Linux -# --------------------------------------------------------------------------- - - -class TestReadImageGate: - """The TARGETS pre-check must prevent image/png requests when no image present.""" - - @pytest.fixture(autouse=True) - def _force_linux(self, monkeypatch): - monkeypatch.setattr(sys, "platform", "linux") + assert result == FAKE_PNG def test_skips_image_fetch_when_targets_has_no_image(self, monkeypatch): """When TARGETS returns no image/png the image/png fetch must not run.""" @@ -212,15 +199,24 @@ def test_returns_none_when_no_tools_installed(self, monkeypatch): # --------------------------------------------------------------------------- -# _out_tick: rate-limiting on Linux +# _out_loop: XFixes-event-driven dispatch on Linux +# +# There is no interval-based throttle anymore. Instead, when an XFixes +# watcher is available, _out_loop blocks on _xfixes_queue and only calls +# _out_tick() when the CLIPBOARD selection owner actually changes, after a +# 300 ms debounce. These tests drive _out_loop directly with a fake queue +# standing in for the real XFixes watcher thread. # --------------------------------------------------------------------------- -class TestOutTickRateLimiting: - """Image checks in _out_tick must be throttled on Linux.""" +class TestOutLoopEventDriven: + """_out_tick must run only on real clipboard-owner-change events, debounced.""" @pytest.fixture(autouse=True) - def _force_linux(self, monkeypatch): + def _force_linux(self, tmp_path, monkeypatch): + # tmp_path must be resolved before the platform patch: pytest's own + # tmp_path fixture checks sys.platform to decide whether to call + # os.getuid(), which doesn't exist on real Windows. monkeypatch.setattr(sys, "platform", "linux") def _make_sync(self, tmp_path) -> ClipboardSync: @@ -228,86 +224,100 @@ def _make_sync(self, tmp_path) -> ClipboardSync: settings.set("sync_folder", str(tmp_path / "sync")) (tmp_path / "sync").mkdir() sync = ClipboardSync(settings) - sync._read_clipboard = lambda: "hello" # type: ignore[method-assign] + sync._read_clipboard = lambda: None # type: ignore[method-assign] + sync._read_clipboard_image = lambda: None # type: ignore[method-assign] sync._write_clipboard = lambda v: True # type: ignore[method-assign] return sync - def test_image_check_skipped_when_recently_checked(self, tmp_path): - """If _last_image_check was just set, _read_clipboard_image must not be called.""" - sync = self._make_sync(tmp_path) - image_check_count = {"n": 0} - - def counting_image_read(): - image_check_count["n"] += 1 - return None - - sync._read_clipboard_image = counting_image_read # type: ignore[method-assign] - sync._last_image_check = time.monotonic() # pretend we just checked + def _run_loop(self, sync: ClipboardSync) -> threading.Thread: + sync._xfixes_queue = queue.SimpleQueue() + t = threading.Thread(target=sync._out_loop, daemon=True) + t.start() + return t - sync._out_tick() + def _stop_loop(self, sync: ClipboardSync, t: threading.Thread) -> None: + sync._stop.set() + sync._xfixes_queue.put(_STOP_SENTINEL) + t.join(timeout=2.0) + assert not t.is_alive(), "_out_loop did not exit after stop sentinel" - assert image_check_count["n"] == 0, ( - f"Image check ran despite throttle; called {image_check_count['n']} time(s)" - ) - - def test_image_check_runs_when_interval_elapsed(self, tmp_path, monkeypatch): - """After _LINUX_IMAGE_CHECK_INTERVAL has elapsed, the check must fire.""" - monkeypatch.setattr(clipboard_module, "_LINUX_IMAGE_CHECK_INTERVAL", 0.0) + def test_out_tick_runs_once_at_startup_with_no_events(self, tmp_path): + """Only the initial tick fires; the loop must not poll while idle.""" sync = self._make_sync(tmp_path) - image_check_count = {"n": 0} - - def counting_image_read(): - image_check_count["n"] += 1 - return None - - sync._read_clipboard_image = counting_image_read # type: ignore[method-assign] - sync._last_image_check = 0.0 # never checked - - sync._out_tick() - - assert image_check_count["n"] == 1, ( - f"Expected 1 image check but got {image_check_count['n']}" - ) - - def test_text_sync_still_works_when_image_check_throttled(self, tmp_path, monkeypatch): - """Text clipboard sync must not be blocked by the image throttle.""" + tick_count = {"n": 0} + sync._out_tick = lambda: tick_count.__setitem__("n", tick_count["n"] + 1) # type: ignore[method-assign] + + t = self._run_loop(sync) + try: + time.sleep(0.5) # well past the 300 ms debounce window, no events sent + assert tick_count["n"] == 1, f"Expected only the initial tick, got {tick_count['n']}" + finally: + self._stop_loop(sync, t) + + def test_out_tick_runs_after_xfixes_event_with_debounce(self, tmp_path): + """An event must trigger a tick, but only after the ~300 ms debounce.""" sync = self._make_sync(tmp_path) - sync._last_image_check = time.monotonic() # throttle image check - sync._last_synced = None # force a text sync - write_count = {"n": 0} - - original_write_file = sync._write_file - - def counting_write(text: str) -> None: - write_count["n"] += 1 - # Don't actually write to disk in this unit test - pass - - sync._write_file = counting_write # type: ignore[method-assign] - sync._out_tick() - - assert write_count["n"] == 1, ( - f"Expected text to be written once but got {write_count['n']} write(s)" - ) + tick_times: list[float] = [] + sync._out_tick = lambda: tick_times.append(time.monotonic()) # type: ignore[method-assign] + + t = self._run_loop(sync) + try: + time.sleep(0.05) # let the initial tick land + event_time = time.monotonic() + sync._xfixes_queue.put(True) + assert _wait_for(lambda: len(tick_times) >= 2, timeout=2.0), "second tick never fired" + gap = tick_times[1] - event_time + assert gap >= 0.25, f"Tick fired too soon after event (after {gap:.3f}s); debounce not honoured" + finally: + self._stop_loop(sync, t) + + def test_rapid_events_during_debounce_collapse_to_one_tick(self, tmp_path): + """Multiple events arriving within the debounce window must cause one tick, not one per event.""" + sync = self._make_sync(tmp_path) + tick_count = {"n": 0} + sync._out_tick = lambda: tick_count.__setitem__("n", tick_count["n"] + 1) # type: ignore[method-assign] + + t = self._run_loop(sync) + try: + time.sleep(0.05) # let the initial tick land + for _ in range(5): + sync._xfixes_queue.put(True) + time.sleep(0.01) # all within the 300 ms debounce window + assert _wait_for(lambda: tick_count["n"] >= 2, timeout=2.0), "follow-up tick never fired" + time.sleep(0.5) # give any erroneous extra ticks time to show up + assert tick_count["n"] == 2, f"Expected exactly 2 ticks (initial + 1 collapsed), got {tick_count['n']}" + finally: + self._stop_loop(sync, t) + + def test_stop_sentinel_exits_promptly_without_ticking(self, tmp_path): + """Stopping while idle must not produce a spurious tick.""" + sync = self._make_sync(tmp_path) + tick_count = {"n": 0} + sync._out_tick = lambda: tick_count.__setitem__("n", tick_count["n"] + 1) # type: ignore[method-assign] - def test_image_check_interval_is_at_least_two_seconds(self): - """Guard against accidentally reducing the interval back to 0.5 s.""" - assert _LINUX_IMAGE_CHECK_INTERVAL >= 2.0, ( - f"_LINUX_IMAGE_CHECK_INTERVAL={_LINUX_IMAGE_CHECK_INTERVAL} is too small; " - "values < 2 s risk reintroducing the paste-freeze bug" - ) + t = self._run_loop(sync) + time.sleep(0.05) + start = time.monotonic() + self._stop_loop(sync, t) + assert time.monotonic() - start < 1.0, "loop took too long to exit on stop sentinel" + assert tick_count["n"] == 1, "only the initial tick should have fired" # --------------------------------------------------------------------------- -# End-to-end: image sync still works with throttling enabled +# End-to-end: image sync still works on the Linux polling fallback +# +# These instances run start()/stop() directly; on a machine without a real +# X display (true in CI and on this dev box), _try_start_xfixes_watcher() +# fails to open the display and returns None, so ClipboardSync falls back to +# plain polling at CLIPBOARD_POLL_INTERVAL -- exercising the same path Wayland +# users hit. # --------------------------------------------------------------------------- @pytest.fixture def two_sided_linux(tmp_path, monkeypatch): - """Two ClipboardSync instances on a shared folder, image interval shrunk for speed.""" + """Two ClipboardSync instances on a shared folder, polling at a fast interval for speed.""" monkeypatch.setattr(config, "CLIPBOARD_POLL_INTERVAL", POLL) - monkeypatch.setattr(clipboard_module, "_LINUX_IMAGE_CHECK_INTERVAL", POLL * 2) monkeypatch.setattr(clipboard_module, "Observer", PollingObserver) monkeypatch.setattr(sys, "platform", "linux") @@ -346,45 +356,19 @@ def two_sided_linux(tmp_path, monkeypatch): b.stop() -def test_image_syncs_with_linux_throttle(two_sided_linux) -> None: - """Images must still reach the remote peer even when the Linux throttle is active.""" +def test_image_syncs_on_linux_polling_fallback(two_sided_linux) -> None: + """Images must still reach the remote peer when XFixes is unavailable.""" _, _, _, img_a, _, img_b = two_sided_linux img_a["image"] = FAKE_PNG assert _wait_for(lambda: img_b["image"] == FAKE_PNG, timeout=5.0), ( - f"Image did not sync under Linux throttle; got {img_b['image']!r}" + f"Image did not sync on polling fallback; got {img_b['image']!r}" ) -def test_text_syncs_normally_under_linux_throttle(two_sided_linux) -> None: - """Text sync must not be disrupted by the image-check rate limiter.""" +def test_text_syncs_on_linux_polling_fallback(two_sided_linux) -> None: + """Text sync must work normally alongside image checks on the polling path.""" _, _, txt_a, _, txt_b, _ = two_sided_linux txt_a["value"] = "paste freeze fixed" assert _wait_for(lambda: txt_b["value"] == "paste freeze fixed"), ( f"Text did not sync; got {txt_b['value']!r}" ) - - -def test_image_polling_frequency_bounded_on_linux(two_sided_linux, monkeypatch) -> None: - """The image read must not be called more often than _LINUX_IMAGE_CHECK_INTERVAL.""" - a, _, _, _, _, _ = two_sided_linux - image_check_times: list[float] = [] - original = a._read_clipboard_image - - def tracking_read(): - image_check_times.append(time.monotonic()) - return original() - - a._read_clipboard_image = tracking_read # type: ignore[method-assign] - - observe_duration = POLL * 30 # ~1.5 s at POLL=0.05 - time.sleep(observe_duration) - - if len(image_check_times) >= 2: - gaps = [image_check_times[i + 1] - image_check_times[i] for i in range(len(image_check_times) - 1)] - min_gap = min(gaps) - # Allow 20 % tolerance below the configured interval. - threshold = clipboard_module._LINUX_IMAGE_CHECK_INTERVAL * 0.8 - assert min_gap >= threshold, ( - f"Image check fired too frequently: min gap {min_gap:.3f}s < threshold {threshold:.3f}s. " - f"Gaps: {[f'{g:.3f}' for g in gaps]}" - )