fix(mqtt): detect a dead printer connection promptly instead of hanging offline detection forever
All checks were successful
Nightly Build / build (push) Successful in 6m38s
All checks were successful
Nightly Build / build (push) Successful in 6m38s
Discovered while testing the smart-plug power-switch feature: unplugging the printer left the dashboard stuck showing it as online/"ready" indefinitely. Live-tested against a real printer to isolate two independent, compounding causes: 1. The MQTT socket had no TCP keepalive. A connection killed without a clean TCP close (unplugged, not a graceful shutdown) looks alive to the OS for as long as its default dead-connection timeout - 15+ minutes on Linux - since a send on a half-open connection is buffered by the kernel and doesn't fail immediately. Fixed with SO_KEEPALIVE (short idle/interval/count) plus TCP_USER_TIMEOUT, since keepalive probes alone only fire on an idle connection - verified live that a printer disappearing while a send was still in flight (the common case, since the poll loop sends every few seconds) instead falls back to the far slower normal TCP retransmission timer, which keepalive settings don't affect at all. 2. Even after the socket was correctly detected as dead, the status poll loop could hang indefinitely inside publish() waiting for a reconnect attempt already running on the MQTT reader thread (the Issue #105 reconnect-lock serialization), and therefore never reached the is_connected() check that flips kobra_state to "offline". _reconnect() now takes wait_if_in_progress/persist flags so the poll loop's call returns immediately with at most one attempt instead of blocking through someone else's multi-minute backoff loop - persistent retrying stays the reader thread's job. A disconnected printer is now detected and reflected on the dashboard within about 15 seconds. Live-verified across repeated disconnect/ reconnect cycles that no sockets, threads, or file descriptors are left behind (checked via /proc/<pid>/fd and /proc/<pid>/task) - the transient FIN-WAIT-2 entries seen while the printer's TLS service is still booting belong to the kernel's own connection teardown, not to processes held by the bridge, and clear on their own.
This commit is contained in:
@ -92,3 +92,61 @@ def test_reconnect_second_waiter_returns_after_first_completes():
|
||||
|
||||
assert result is True
|
||||
assert c._sock is not None
|
||||
|
||||
|
||||
def test_reconnect_non_blocking_returns_immediately_while_reconnect_in_progress():
|
||||
"""The poll-loop path (publish/publish_web) must NOT block while the reader
|
||||
thread's persistent reconnect is running its multi-minute backoff loop -
|
||||
it has to return so the poll loop can flip kobra_state to "offline".
|
||||
A printer unplugged mid-connection otherwise left the dashboard stuck on
|
||||
the last known state indefinitely (Issue #103 follow-up)."""
|
||||
c = _client()
|
||||
c._running = True
|
||||
started = threading.Event()
|
||||
release = threading.Event()
|
||||
|
||||
def slow_persistent_do_connect():
|
||||
started.set()
|
||||
# Simulate the printer still being gone: never succeeds until released.
|
||||
release.wait(timeout=5.0)
|
||||
raise OSError("still unreachable")
|
||||
|
||||
c._do_connect = slow_persistent_do_connect
|
||||
|
||||
# First reconnect (reader-thread style): persistent, holds the lock, stuck
|
||||
# in backoff.
|
||||
t1 = threading.Thread(target=lambda: c._reconnect(persist=True), daemon=True)
|
||||
t1.start()
|
||||
assert started.wait(timeout=2.0)
|
||||
|
||||
# Poll-loop style call must return basically instantly, not block on t1.
|
||||
t0 = time.time()
|
||||
result = c._reconnect(wait_if_in_progress=False, persist=False)
|
||||
elapsed = time.time() - t0
|
||||
|
||||
assert elapsed < 0.5, f"non-blocking reconnect blocked for {elapsed:.2f}s"
|
||||
assert result is False # socket is down while the other reconnect churns
|
||||
|
||||
release.set() # let the daemon thread unwind
|
||||
|
||||
|
||||
def test_reconnect_one_shot_does_not_loop_on_failure():
|
||||
"""persist=False must attempt the handshake at most once and return,
|
||||
instead of entering the backoff loop (which would block the caller)."""
|
||||
c = _client()
|
||||
c._running = True
|
||||
attempts = []
|
||||
|
||||
def failing_do_connect():
|
||||
attempts.append(1)
|
||||
raise OSError("unreachable")
|
||||
|
||||
c._do_connect = failing_do_connect
|
||||
|
||||
t0 = time.time()
|
||||
result = c._reconnect(persist=False)
|
||||
elapsed = time.time() - t0
|
||||
|
||||
assert result is False
|
||||
assert len(attempts) == 1 # exactly one attempt, no backoff retries
|
||||
assert elapsed < 0.5
|
||||
|
||||
80
tests/test_tcp_keepalive.py
Normal file
80
tests/test_tcp_keepalive.py
Normal file
@ -0,0 +1,80 @@
|
||||
"""TCP keepalive + TCP_USER_TIMEOUT on the MQTT socket (Issue #103 follow-up).
|
||||
|
||||
Without these, a printer that disappears without a clean TCP close
|
||||
(unplugged, not gracefully shut down) leaves the socket looking alive to
|
||||
is_connected() for as long as the OS's default dead-connection timeout -
|
||||
often 15+ minutes on Linux - since sendall() on a half-open connection is
|
||||
buffered by the kernel and doesn't fail immediately. This left the
|
||||
dashboard's printer-state indicator stuck showing the last known state
|
||||
(e.g. green "ready") long after the printer was actually unreachable,
|
||||
reported when testing the smart-plug power-switch feature by physically
|
||||
unplugging the printer.
|
||||
|
||||
Verified live (real printer, physically unplugged) that SO_KEEPALIVE alone
|
||||
is not sufficient: keepalive probes only fire on an idle connection, but if
|
||||
the printer disappears while a send is still unacknowledged - the normal
|
||||
case, since the poll loop is sending every few seconds - the kernel's
|
||||
regular TCP retransmission timer takes over instead (tcp_retries2, 13-30+
|
||||
minutes on Linux), which keepalive settings don't affect. TCP_USER_TIMEOUT
|
||||
closes that gap by capping how long ANY unacknowledged data may sit in the
|
||||
send queue, regardless of which retry mechanism would otherwise still be
|
||||
running.
|
||||
"""
|
||||
import socket
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from kobrax_client import _enable_tcp_keepalive
|
||||
|
||||
|
||||
def test_enable_tcp_keepalive_sets_so_keepalive():
|
||||
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
|
||||
try:
|
||||
_enable_tcp_keepalive(s)
|
||||
assert s.getsockopt(socket.SOL_SOCKET, socket.SO_KEEPALIVE) == 1
|
||||
finally:
|
||||
s.close()
|
||||
|
||||
|
||||
@pytest.mark.skipif(not hasattr(socket, "TCP_KEEPIDLE"), reason="Linux-specific option")
|
||||
def test_enable_tcp_keepalive_sets_short_idle_and_interval():
|
||||
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
|
||||
try:
|
||||
_enable_tcp_keepalive(s)
|
||||
idle = s.getsockopt(socket.IPPROTO_TCP, socket.TCP_KEEPIDLE)
|
||||
intvl = s.getsockopt(socket.IPPROTO_TCP, socket.TCP_KEEPINTVL)
|
||||
cnt = s.getsockopt(socket.IPPROTO_TCP, socket.TCP_KEEPCNT)
|
||||
# Short enough that a dead connection is detected within a couple of
|
||||
# poll cycles (default poll_interval is 3s), not the OS default of
|
||||
# minutes.
|
||||
assert idle <= 10
|
||||
assert intvl <= 5
|
||||
assert cnt <= 5
|
||||
finally:
|
||||
s.close()
|
||||
|
||||
|
||||
@pytest.mark.skipif(not hasattr(socket, "TCP_USER_TIMEOUT"), reason="Linux-specific option")
|
||||
def test_enable_tcp_keepalive_sets_user_timeout():
|
||||
"""The critical fix, verified live against a real printer: without this,
|
||||
a dead connection with unacknowledged data in flight is only detected
|
||||
after the OS's normal TCP retransmission timeout (13-30+ minutes on
|
||||
Linux), not the keepalive interval."""
|
||||
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
|
||||
try:
|
||||
_enable_tcp_keepalive(s)
|
||||
user_timeout_ms = s.getsockopt(socket.IPPROTO_TCP, socket.TCP_USER_TIMEOUT)
|
||||
# Short enough that a dead connection with in-flight data is detected
|
||||
# within a couple of poll cycles, not tens of minutes.
|
||||
assert 0 < user_timeout_ms <= 20000
|
||||
finally:
|
||||
s.close()
|
||||
|
||||
|
||||
def test_enable_tcp_keepalive_does_not_raise_on_unsupported_platform():
|
||||
"""A platform without TCP_KEEPIDLE/INTVL/CNT (e.g. some Windows builds)
|
||||
must not crash the connection attempt - keepalive is best-effort."""
|
||||
s = MagicMock()
|
||||
s.setsockopt.side_effect = OSError("unsupported")
|
||||
_enable_tcp_keepalive(s) # must not raise
|
||||
Reference in New Issue
Block a user