Fix Unix remote watchdog socket leak
This commit is contained in:
@@ -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()
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user