From 9011baa3ebe342c56746ba7c6f5bb99e78d7a230 Mon Sep 17 00:00:00 2001 From: nightjoker7 Date: Wed, 22 Apr 2026 19:08:32 -0500 Subject: [PATCH] StreamInterface: prevent socket/reader-thread leak on handshake failure in __init__ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If connect() or waitForConfig() raises during __init__ (handshake timeout, bad stream, config error), the reader thread started by connect() keeps running and the underlying stream/socket stays open — but the caller never receives a reference to the half-initialized instance, so they cannot call close() themselves. The leak compounds on every retry from a caller's reconnect loop. Fix: wrap connect() + waitForConfig() in try/except; call self.close() on any exception before re-raising. Also guard close() against RuntimeError from joining an unstarted reader thread (happens when close() runs from a failed __init__ before connect() could spawn it). Discovered while debugging a real-world Meshtastic firmware crash where a passive logger's retrying TCPInterface() calls against a node with 250-entry NodeDB produced a reconnect storm — every retry triggered a full config+NodeDB dump on the node, compounding heap pressure, which then exposed null-deref bugs in Router::perhapsDecode / MeshService (firmware side fixed in meshtastic/firmware#10226 and #10229). The client-side leak is independent of those firmware bugs and worth fixing on its own. --- meshtastic/stream_interface.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/meshtastic/stream_interface.py b/meshtastic/stream_interface.py index 06ee28a3a..1e7d34038 100644 --- a/meshtastic/stream_interface.py +++ b/meshtastic/stream_interface.py @@ -1,5 +1,6 @@ """Stream Interface base class """ +import contextlib import io import logging import threading @@ -61,9 +62,20 @@ def __init__( # pylint: disable=R0917 # Start the reader thread after superclass constructor completes init if connectNow: - self.connect() - if not noProto: - self.waitForConfig() + try: + self.connect() + if not noProto: + self.waitForConfig() + except Exception: + # Handshake failed (timeout, config error, bad stream). The caller + # never receives a reference to this half-initialized instance, so + # they cannot call close() themselves. If we don't clean up here, + # the reader thread (already started by connect()) keeps running + # and the underlying stream/socket leaks — the leak compounds on + # every retry from the caller's reconnect loop. + with contextlib.suppress(Exception): + self.close() + raise def connect(self) -> None: """Connect to our radio @@ -136,7 +148,13 @@ def close(self) -> None: # reader thread to close things for us self._wantExit = True if self._rxThread != threading.current_thread(): - self._rxThread.join() # wait for it to exit + try: + self._rxThread.join() # wait for it to exit + 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 def _handleLogByte(self, b): """Handle a byte that is part of a log message from the device."""