diff --git a/src/fenrirscreenreader/remoteDriver/unixDriver.py b/src/fenrirscreenreader/remoteDriver/unixDriver.py index 720671cb..7276689a 100644 --- a/src/fenrirscreenreader/remoteDriver/unixDriver.py +++ b/src/fenrirscreenreader/remoteDriver/unixDriver.py @@ -329,48 +329,54 @@ class driver(remoteDriver): def watch_dog(self, active, event_queue): # echo "command say this is a test" | socat - # UNIX-CLIENT:/tmp/fenrirscreenreader-daemon.sock - for socket_file, optional in self._get_socket_candidates(): - fenrir_sock = self._bind_socket(socket_file, optional) - if fenrir_sock is None: - continue - self.fenrirSocks.append(fenrir_sock) - self.bound_sockets.append((fenrir_sock, socket_file)) + try: + for socket_file, optional in self._get_socket_candidates(): + fenrir_sock = self._bind_socket(socket_file, optional) + if fenrir_sock is None: + continue + self.fenrirSocks.append(fenrir_sock) + self.bound_sockets.append((fenrir_sock, socket_file)) - if not self.fenrirSocks: - return + if not self.fenrirSocks: + return - self._try_register_instance() - last_register = time.time() - while active.value: - if time.time() - last_register > 10.0: - self._try_register_instance() - last_register = time.time() + self._try_register_instance() + last_register = time.time() + while active.value: + if time.time() - last_register > 10.0: + self._try_register_instance() + last_register = time.time() - # Check if the client is still connected and if data is available: - try: - r, _, _ = select.select(self.fenrirSocks, [], [], 0.8) - except select.error: - break - if r == []: - continue - for fenrir_sock in r: - client_sock, client_addr = fenrir_sock.accept() - socket_file = self._socket_file_for_socket(fenrir_sock) - # Ensure client socket is always closed to prevent resource - # leaks + # Check if the client is still connected and data is available. try: - self._handle_client(client_sock, event_queue, socket_file) - finally: - # Always close client socket, even if data processing fails + r, _, _ = select.select(self.fenrirSocks, [], [], 0.8) + except select.error: + break + if r == []: + continue + for fenrir_sock in r: + client_sock, _client_addr = fenrir_sock.accept() + socket_file = self._socket_file_for_socket(fenrir_sock) try: - client_sock.close() - except Exception as e: - self.env["runtime"]["DebugManager"].write_debug_out( - "unixDriver watch_dog: Error closing client socket: " - + str(e), - debug.DebugLevel.ERROR, + self._handle_client( + client_sock, event_queue, socket_file ) - self._cleanup() + finally: + try: + client_sock.close() + except Exception as e: + self.env["runtime"][ + "DebugManager" + ].write_debug_out( + "unixDriver watch_dog: Error closing client " + "socket: " + + str(e), + debug.DebugLevel.ERROR, + ) + finally: + # ProcessManager restarts a failed watchdog. Always release the + # old listeners first so each restart cannot leak another socket. + self._cleanup() def shutdown(self): self._cleanup() diff --git a/tests/unit/test_unix_remote_driver.py b/tests/unit/test_unix_remote_driver.py index 325aa733..1ff7b443 100644 --- a/tests/unit/test_unix_remote_driver.py +++ b/tests/unit/test_unix_remote_driver.py @@ -1,5 +1,7 @@ from unittest.mock import Mock, mock_open, patch +import pytest + from fenrirscreenreader.remoteDriver import unixDriver @@ -27,6 +29,40 @@ def test_watchdog_keeps_serving_when_instance_registration_fails( select_call.assert_called_once() +def test_watchdog_closes_listeners_when_unexpected_error_escapes( + mock_environment, +): + driver = unixDriver.driver() + driver.env = mock_environment + listener = Mock() + active = Mock(value=True) + + with patch.object( + driver, + "_get_socket_candidates", + return_value=[("/tmp/fenrir-test.sock", False)], + ), patch.object( + driver, "_bind_socket", return_value=listener + ), patch.object( + driver, "_try_register_instance" + ), patch( + "fenrirscreenreader.remoteDriver.unixDriver.select.select", + side_effect=RuntimeError("unexpected failure"), + ), patch( + "fenrirscreenreader.remoteDriver.unixDriver.os.path.exists", + return_value=False, + ), patch( + "fenrirscreenreader.remoteDriver.unixDriver.remoteInstanceRegistry.remove_instance" + ) as remove_instance: + with pytest.raises(RuntimeError, match="unexpected failure"): + driver.watch_dog(active, Mock()) + + listener.close.assert_called_once_with() + assert driver.fenrirSocks == [] + assert driver.bound_sockets == [] + remove_instance.assert_called_once_with() + + class FakeClientSocket: def __init__(self, data): self.data = data