diff --git a/src/vocalinux/ui/keyboard_backends/evdev_backend.py b/src/vocalinux/ui/keyboard_backends/evdev_backend.py index a45a7b5..cfb023a 100644 --- a/src/vocalinux/ui/keyboard_backends/evdev_backend.py +++ b/src/vocalinux/ui/keyboard_backends/evdev_backend.py @@ -70,6 +70,12 @@ KEY_RIGHTSHIFT = 54 KEY_LEFTMETA = 125 # Super/Windows key KEY_RIGHTMETA = 126 DEVICE_RESCAN_SECONDS = 2.0 +# The monitor thread stamps ``_monitor_heartbeat`` on every loop iteration; +# a heartbeat this stale while grabs are held means keystrokes queue on +# devices only a wedged reader can see. The forwarding watchdog releases +# every grab then, so input reaches applications directly again (#1021). +_MONITOR_STALL_SECONDS = 4.0 +_FORWARDING_WATCHDOG_POLL_SECONDS = 1.0 # Event types re-emitted on the uinput clone. Feedback events (EV_LED, # EV_SND, EV_FF) are kernel->device traffic already delivered to every @@ -702,6 +708,9 @@ _HUB_OWNED_ATTRIBUTES = frozenset( "_close_detached", "_resync_clone_key_state", "_pending_events", + "_monitor_heartbeat", + "_grab_suspended_devices", + "_watchdog_thread", } ) @@ -724,6 +733,9 @@ _HUB_METHOD_ATTRIBUTES = frozenset( "_detach_devices", "_close_detached", "_resync_clone_key_state", + "_forwarding_watchdog", + "_check_forwarding_health", + "_rearm_suspended_devices", } ) @@ -764,6 +776,17 @@ class EvdevDeviceHub: self.device_paths: set[str] = set() self.running = False self.monitor_thread: Optional[threading.Thread] = None + # The watchdog sibling of the monitor: releases grabs whenever the + # monitor stops dispatching while it still holds them. + self._watchdog_thread: Optional[threading.Thread] = None + # Stamped by the monitor on every loop iteration; 0.0 means it has + # not run its first pass yet. + self._monitor_heartbeat = 0.0 + # fd -> the device the watchdog ungrabbed on a stall, to be re-armed + # once the monitor is dispatching again. The device object itself is + # re-verified at recovery so an fd recycled for another device while + # suspended is never torn down under it. + self._grab_suspended_devices: dict[int, InputDevice] = {} # True once a teardown detached the containers; new opens are # refused so a rescan can't leak a device nobody will close. self._closed = False @@ -859,6 +882,12 @@ class EvdevDeviceHub: daemon=True, ) self.monitor_thread.start() + self._watchdog_thread = threading.Thread( + target=self._forwarding_watchdog, + kwargs={"generation": self._generation}, + daemon=True, + ) + self._watchdog_thread.start() logger.info("Shared evdev device layer started") return True @@ -890,9 +919,13 @@ class EvdevDeviceHub: self._close_all_devices() monitor = self.monitor_thread self.monitor_thread = None + watchdog = self._watchdog_thread + self._watchdog_thread = None if monitor is not None: monitor.join(timeout=2.0) + if watchdog is not None: + watchdog.join(timeout=_FORWARDING_WATCHDOG_POLL_SECONDS + 0.5) def _engine_snapshot( self, @@ -1028,6 +1061,16 @@ class EvdevDeviceHub: while self.running and self._generation == generation: try: now = time.monotonic() + # Liveness for the forwarding watchdog: a heartbeat this + # stale while grabs are held means this loop wedged + # mid-iteration — the watchdog then releases every grab. + self._monitor_heartbeat = now + if self._grab_suspended_devices: + # Watchdog-released devices are retired and re-opened + # HERE, on the monitor thread: closing an fd from the + # watchdog would put a dead descriptor in the select + # set below. + self._rearm_suspended_devices(generation) if now - last_scan >= DEVICE_RESCAN_SECONDS: self._scan_for_new_devices(generation=generation) last_scan = now @@ -1151,6 +1194,7 @@ class EvdevDeviceHub: self._clone_paths = set() self._forwarded_held = {} self._pending_events = {} + self._grab_suspended_devices = {} return devices, forwarders def _close_detached(self, devices: list[InputDevice], forwarders: list[UInput]) -> None: @@ -1584,6 +1628,90 @@ class EvdevDeviceHub: for engine in engines: engine._resync_shortcut_state(fd, actually_held) + def _forwarding_watchdog(self, generation: Optional[int] = None) -> None: + """Release grabs while forwarding cannot run; re-arm on recovery. + + The monitor's own unexpected-exit cleanup covers a thread that + dies; a thread that stalls mid-iteration — wedged on an fd, a + lock, or a clone write that never returns — keeps every grab it + holds while nothing reads the devices, so keystrokes queue on + hardware only a dead reader can see and applications stay dead + (#1021). A heartbeat stale beyond ``_MONITOR_STALL_SECONDS`` while + grab->clone pairs exist therefore releases each grab, handing + input back to the kernel. Once dispatching resumes, the monitor + retires the suspended devices at its next iteration — closing + their fds from this thread would put dead descriptors in the + monitor's select set — so the rescan re-opens them through the + standard open->clone->grab path. + """ + if generation is None: + generation = self._generation + logger.debug("Starting forwarding watchdog thread") + try: + while self.running and self._generation == generation: + time.sleep(_FORWARDING_WATCHDOG_POLL_SECONDS) + if not self.running or self._generation != generation: + break + self._check_forwarding_health(generation) + finally: + logger.debug("Forwarding watchdog thread stopped") + + def _check_forwarding_health(self, generation: int) -> None: + """One watchdog pass: ungrab everything while the monitor is stalled.""" + heartbeat = self._monitor_heartbeat + if heartbeat == 0.0: + # The monitor has not run its first iteration yet. + return + if time.monotonic() - heartbeat <= _MONITOR_STALL_SECONDS: + return + + stalled: list[tuple[int, UInput, InputDevice]] = [] + with self._devices_lock: + for fd, forwarder in self._forwarders.items(): + if self._fd_generation.get(fd, generation) != generation: + continue + device = next((d for d in self.devices if d.fileno() == fd), None) + if device is not None: + stalled.append((fd, forwarder, device)) + for fd, forwarder, device in stalled: + # Passing the captured forwarder keeps the release a no-op + # when the fd was recycled for a newer generation's clone. + # ungrab/close-clone are safe cross-thread: neither fd sits in + # the monitor's select set. + self._release_failed_forwarder(fd, forwarder) + with self._devices_lock: + if fd in self.device_fds: + self._grab_suspended_devices[fd] = device + if stalled: + logger.warning( + f"Keyboard monitor stalled with {len(stalled)} grabbed " + "device(s); released the grabs so keyboards keep working " + "(dictation shortcuts stay live but unsuppressed)" + ) + + def _rearm_suspended_devices(self, generation: int) -> None: + """Retire watchdog-released devices so the rescan re-opens them. + + Runs on the monitor thread only, at a loop boundary where no + select is in flight — closing an fd from the watchdog thread + would leave a dead descriptor in the monitor's select set and its + error handler would tear down every device. The stored device + object is re-verified before removal so an fd recycled for + another device while suspended is never torn down under it. + """ + with self._devices_lock: + suspended = list(self._grab_suspended_devices.items()) + self._grab_suspended_devices.clear() + for fd, device in suspended: + with self._devices_lock: + still_ours = ( + device in self.devices and self._fd_generation.get(fd, generation) == generation + ) + if still_ours: + self._remove_keyboard_device(fd, device, generation) + if suspended: + self._scan_for_new_devices(generation=generation) + class EvdevKeyboardBackend(KeyboardBackend): """ diff --git a/src/vocalinux/ui/tray_indicator.py b/src/vocalinux/ui/tray_indicator.py index cd7d3fc..a9564b1 100644 --- a/src/vocalinux/ui/tray_indicator.py +++ b/src/vocalinux/ui/tray_indicator.py @@ -1699,6 +1699,14 @@ class TrayIndicator: keepalive = getattr(self, "_model_keepalive", None) if keepalive is not None: keepalive.cancel() + # Release the internal hotkey listeners (and their EVIOCGRABs) + # before the machine sleeps: a grab held through suspend keeps + # feeding a forwarder whose liveness across resume cannot be + # guaranteed, and the lock screen ends up behind a dead input + # path (#1021). The resume flow rebuilds the listeners once + # /dev/input settles. + self.shortcut_manager.stop() + self._stop_language_shortcut_managers() if self.speech_engine.state != RecognitionState.IDLE: logger.info("System suspending — stopping active recognition") self.speech_engine.stop_recognition() diff --git a/tests/test_evdev_backend_ext.py b/tests/test_evdev_backend_ext.py index 42e26d1..dfa0d5f 100644 --- a/tests/test_evdev_backend_ext.py +++ b/tests/test_evdev_backend_ext.py @@ -22,6 +22,7 @@ from unittest.mock import MagicMock, Mock, call, mock_open, patch import pytest from vocalinux.ui.keyboard_backends.evdev_backend import ( + _MONITOR_STALL_SECONDS, DEVICE_RESCAN_SECONDS, EvdevDeviceHub, EvdevKeyboardBackend, @@ -2954,6 +2955,224 @@ class TestEvdevGrabAndForwarding: assert backend._withheld_modifier[fd] == {} +class TestForwardingWatchdog: + """A stalled monitor must not hold grabs while nothing forwards (#1021). + + Suspend/resume can leave the reader wedged with every EVIOCGRAB held: + keystrokes queue on hardware only a dead reader can see, and the lock + screen goes dark. The watchdog stamps the monitor's liveness through + ``_monitor_heartbeat``; a heartbeat past ``_MONITOR_STALL_SECONDS`` + releases every grab, and a recovering monitor re-arms the suspended + devices through the standard open->clone->grab path. + """ + + def _hub_with_grabbed_device( + self, heartbeat: float, generation: int = 3 + ) -> tuple[EvdevDeviceHub, MagicMock, MagicMock]: + """Hub state as the monitor left it: one grabbed device + clone.""" + backend = EvdevKeyboardBackend() + hub = backend._hub + hub.reset() + hub.running = True + hub._generation = generation + hub._monitor_heartbeat = heartbeat + device = MagicMock() + device.fileno.return_value = 10 + device.name = "Keyboard" + forwarder = MagicMock() + hub.devices = [device] + hub.device_fds = [10] + hub.device_paths = {"/dev/input/event0"} + hub._device_paths_by_fd = {10: "/dev/input/event0"} + hub._fd_generation = {10: generation} + hub._forwarders = {10: forwarder} + hub._forwarded_held = {10: set()} + return hub, device, forwarder + + def test_stalled_monitor_releases_every_grab(self) -> None: + """A stale heartbeat ungrabs the device, closes its clone, suspends it.""" + hub, device, forwarder = self._hub_with_grabbed_device( + heartbeat=time.monotonic() - _MONITOR_STALL_SECONDS - 1.0 + ) + try: + hub._check_forwarding_health(hub._generation) + + device.ungrab.assert_called_once() + forwarder.close.assert_called_once() + assert hub._forwarders == {} + assert hub._grab_suspended_devices == {10: device} + # Still monitored, ungrabbed: keys reach the lock screen and the + # shortcut keeps detecting, just unsuppressed. + assert device in hub.devices + assert hub.device_fds == [10] + finally: + hub.reset() + + def test_fresh_heartbeat_keeps_grabs(self) -> None: + """A live monitor's grabs are never touched by the watchdog.""" + hub, device, forwarder = self._hub_with_grabbed_device(heartbeat=time.monotonic()) + try: + hub._check_forwarding_health(hub._generation) + + device.ungrab.assert_not_called() + forwarder.close.assert_not_called() + assert hub._forwarders == {10: forwarder} + assert hub._grab_suspended_devices == {} + finally: + hub.reset() + + def test_unstarted_monitor_is_ignored(self) -> None: + """Heartbeat 0.0 means the first iteration has not run: not a stall.""" + hub, device, forwarder = self._hub_with_grabbed_device(heartbeat=0.0) + try: + hub._check_forwarding_health(hub._generation) + + device.ungrab.assert_not_called() + forwarder.close.assert_not_called() + assert hub._forwarders == {10: forwarder} + finally: + hub.reset() + + def test_stall_skips_fds_owned_by_a_newer_generation(self) -> None: + """An fd recycled for a newer generation is never released under it.""" + hub, device, forwarder = self._hub_with_grabbed_device( + heartbeat=time.monotonic() - _MONITOR_STALL_SECONDS - 1.0, generation=3 + ) + hub._fd_generation[10] = 4 # reopened by the newer generation + try: + hub._check_forwarding_health(3) + + device.ungrab.assert_not_called() + forwarder.close.assert_not_called() + assert hub._forwarders == {10: forwarder} + assert hub._grab_suspended_devices == {} + finally: + hub.reset() + + @patch("vocalinux.ui.keyboard_backends.evdev_backend.find_keyboard_devices") + @patch("vocalinux.ui.keyboard_backends.evdev_backend._is_virtual_keyboard") + @patch("vocalinux.ui.keyboard_backends.evdev_backend.InputDevice") + @patch("vocalinux.ui.keyboard_backends.evdev_backend.UInput") + def test_recovery_reopens_suspended_devices_through_grab_path( + self, + mock_uinput: Mock, + mock_input_device: Mock, + mock_is_virtual: Mock, + mock_find: Mock, + ) -> None: + """A live heartbeat re-arms suspended devices via rescan and re-grab.""" + hub, old_device, _ = self._hub_with_grabbed_device(heartbeat=time.monotonic()) + hub._grab_suspended_devices = {10: old_device} + + new_device = MagicMock() + new_device.fileno.return_value = 20 + new_device.info.bustype = 0x03 + new_device.name = "Keyboard" + new_device.capabilities.return_value = _keyboard_only_capabilities() + new_device.active_keys.return_value = [] + mock_input_device.return_value = new_device + new_forwarder = MagicMock() + new_forwarder.device.path = "/dev/input/event9" + mock_uinput.return_value = new_forwarder + mock_is_virtual.return_value = False + mock_find.return_value = ["/dev/input/event0"] + + try: + # The monitor re-arms suspended devices at its loop top. + hub._rearm_suspended_devices(hub._generation) + + old_device.close.assert_called_once() + assert hub._grab_suspended_devices == {} + # Reopened through the normal path: grabbed, cloned, registered. + new_device.grab.assert_called_once() + assert hub._forwarders[20] is new_forwarder + assert new_device in hub.devices + assert hub.device_paths == {"/dev/input/event0"} + finally: + hub.reset() + + @patch("vocalinux.ui.keyboard_backends.evdev_backend.find_keyboard_devices") + def test_recovery_skips_suspended_device_replaced_by_another(self, mock_find: Mock) -> None: + """A suspended fd recycled for a different device tears nothing down.""" + mock_find.return_value = [] + hub, old_device, _ = self._hub_with_grabbed_device(heartbeat=time.monotonic()) + hub._grab_suspended_devices = {10: old_device} + other_device = MagicMock() + other_device.fileno.return_value = 10 + hub.devices = [other_device] # fd 10 now belongs to a newer device + + try: + hub._rearm_suspended_devices(hub._generation) + + old_device.close.assert_not_called() + other_device.close.assert_not_called() + assert other_device in hub.devices + assert hub._grab_suspended_devices == {} + finally: + hub.reset() + + def test_monitor_rearms_suspended_devices_in_loop(self) -> None: + """The monitor retires watchdog-released devices between selects. + + Recovery must run on the monitor thread: closing an fd from the + watchdog would leave a dead descriptor in the select set and the + monitor's error handler would tear down every device. + """ + hub, old_device, _ = self._hub_with_grabbed_device(heartbeat=0.0) + hub._grab_suspended_devices = {10: old_device} + rearmed: list[int] = [] + + def _fake_rearm(generation: int) -> None: + rearmed.append(generation) + hub.running = False + + try: + with ( + patch.object(hub, "_rearm_suspended_devices", side_effect=_fake_rearm), + patch( + "vocalinux.ui.keyboard_backends.evdev_backend.select.select", + return_value=([], [], []), + ), + patch( + "vocalinux.ui.keyboard_backends.evdev_backend.time.monotonic", + side_effect=[0.0, 0.0], + ), + ): + hub._monitor_devices(generation=hub._generation) + + assert rearmed == [3] + finally: + hub.reset() + + def test_watchdog_exits_when_running_stops(self) -> None: + """The watchdog returns immediately once the layer is not running.""" + hub, _, _ = self._hub_with_grabbed_device(heartbeat=time.monotonic()) + hub.running = False + try: + # Returns instead of looping: running is already False. + hub._forwarding_watchdog(generation=hub._generation) + finally: + hub.reset() + + def test_unregister_stops_and_joins_watchdog(self) -> None: + """Teardown clears the watchdog slot once the last engine leaves.""" + hub, _, _ = self._hub_with_grabbed_device(heartbeat=time.monotonic()) + engine = EvdevKeyboardBackend(shortcut="ctrl+ctrl") + engine.active = True + watchdog = MagicMock(spec=threading.Thread) + hub._watchdog_thread = watchdog + try: + with hub._engines_lock: + hub._engines.add(engine) + hub.unregister(engine) + + watchdog.join.assert_called_once() + assert hub._watchdog_thread is None + assert hub.running is False + finally: + hub.reset() + + class TestWriteOnlyUInput: """The write-only clone drives the real _uinput setup on an O_WRONLY fd.""" diff --git a/tests/test_tray_indicator.py b/tests/test_tray_indicator.py index 35c4e1f..b837a8e 100644 --- a/tests/test_tray_indicator.py +++ b/tests/test_tray_indicator.py @@ -1292,6 +1292,40 @@ class TestTrayIndicator(unittest.TestCase): self.assertEqual(len(history), 0) + def test_system_suspend_stops_keyboard_listeners(self) -> None: + """Suspend releases the internal listeners' grabs before sleep (#1021). + + A grab held through suspend feeds a forwarder whose liveness across + resume cannot be guaranteed; the lock screen ends up behind a dead + input path. Suspend therefore tears the listeners down — the resume + flow rebuilds them once /dev/input settles. + """ + language_manager = MagicMock() + self.tray_indicator._language_shortcut_managers = [language_manager] + self.mock_ksm.active = True + # _setup_keyboard_shortcuts already stopped/started the manager when + # the tray initialized; forget that history before asserting. + self.mock_ksm.reset_mock() + + self.tray_indicator._on_system_suspend() + + self.mock_ksm.stop.assert_called_once() + language_manager.stop.assert_called_once() + self.assertEqual(self.tray_indicator._language_shortcut_managers, []) + # Recognition was idle: nothing else to stop. + self.mock_speech_engine.stop_recognition.assert_not_called() + + def test_system_suspend_stops_active_recognition(self) -> None: + """Suspend still stops a listening session alongside the listeners.""" + self.tray_indicator._language_shortcut_managers = [] + self.mock_speech_engine.state = self.RecognitionState.LISTENING + self.mock_ksm.reset_mock() + + self.tray_indicator._on_system_suspend() + + self.mock_ksm.stop.assert_called_once() + self.mock_speech_engine.stop_recognition.assert_called_once() + def test_construct_with_history_creates_submenu_and_wires_callback(self) -> None: from vocalinux.ui.transcription_history import TranscriptionHistory from vocalinux.ui.tray_indicator import TrayIndicator