Half-close TCP connections so the last write is not lost

close() went straight to shutdown(SHUT_RDWR) + close(). Doing that while either
side still has unread data makes the stack send RST rather than FIN, and the
device's FromRadio stream means there is essentially always unread data.

Winsock discards data that was received but not yet read when an RST arrives, so
an admin message written moments earlier is thrown away before the device reads
it. Linux delivers the buffered bytes before reporting ECONNRESET, which is why
this only surfaced on Windows: every one-shot TCP write there (--set, --seturl,
--sendtext) reported success and did nothing.

writeConfig() does not wait for an ack, so close() runs microseconds after
sendall(). The device cannot compensate; it polls every 5ms and the RST beats its
next read.

Send FIN first with shutdown(SHUT_WR), which lets the device consume what we
wrote, wait briefly for the reader thread to drain and exit, then tear down
exactly as before. The full shutdown is kept so a reader still blocked in recv()
is unblocked for the join in StreamInterface.close(). The wait is bounded: the
device is not obliged to close just because we half-closed.

Fixes #957
This commit is contained in:
Thomas Göttgens
2026-07-17 14:34:18 +02:00
parent 82220ba49b
commit f623732694
2 changed files with 82 additions and 0 deletions

View File

@@ -1,6 +1,7 @@
"""Meshtastic unit tests for tcp_interface.py"""
import re
import socket
from unittest.mock import MagicMock, patch
import pytest
@@ -78,6 +79,56 @@ def test_TCPInterface_close_shutdowns_socket_before_super_close():
assert iface.socket is None
@pytest.mark.unit
def test_TCPInterface_close_half_closes_before_shutdown():
"""Close should send FIN and let the device drain before forcing the link down.
Going straight to shutdown(SHUT_RDWR)/close() with data still unread makes the
stack send RST, and Winsock then discards data the device had received but not
yet read, silently losing writes such as `--set`.
"""
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.object(
TCPInterface, "_wait_for_reader_exit", autospec=True
) as mock_wait:
with patch(
"meshtastic.stream_interface.StreamInterface.close", autospec=True
) as mock_super_close:
sock.shutdown.side_effect = lambda how: call_order.append(f"shutdown_{how}")
mock_wait.side_effect = lambda _self, _t: call_order.append("wait")
mock_shutdown.side_effect = lambda _self: call_order.append("full_shutdown")
mock_super_close.side_effect = lambda _self: call_order.append("super_close")
iface.close()
assert call_order == [
f"shutdown_{socket.SHUT_WR}",
"wait",
"full_shutdown",
"super_close",
]
@pytest.mark.unit
def test_TCPInterface_close_survives_half_close_failure():
"""A peer that already vanished must not break close()."""
iface = TCPInterface(hostname="localhost", noProto=True, connectNow=False)
sock = MagicMock()
sock.shutdown.side_effect = OSError("already gone")
iface.socket = sock
with patch("meshtastic.stream_interface.StreamInterface.close", autospec=True):
iface.close() # must not raise
sock.close.assert_called_once()
assert iface.socket is None
@pytest.mark.unit
def test_TCPInterface_reconnect():
"""Test that _reconnect correctly reconnects"""