diff --git a/CLAUDE.md b/CLAUDE.md index 04acb8e..05c56cc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -440,11 +440,20 @@ persistence) → GUI (Qt widgets that read the manager and connect to its signal is copied, writing nothing when nothing moved. `PlayJournal.load` then treats the R1 as one more machine. **A wedged gvfs-MTP mount blocks in uninterruptible FUSE waits**: `timeout` - can't kill a process stuck on it, and `find_device()` — called on the GUI - thread every time the Connections menu opens, and by the GUI tests — - freezes with it. USB re-enumeration (lock/unlock with `mtp,adb`, an - `adb install`) is what wedged it in Round 51. Recovery: `kill` the - `gvfsd-mtp` process, then `gio mount mtp:///`. + can't kill a process stuck on it. USB re-enumeration (lock/unlock with + `mtp,adb`, an `adb install`) is what wedged it in Round 51. Recovery: + `kill` the `gvfsd-mtp` process, then `gio mount mtp:///` — + `device_sync.reset_mtp_connection`, offered as **Reset Connection** when + the menu item reads "Rabbit isn't responding…". Since Round 71 **the GUI + thread never calls `find_device()`**: it froze the window every time the + Connections menu opened on a wedged mount. `MainWindow._devices` + (`device_sync.DeviceWatcher`) probes on a daemon thread, one at a time, + and declares `STUCK` after 3 s; click handlers go through + `when_ready(callback)`. A stuck probe thread can't be killed, so quitting + then may still hang until the reset. `tests/conftest.py` patches + `find_device` to `None` for every test, and nothing — test or tool call — + may list `/run/user/*/gvfs`; ask `gio mount -l` instead, which goes over + D-Bus. - **`lintunes/export/`** — `File → Export Playlist…` (also on a playlist's right-click menu). A sibling of `device_sync`, reusing its filename helpers diff --git a/lintunes/__init__.py b/lintunes/__init__.py index 0c6c7e4..a8bae8b 100644 --- a/lintunes/__init__.py +++ b/lintunes/__init__.py @@ -1,3 +1,3 @@ """LinTunes — iTunes-style music library manager and player for Linux.""" -__version__ = "0.32.0" +__version__ = "0.32.1" diff --git a/lintunes/device_sync.py b/lintunes/device_sync.py index 234b995..485c09f 100644 --- a/lintunes/device_sync.py +++ b/lintunes/device_sync.py @@ -63,6 +63,134 @@ def find_device() -> Device | None: return find_rabbit() +UNKNOWN, PRESENT, ABSENT, STUCK = "unknown", "present", "absent", "stuck" +PROBE_TIMEOUT_MS = 3000 + + +class DeviceWatcher(QObject): + """``find_device()`` off the GUI thread, because a wedged gvfs-MTP mount + blocks it in an uninterruptible FUSE wait and nothing can kill it. + + The Connections menu used to call ``find_device()`` on every open, which + froze the whole window the moment the mount wedged (Round 71). Now the + GUI only ever reads ``device``/``state``; ``probe()`` refreshes them on a + daemon thread. One probe at a time — a stuck one is never joined and + never stacked on — and one that hasn't answered within PROBE_TIMEOUT_MS + reports ``STUCK``. A late answer still lands, so a mount that was only + slow recovers by itself. + """ + + changed = pyqtSignal() + _answered = pyqtSignal(object) # thread -> GUI thread (queued) + + def __init__(self, finder=None, timeout_ms: int = PROBE_TIMEOUT_MS, + parent=None): + super().__init__(parent) + from PyQt6.QtCore import QTimer + self._finder = finder or find_device + self.device: Device | None = None + self.state = UNKNOWN + self._in_flight = False + self._waiters: list = [] + self._timer = QTimer(self) + self._timer.setSingleShot(True) + self._timer.setInterval(timeout_ms) + self._timer.timeout.connect(self._on_timeout) + self._answered.connect(self._on_answer) + + def probe(self): + """Refresh in the background; a probe already running is enough.""" + if self._in_flight: + return + self._in_flight = True + self._timer.start() + threading.Thread(target=self._run, daemon=True, + name="device-probe").start() + + def when_ready(self, callback): + """``callback(device, state)`` once a fresh probe answers, or as soon + as it is declared stuck — whichever comes first, exactly once.""" + self._waiters.append(callback) + if self.state == STUCK and self._in_flight: + # Still the same stuck probe: no point waiting another timeout. + self._flush() + return + self.probe() + + def _run(self): + try: + device = self._finder() + except Exception: + device = None + self._answered.emit(device) + + def _on_answer(self, device): + self._in_flight = False + self._timer.stop() + self.device = device + self.state = PRESENT if device is not None else ABSENT + self.changed.emit() + self._flush() + + def _on_timeout(self): + if not self._in_flight: + return + self.device = None + self.state = STUCK + self.changed.emit() + self._flush() + + def _flush(self): + waiters, self._waiters = self._waiters, [] + for callback in waiters: + callback(self.device, self.state) + + +def mtp_uris(gio_output: str) -> list[str]: + """The Rabbit's ``mtp://…/`` URIs from ``gio mount -l`` output. + + gio asks the gvfs daemon over D-Bus, so unlike listing the FUSE path it + answers even while the mount is wedged.""" + uris = [] + for match in re.finditer(r"(mtp://\S+)", gio_output): + uri = match.group(1) + if "rabbit" in uri.lower() and uri not in uris: + uris.append(uri) + return uris + + +def reset_mtp_connection(run=None) -> str | None: + """Kill gvfsd-mtp and remount the Rabbit — the recovery for a wedged + mount. Blocking (subprocesses with timeouts); call it from a thread. + Returns None on success, else what went wrong.""" + import subprocess + run = run or subprocess.run + try: + listing = run(["gio", "mount", "-l"], capture_output=True, + text=True, timeout=10).stdout + except (OSError, subprocess.SubprocessError) as e: + return f"gio isn't available: {e}" + uris = mtp_uris(listing) + try: + run(["pkill", "-x", "gvfsd-mtp"], timeout=10) + except (OSError, subprocess.SubprocessError) as e: + return f"couldn't stop gvfsd-mtp: {e}" + if not uris: + return None # nothing to remount; replugging will mount it + import time + time.sleep(1) # let gvfs notice the daemon went away + for uri in uris: + try: + result = run(["gio", "mount", uri], capture_output=True, + text=True, timeout=30) + except (OSError, subprocess.SubprocessError) as e: + return f"couldn't remount {uri}: {e}" + if result.returncode != 0 and "already mounted" not in ( + result.stderr or "").lower(): + return (result.stderr or "").strip() or f"gio mount {uri} failed" + return None + + def sanitize_name(name: str) -> str: """A playlist/track name reduced to a safe cross-filesystem filename.""" cleaned = " ".join(_FORBIDDEN.sub(" ", name).split()) diff --git a/lintunes/gui/main_window.py b/lintunes/gui/main_window.py index 9b6a08a..9441e68 100644 --- a/lintunes/gui/main_window.py +++ b/lintunes/gui/main_window.py @@ -1,4 +1,5 @@ import shutil +import threading from pathlib import Path from PyQt6.QtWidgets import ( @@ -6,7 +7,7 @@ from PyQt6.QtWidgets import ( QPlainTextEdit, QTextEdit, QAbstractSpinBox, QComboBox, QApplication, QLabel, QMessageBox, QFileDialog, QProgressBar, QPushButton, ) -from PyQt6.QtCore import Qt, QEvent, QTimer +from PyQt6.QtCore import Qt, QEvent, QObject, QTimer, pyqtSignal from PyQt6.QtGui import QAction, QKeySequence from lintunes import device_sync, music_folder, theme, url_import @@ -82,6 +83,11 @@ class _CancelSyncButton(QPushButton): super().leaveEvent(event) +class _ResetRelay(QObject): + """Carries reset_mtp_connection's answer back to the GUI thread.""" + done = pyqtSignal(object) + + class MainWindow(QMainWindow): def __init__(self, manager, prefs, lastfm=None, parent=None): super().__init__(parent) @@ -93,6 +99,11 @@ class MainWindow(QMainWindow): # Owns the cast session, if any. Built before the transport bar, which # takes it to drive the little cast indicator under the volume slider. self._cast = CastController(self.player, self) + # Where the Rabbit is, found off the GUI thread: a wedged gvfs-MTP + # mount blocks any listing of it forever (Round 71). Nothing probes + # until the Connections menu opens. + self._devices = device_sync.DeviceWatcher(parent=self) + self._devices.changed.connect(self._refresh_device_actions) # Keep the machine awake while audio is actually playing. self._inhibitor = SleepInhibitor() # Context playback started from: "library" or "playlist:". Scopes @@ -413,8 +424,8 @@ class MainWindow(QMainWindow): self._cassette.build_menu(connections_menu) self._cast_action = self._add_action( connections_menu, "Connect to Chromecast…", "", self._toggle_cast) - # Re-checked every time the menu opens: cheap (one gvfs listdir), and - # always reflects plug/unplug, the cast session and the current view. + # Re-checked every time the menu opens. The device part is a + # background probe; its answer updates the open menu when it lands. connections_menu.aboutToShow.connect(self._refresh_connection_actions) @property @@ -475,11 +486,8 @@ class MainWindow(QMainWindow): def _refresh_connection_actions(self): # An export in flight owns the same status-bar widgets, so it blocks # a sync just as another sync would. - busy = self._busy_worker() is not None - device = device_sync.find_device() - # Whole-library sync doesn't care which view is open — only that a - # device is plugged in and at least one playlist is ticked. - self._andtunes_action.setEnabled(not busy and device is not None) + self._devices.probe() + self._refresh_device_actions() self._cassette.refresh_menu() # Enabled whenever there's an APK to offer; the click finds the # Rabbit (adb or MTP) and says so if it can't — cheaper than asking @@ -583,6 +591,57 @@ class MainWindow(QMainWindow): # cancelled sync has news for the journals. self._manager.reload_from_disk() + def _refresh_device_actions(self): + """Sync to Rabbit from the watcher's cached answer — never a fresh + look at the mount, which is what froze the menu.""" + busy = self._busy_worker() is not None + if self._devices.state == device_sync.STUCK: + # Enabled: the click explains and offers the reset. + self._andtunes_action.setText("Rabbit isn't responding…") + self._andtunes_action.setEnabled(True) + return + self._andtunes_action.setText("Sync to Rabbit") + # Whole-library sync doesn't care which view is open — only that a + # device is plugged in and at least one playlist is ticked. + self._andtunes_action.setEnabled( + not busy and self._devices.device is not None) + + def _rabbit_stuck(self): + """The mount stopped answering: say so, and offer the CLAUDE.md + recovery (kill gvfsd-mtp, remount) as one button.""" + box = QMessageBox(self) + box.setIcon(QMessageBox.Icon.Warning) + box.setWindowTitle("Rabbit isn't responding") + box.setText("The Rabbit's USB connection has stopped answering, so " + "LinTunes can't reach it.") + box.setInformativeText( + "Reset Connection restarts the desktop's MTP service and " + "remounts the Rabbit. If that doesn't do it, unplug the Rabbit " + "and plug it back in.") + reset = box.addButton("Reset Connection", + QMessageBox.ButtonRole.AcceptRole) + box.addButton(QMessageBox.StandardButton.Cancel) + box.setDefaultButton(reset) + box.exec() + if box.clickedButton() is not reset: + return + self.statusBar().showMessage("Resetting the Rabbit connection…") + relay = self._reset_relay = _ResetRelay(self) + relay.done.connect(self._on_rabbit_reset) + threading.Thread( + target=lambda: relay.done.emit(device_sync.reset_mtp_connection()), + daemon=True).start() + + def _on_rabbit_reset(self, error): + if error: + self.statusBar().clearMessage() + QMessageBox.warning(self, "Rabbit isn't responding", + f"The reset didn't work: {error}\n\n" + "Unplug the Rabbit and plug it back in.") + return + self.statusBar().showMessage("Rabbit connection reset", 6000) + self._devices.probe() + def _on_sync_failed(self, message): self._sync_inhibitor.release() self._hide_sync_widgets() @@ -591,7 +650,7 @@ class MainWindow(QMainWindow): # ---- connections: andTunes (whole-library sync) ---- def _show_device_sync_settings(self): - device = device_sync.find_device() + device = self._devices.device # only for its name dialog = DeviceSyncSettingsDialog( self._prefs, self._manager.library, self, device_name=device.name if device else "Rabbit") @@ -606,8 +665,13 @@ class MainWindow(QMainWindow): # Everything may have changed since the menu opened — re-verify. if self._busy_worker() is not None: return - device = device_sync.find_device() - if device is None: + self._devices.when_ready(self._sync_to_device) + + def _sync_to_device(self, device, state): + if state == device_sync.STUCK: + self._rabbit_stuck() + return + if device is None or self._busy_worker() is not None: return playlists = self._selected_sync_playlists() if not playlists: @@ -731,9 +795,15 @@ class MainWindow(QMainWindow): apk = andtunes_install.bundled_apk() if apk is None: return + self._devices.when_ready( + lambda device, state: self._install_andtunes_to(apk, device, state)) + + def _install_andtunes_to(self, apk, device, state): adb = andtunes_install.adb_path() serial = andtunes_install.rabbit_serial(adb) if adb else None - device = device_sync.find_device() + if serial is None and state == device_sync.STUCK: + self._rabbit_stuck() + return if serial is None and device is None: QMessageBox.information( self, "Rabbit not found", diff --git a/tests/conftest.py b/tests/conftest.py index cd03915..5d29086 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,6 +17,15 @@ def _isolate_eventlog(tmp_path, monkeypatch): lambda: tmp_path / "control-events.log") +@pytest.fixture(autouse=True) +def _no_real_device(monkeypatch): + """No test may look at the real gvfs mount: a wedged gvfs-MTP mount + blocks the listing uninterruptibly and hangs the suite (Round 71). + Tests that need a device patch their own finder or pass a fake root.""" + from lintunes import device_sync + monkeypatch.setattr(device_sync, "find_device", lambda: None) + + @pytest.fixture(scope="session") def qapp(): """A Qt application so QObject/QTimer and real widgets work in tests. diff --git a/tests/test_round71.py b/tests/test_round71.py new file mode 100644 index 0000000..428150d --- /dev/null +++ b/tests/test_round71.py @@ -0,0 +1,148 @@ +"""Round 71: a wedged Rabbit mount must never freeze the window. + +The Connections menu used to call ``find_device()`` on the GUI thread, and on +a wedged gvfs-MTP mount that listing blocks forever in an uninterruptible FUSE +wait — trav had to force-quit. ``DeviceWatcher`` does the looking on a daemon +thread and gives up waiting (not the thread) after a timeout. +""" +import threading +import time + +from PyQt6.QtCore import QCoreApplication + +from lintunes import device_sync +from lintunes.device_sync import ( + ABSENT, PRESENT, STUCK, Device, DeviceWatcher, mtp_uris, + reset_mtp_connection, +) + + +def _pump(until, timeout=3.0): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + QCoreApplication.processEvents() + if until(): + return True + time.sleep(0.01) + return False + + +def _device(tmp_path): + return Device("Rabbit R1", tmp_path, tmp_path / "Music") + + +class TestWatcher: + def test_a_hung_finder_is_reported_stuck_without_blocking(self, qapp): + release = threading.Event() + watcher = DeviceWatcher(finder=lambda: release.wait() and None, + timeout_ms=50) + started = time.monotonic() + watcher.probe() + assert time.monotonic() - started < 0.5 # probe() returns at once + assert _pump(lambda: watcher.state == STUCK) + release.set() + + def test_one_probe_at_a_time(self, qapp): + release = threading.Event() + calls = [] + + def finder(): + calls.append(1) + release.wait() + + watcher = DeviceWatcher(finder=finder, timeout_ms=50) + watcher.probe() + watcher.probe() + watcher.probe() + assert _pump(lambda: watcher.state == STUCK) + watcher.probe() + assert len(calls) == 1 + release.set() + + def test_a_late_answer_recovers(self, qapp, tmp_path): + release = threading.Event() + device = _device(tmp_path) + watcher = DeviceWatcher(finder=lambda: release.wait() and device, + timeout_ms=50) + watcher.probe() + assert _pump(lambda: watcher.state == STUCK) + release.set() + assert _pump(lambda: watcher.state == PRESENT) + assert watcher.device is device + + def test_when_ready_fires_once_with_a_fresh_answer(self, qapp, tmp_path): + watcher = DeviceWatcher(finder=lambda: None) + seen = [] + watcher.when_ready(lambda d, s: seen.append((d, s))) + assert _pump(lambda: seen) + assert seen == [(None, ABSENT)] + _pump(lambda: False, timeout=0.1) + assert len(seen) == 1 + + def test_when_ready_while_stuck_answers_immediately(self, qapp): + release = threading.Event() + watcher = DeviceWatcher(finder=lambda: release.wait(), timeout_ms=50) + watcher.probe() + assert _pump(lambda: watcher.state == STUCK) + seen = [] + watcher.when_ready(lambda d, s: seen.append(s)) + assert seen == [STUCK] + release.set() + + +class TestReset: + GIO = """Volume(0): Rabbit R1 + Type: GProxyVolume (GProxyVolumeMonitorMTP) + Mount(0): Rabbit R1 -> mtp://unknown_Rabbit_R1_9191/ +Mount(1): mtp -> mtp://unknown_Rabbit_R1_9191/ +Mount(2): Phone -> mtp://Google_Pixel_1234/ +""" + + def test_uris_come_from_gio_not_the_mount(self): + assert mtp_uris(self.GIO) == ["mtp://unknown_Rabbit_R1_9191/"] + + def test_reset_kills_then_remounts(self, monkeypatch): + monkeypatch.setattr(time, "sleep", lambda s: None) + commands = [] + + class R: + returncode, stderr = 0, "" + stdout = TestReset.GIO + + def run(cmd, **kw): + commands.append(cmd) + return R() + + assert reset_mtp_connection(run) is None + assert commands == [["gio", "mount", "-l"], + ["pkill", "-x", "gvfsd-mtp"], + ["gio", "mount", "mtp://unknown_Rabbit_R1_9191/"]] + + +class TestMenu: + def test_menu_open_never_waits_on_the_mount(self, qapp, monkeypatch, + tmp_path): + from lintunes.gui.main_window import MainWindow + from lintunes.library_manager import LibraryManager + from lintunes.models import Library + from lintunes.preferences import Preferences + + release = threading.Event() + monkeypatch.setattr(device_sync, "find_device", + lambda: release.wait() and None) + manager = LibraryManager(Library(), tmp_path / "data") + window = MainWindow(manager, Preferences(tmp_path / "data")) + window._devices._timer.setInterval(50) + try: + started = time.monotonic() + window._refresh_connection_actions() + assert time.monotonic() - started < 0.5 + assert not window._andtunes_action.isEnabled() + assert _pump(lambda: window._andtunes_action.text() + == "Rabbit isn't responding…") + assert window._andtunes_action.isEnabled() + finally: + release.set() + assert _pump(lambda: window._devices.state == ABSENT) + assert window._andtunes_action.text() == "Sync to Rabbit" + window.close()