mirror of
https://github.com/meshtastic/python.git
synced 2026-06-02 12:45:00 -04:00
Fix some leaks/hangs on close: unstarted StreamInterface streams & TCP reader unblock
This commit is contained in:
@@ -149,8 +149,11 @@ class StreamInterface(MeshInterface):
|
||||
except RuntimeError:
|
||||
# Thread was never started — happens when close() is invoked
|
||||
# from a failed __init__ before connect() could spawn it.
|
||||
# Nothing to join; safe to ignore.
|
||||
pass
|
||||
# In this case there is no reader thread to close the stream.
|
||||
if self.stream is not None:
|
||||
with contextlib.suppress(Exception):
|
||||
self.stream.close()
|
||||
self.stream = None
|
||||
|
||||
def _handleLogByte(self, b):
|
||||
"""Handle a byte that is part of a log message from the device."""
|
||||
|
||||
@@ -74,16 +74,17 @@ class TCPInterface(StreamInterface):
|
||||
def close(self) -> None:
|
||||
"""Close a connection to the device"""
|
||||
logger.debug("Closing TCP stream")
|
||||
super().close()
|
||||
# Sometimes the socket read might be blocked in the reader thread.
|
||||
# Therefore we force the shutdown by closing the socket here
|
||||
# Therefore force a shutdown first to unblock reader thread reads.
|
||||
self._wantExit = True
|
||||
if self.socket is not None:
|
||||
with contextlib.suppress(Exception): # Ignore errors in shutdown, because we might have a race with the server
|
||||
self._socket_shutdown()
|
||||
self.socket.close()
|
||||
with contextlib.suppress(Exception):
|
||||
self.socket.close()
|
||||
|
||||
self.socket = None
|
||||
super().close()
|
||||
|
||||
def _writeBytes(self, b: bytes) -> None:
|
||||
"""Write an array of bytes to our stream and flush"""
|
||||
|
||||
@@ -33,6 +33,17 @@ def test_StreamInterface_close_safe_when_thread_never_started():
|
||||
iface.close()
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
@pytest.mark.usefixtures("reset_mt_config")
|
||||
def test_StreamInterface_close_when_thread_never_started_closes_stream():
|
||||
"""If no reader thread was started, close() should still close the stream."""
|
||||
iface = StreamInterface(noProto=True, connectNow=False)
|
||||
stream = MagicMock()
|
||||
iface.stream = stream
|
||||
iface.close()
|
||||
stream.close.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
@pytest.mark.usefixtures("reset_mt_config")
|
||||
def test_StreamInterface_init_cleans_up_when_connect_raises():
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
"""Meshtastic unit tests for tcp_interface.py"""
|
||||
|
||||
import re
|
||||
from unittest.mock import patch
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -54,3 +54,25 @@ def test_TCPInterface_without_connecting():
|
||||
with patch("socket.socket"):
|
||||
iface = TCPInterface(hostname="localhost", noProto=True, connectNow=False)
|
||||
assert iface.socket is None
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_TCPInterface_close_shutdowns_socket_before_super_close():
|
||||
"""Close should unblock socket reads before waiting on StreamInterface.close()."""
|
||||
iface = TCPInterface(hostname="localhost", noProto=True, connectNow=False)
|
||||
sock = MagicMock()
|
||||
iface.socket = sock
|
||||
call_order = []
|
||||
|
||||
with patch.object(TCPInterface, "_socket_shutdown", autospec=True) as mock_shutdown:
|
||||
with patch(
|
||||
"meshtastic.stream_interface.StreamInterface.close", autospec=True
|
||||
) as mock_super_close:
|
||||
mock_shutdown.side_effect = lambda _self: call_order.append("shutdown")
|
||||
mock_super_close.side_effect = lambda _self: call_order.append("super_close")
|
||||
|
||||
iface.close()
|
||||
|
||||
assert call_order == ["shutdown", "super_close"]
|
||||
sock.close.assert_called_once()
|
||||
assert iface.socket is None
|
||||
|
||||
Reference in New Issue
Block a user