Merge pull request #101 from joeyleake/fix/95-serial-fd-leak-on-timeout

Fix serial fd leak in SerialConnection.connect() on timeout

src/meshcore/serial_cx.py

@@ -71,14 +71,24 @@ class SerialConnection:
         self._connected_event.clear()
 
         loop = asyncio.get_running_loop()
-        await serial_asyncio.create_serial_connection(
+        transport, _ = await serial_asyncio.create_serial_connection(
             loop,
             lambda: self.MCSerialClientProtocol(self),
             self.port,
             baudrate=self.baudrate,
         )
 
+        try:
             await asyncio.wait_for(self._connected_event.wait(), timeout=timeout)
+        except Exception:
+            # create_serial_connection() already opened the port's fds;
+            # connection_made() never fired (or didn't in time) to hand them
+            # to self.transport, so close directly on the local reference or
+            # they leak until the process runs out of fds (#95).
+            if self.transport is transport:
+                self.transport = None
+            transport.close()
+            raise
         logger.info("Serial Connection started")
         return self.port
 

tests/unit/test_serial_connection.py

@@ -1,4 +1,5 @@
 import asyncio
+from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
 
@@ -29,3 +30,28 @@ async def test_handle_rx_discards_leading_junk_before_frame_start():
     assert conn.header == b""
     assert conn.inframe == b""
     assert conn.frame_expected_size == 0
+
+
+@pytest.mark.asyncio
+async def test_connect_closes_transport_on_timeout():
+    """Regression for #95: if connection_made() never fires before the
+    connect() timeout, the fds create_serial_connection() already opened
+    must not leak -- connect() should close the transport before raising."""
+    conn = SerialConnection("/dev/null", 115200)
+
+    mock_transport = MagicMock()
+    mock_protocol = MagicMock()
+
+    async def fake_create_serial_connection(loop, protocol_factory, port, baudrate):
+        # Never call connection_made(), so _connected_event stays unset.
+        return mock_transport, mock_protocol
+
+    with patch(
+        "meshcore.serial_cx.serial_asyncio.create_serial_connection",
+        side_effect=fake_create_serial_connection,
+    ):
+        with pytest.raises(asyncio.TimeoutError):
+            await conn.connect(timeout=0.05)
+
+    mock_transport.close.assert_called_once()
+    assert conn.transport is None