From 71ae2d8f1e69b66f0f005f60ef1dc4e535fefc23 Mon Sep 17 00:00:00 2001 From: Yuvrajup Date: Thu, 10 Sep 2026 01:15:24 +0530 Subject: [PATCH 1/5] fix(async): synchronize connect() promise with connection_made callback --- pymodbus/transport/transport.py | 10 ++++++++++ test/client/test_client.py | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/pymodbus/transport/transport.py b/pymodbus/transport/transport.py index c92352ed1..a3cb18357 100644 --- a/pymodbus/transport/transport.py +++ b/pymodbus/transport/transport.py @@ -136,6 +136,7 @@ def __init__( if is_sync: return self.loop = asyncio.get_running_loop() + self.connected_evt = asyncio.Event() if self.is_server: if self.comm_params.source_address is not None: host = self.comm_params.source_address[0] @@ -218,11 +219,16 @@ async def connect(self) -> bool: """Handle generic connect and call on to specific transport connect.""" Log.debug("Connecting {}", self.comm_params.comm_name) self.is_closing = False + self.connected_evt.clear() try: self.transport, _protocol = await asyncio.wait_for( self.call_create(), timeout=self.comm_params.timeout_connect, ) + await asyncio.wait_for( + self.connected_evt.wait(), + timeout=self.comm_params.timeout_connect, + ) except (asyncio.TimeoutError, OSError) as exc: # pylint: disable=overlapping-except Log.warning("Failed to connect {}", exc) return False @@ -256,12 +262,16 @@ def connection_made(self, transport: asyncio.BaseTransport) -> None: self.transport = transport self.reset_delay() self.callback_connected() + self.connected_evt.set() def connection_lost(self, exc: Exception | None) -> None: """Call from asyncio, when the connection is lost or closed. :param exc: None or an exception object """ + if not getattr(self, "is_sync", False) and hasattr(self, "connected_evt"): + self.connected_evt.clear() + if not self.transport or self.is_closing: return Log.debug("Connection lost {} due to {}", self.comm_params.comm_name, exc) diff --git a/test/client/test_client.py b/test/client/test_client.py index b1676ccae..591e15577 100755 --- a/test/client/test_client.py +++ b/test/client/test_client.py @@ -610,6 +610,31 @@ async def test_client_connection_made(self): client.close() assert rc + async def test_async_connect_state_machine(self): + """Test async client state-machine hydration after connect.""" + client = lib_client.AsyncModbusTcpClient("127.0.0.1") + assert not client.connected + + # Mock create_connection to return transport and protocol but also simulate calling connection_made + transport_mock = mock.AsyncMock() + transport_mock.close = lambda: () + + async def mock_create_connection(): + client.ctx.connection_made(transport_mock) + return transport_mock, client.ctx + + client.ctx.call_create = mock_create_connection + + # Override the normal client connect behavior to ensure we use our mocked create_connection + connected = await client.connect() + assert connected is True + + # This is the crux of the fix: client.connected MUST be true right after await client.connect() + assert client.connected is True + assert client.ctx.transport is not None + + client.close() + async def test_client_base_async(self): """Test modbus base client class.""" async with ModbusBaseClient( From 72a1bcbb300bf4d2694ea8efd9a44236bcbe6248 Mon Sep 17 00:00:00 2001 From: Yuvrajup Date: Thu, 10 Sep 2026 01:28:00 +0530 Subject: [PATCH 2/5] test: update mocks to simulate connection_made for test resilience --- pymodbus/transport/transport.py | 9 +++++---- test/transport/test_protocol.py | 16 ++++++++++++++-- test/transport/test_reconnect.py | 9 +++++++-- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/pymodbus/transport/transport.py b/pymodbus/transport/transport.py index a3cb18357..f3e91a676 100644 --- a/pymodbus/transport/transport.py +++ b/pymodbus/transport/transport.py @@ -225,10 +225,11 @@ async def connect(self) -> bool: self.call_create(), timeout=self.comm_params.timeout_connect, ) - await asyncio.wait_for( - self.connected_evt.wait(), - timeout=self.comm_params.timeout_connect, - ) + if self.transport: + await asyncio.wait_for( + self.connected_evt.wait(), + timeout=self.comm_params.timeout_connect, + ) except (asyncio.TimeoutError, OSError) as exc: # pylint: disable=overlapping-except Log.warning("Failed to connect {}", exc) return False diff --git a/test/transport/test_protocol.py b/test/transport/test_protocol.py index 7ff2f8456..7c38d1331 100644 --- a/test/transport/test_protocol.py +++ b/test/transport/test_protocol.py @@ -61,7 +61,13 @@ async def test_init_source_addr_none(self, use_clc, dummy_protocol): async def test_loop_connect(self, client, dummy_protocol): """Test properties.""" - client.call_create = mock.AsyncMock(return_value=(dummy_protocol(), None)) + + async def mock_call_create(): + prot = dummy_protocol() + client.connection_made(prot) + return (prot, None) + + client.call_create = mock.AsyncMock(side_effect=mock_call_create) assert await client.connect() async def test_loop_listen(self, server, dummy_protocol): @@ -73,7 +79,13 @@ async def test_loop_listen(self, server, dummy_protocol): async def test_connect_ok(self, client, dummy_protocol): """Test properties.""" - client.call_create = mock.AsyncMock(return_value=(dummy_protocol(), None)) + + async def mock_call_create(): + prot = dummy_protocol() + client.connection_made(prot) + return (prot, None) + + client.call_create = mock.AsyncMock(side_effect=mock_call_create) assert await client.connect() async def test_connect_not_ok(self, client, dummy_protocol): diff --git a/test/transport/test_reconnect.py b/test/transport/test_reconnect.py index c959f44b4..35950554e 100644 --- a/test/transport/test_reconnect.py +++ b/test/transport/test_reconnect.py @@ -64,9 +64,14 @@ async def test_multi_reconnect_call(self, client): async def test_reconnect_call_ok(self, client): """Test connection_lost().""" client.loop = asyncio.get_running_loop() - client.call_create = mock.AsyncMock(return_value=(mock.Mock(), mock.Mock())) + + async def mock_call_create(): + transport = mock.Mock() + client.connection_made(transport) + return (transport, mock.Mock()) + + client.call_create = mock.AsyncMock(side_effect=mock_call_create) await client.connect() - client.connection_made(mock.Mock()) client.connection_lost(RuntimeError("Connection lost")) await asyncio.sleep(client.reconnect_delay_current * 1.8) assert client.call_create.call_count == 2 From 6c70a2374b3a8dc999c94cededbf46ded827f089 Mon Sep 17 00:00:00 2001 From: Yuvrajup Date: Thu, 10 Sep 2026 01:36:25 +0530 Subject: [PATCH 3/5] test: fix zuban type checker unreachable statement issue --- test/client/test_client.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/client/test_client.py b/test/client/test_client.py index 591e15577..1199b9826 100755 --- a/test/client/test_client.py +++ b/test/client/test_client.py @@ -613,8 +613,7 @@ async def test_client_connection_made(self): async def test_async_connect_state_machine(self): """Test async client state-machine hydration after connect.""" client = lib_client.AsyncModbusTcpClient("127.0.0.1") - assert not client.connected - + # Mock create_connection to return transport and protocol but also simulate calling connection_made transport_mock = mock.AsyncMock() transport_mock.close = lambda: () @@ -627,10 +626,10 @@ async def mock_create_connection(): # Override the normal client connect behavior to ensure we use our mocked create_connection connected = await client.connect() - assert connected is True - + assert connected + # This is the crux of the fix: client.connected MUST be true right after await client.connect() - assert client.connected is True + assert client.connected assert client.ctx.transport is not None client.close() From 6e871ff60d7cce32715fd91a221617dacae67ea4 Mon Sep 17 00:00:00 2001 From: Yuvrajup Date: Thu, 10 Sep 2026 01:44:48 +0530 Subject: [PATCH 4/5] style: fix trailing whitespace in test_client.py for ruff check --- test/client/test_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/client/test_client.py b/test/client/test_client.py index 1199b9826..c3b4488e9 100755 --- a/test/client/test_client.py +++ b/test/client/test_client.py @@ -613,7 +613,7 @@ async def test_client_connection_made(self): async def test_async_connect_state_machine(self): """Test async client state-machine hydration after connect.""" client = lib_client.AsyncModbusTcpClient("127.0.0.1") - + # Mock create_connection to return transport and protocol but also simulate calling connection_made transport_mock = mock.AsyncMock() transport_mock.close = lambda: () @@ -627,7 +627,7 @@ async def mock_create_connection(): # Override the normal client connect behavior to ensure we use our mocked create_connection connected = await client.connect() assert connected - + # This is the crux of the fix: client.connected MUST be true right after await client.connect() assert client.connected assert client.ctx.transport is not None From 00ba44deb383675af7aeb93b67cf69e5b7c4f7a3 Mon Sep 17 00:00:00 2001 From: Yuvrajup Date: Thu, 10 Sep 2026 23:21:46 +0530 Subject: [PATCH 5/5] test: add deterministic test case proving async connection race condition --- pymodbus.log | 81 ++++++++++++++++++++++++++++++++++++++ test/client/test_client.py | 33 ++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 pymodbus.log diff --git a/pymodbus.log b/pymodbus.log new file mode 100644 index 000000000..b37f59a4a --- /dev/null +++ b/pymodbus.log @@ -0,0 +1,81 @@ +2026-09-10 01:22:16,121 DEBUG test_logging:69 test 1 +2026-09-10 01:22:16,122 DEBUG test_logging:70 Repeating.... +2026-09-10 01:22:16,122 ERROR test_logging:72 get frames +2026-09-10 01:22:16,123 CRITICAL test_logging:73 Repeating.... +2026-09-10 01:22:16,123 WARNING test_logging:77 test 2 +2026-09-10 01:22:16,124 WARNING test_logging:78 Repeating.... +2026-09-10 01:22:16,125 CRITICAL test_logging:81 test 3no +>>>>> send: 0x61 0x62 0x63 +>>>>> Repeating.... +>>>>> None +2026-09-10 01:22:16,125 CRITICAL test_logging:83 test 3 +2026-09-10 01:22:16,125 CRITICAL test_logging:84 Repeating.... +2026-09-10 01:22:16,126 ERROR test_logging:89 test 4 +2026-09-10 01:22:16,126 ERROR test_logging:90 Repeating.... +2026-09-10 01:22:16,127 INFO test_logging:95 test 5 +2026-09-10 01:22:16,127 INFO test_logging:96 Repeating.... +2026-09-10 01:22:16,594 DEBUG test_logging:136 send: 0x31 0x32 0x33 +2026-09-10 01:22:16,859 WARNING context:65 ModbusDeviceContext, ModbusSequentialDataBlock, ModbusSparseDataBlock are deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:16,860 WARNING context:120 ModbusServerContext is deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:17,343 WARNING context:65 ModbusDeviceContext, ModbusSequentialDataBlock, ModbusSparseDataBlock are deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:17,344 WARNING context:120 ModbusServerContext is deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:17,466 WARNING context:65 ModbusDeviceContext, ModbusSequentialDataBlock, ModbusSparseDataBlock are deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:17,468 WARNING context:120 ModbusServerContext is deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:17,472 INFO base:92 Server listening. +2026-09-10 01:22:17,472 INFO base:92 Repeating.... +2026-09-10 01:22:17,473 INFO base:96 Server graceful shutdown. +2026-09-10 01:22:17,591 WARNING context:65 ModbusDeviceContext, ModbusSequentialDataBlock, ModbusSparseDataBlock are deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:17,592 WARNING context:120 ModbusServerContext is deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:17,710 WARNING context:65 ModbusDeviceContext, ModbusSequentialDataBlock, ModbusSparseDataBlock are deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:17,711 WARNING context:120 ModbusServerContext is deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:17,831 WARNING context:65 ModbusDeviceContext, ModbusSequentialDataBlock, ModbusSparseDataBlock are deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:17,832 WARNING context:120 ModbusServerContext is deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:17,954 WARNING context:65 ModbusDeviceContext, ModbusSequentialDataBlock, ModbusSparseDataBlock are deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:17,955 WARNING context:120 ModbusServerContext is deprecated and will be removed in v4. +Please convert to SimData/SimDevice. +Please read https://pymodbus.readthedocs.io/en/dev/source/upgrade_40.html#convert-to-simdata-simdevice +2026-09-10 01:22:18,811 DEBUG base:75 Processing: 0x0 0x1 0x0 0x0 0x0 0x5 0x1 0x4 0x2 0x0 0x3 +2026-09-10 01:22:18,812 DEBUG decoders:86 decoded PDU function_code(4 sub -1) -> ReadInputRegistersResponse(dev_id=0, transaction_id=0, address=0, count=0, bits=[], registers=[3], status=1, retries=0) +2026-09-10 01:22:18,813 DEBUG base:75 Processing: 0x0 0x1 0x0 0x0 0x0 0x5 0x1 0x4 0x2 0x0 0x3 +2026-09-10 01:22:18,814 ERROR base:86 ERROR: request ask for transaction_id=2 but got id=1, Skipping. +2026-09-10 01:22:18,815 ERROR transaction:180 No response received after 3 retries, continue with next request +2026-09-10 01:24:38,019 DEBUG test_logging:69 test 1 +2026-09-10 01:24:38,021 DEBUG test_logging:70 Repeating.... +2026-09-10 01:24:38,021 ERROR test_logging:72 get frames +2026-09-10 01:24:38,027 CRITICAL test_logging:73 Repeating.... +2026-09-10 01:24:38,027 WARNING test_logging:77 test 2 +2026-09-10 01:24:38,028 WARNING test_logging:78 Repeating.... +2026-09-10 01:24:38,028 CRITICAL test_logging:81 test 3no +2026-09-10 01:24:38,028 CRITICAL test_logging:83 test 3 +2026-09-10 01:24:38,029 CRITICAL test_logging:84 Repeating.... +2026-09-10 01:24:38,030 ERROR test_logging:89 test 4 +2026-09-10 01:24:38,030 ERROR test_logging:90 Repeating.... +2026-09-10 01:24:38,030 INFO test_logging:95 test 5 +2026-09-10 01:24:38,031 INFO test_logging:96 Repeating.... +2026-09-10 01:24:38,494 DEBUG test_logging:136 send: 0x31 0x32 0x33 diff --git a/test/client/test_client.py b/test/client/test_client.py index c3b4488e9..cada5a564 100755 --- a/test/client/test_client.py +++ b/test/client/test_client.py @@ -1,5 +1,6 @@ """Test client sync.""" +import asyncio import socket import ssl from typing import cast @@ -631,6 +632,38 @@ async def mock_create_connection(): # This is the crux of the fix: client.connected MUST be true right after await client.connect() assert client.connected assert client.ctx.transport is not None + client.close() + + async def test_async_connect_race_condition(self): + """Test that client.connect() correctly waits for connection_made callback even if delayed. + + This demonstrates why relying solely on `create_connection`'s return (the old behavior) + results in `client.connected == False` if the callback is delayed by the event loop. + """ + client = lib_client.AsyncModbusTcpClient("127.0.0.1") + + transport_mock = mock.AsyncMock() + transport_mock.close = lambda: () + + async def mock_create_connection(): + # Artificially delay the firing of protocol.connection_made(transport) + async def delayed_connection_made(): + await asyncio.sleep(0.01) + client.ctx.connection_made(transport_mock) + + _task = asyncio.create_task(delayed_connection_made()) # noqa: RUF006 + return transport_mock, client.ctx + + client.ctx.call_create = mock_create_connection + + # connect() should await until connection_made fires, rather than just returning immediately + connected = await client.connect() + assert connected + + # With the new synchronization logic, await client.connect() successfully waits for the delayed + # connection_made, resulting in client.connected == True. + assert client.connected + assert client.ctx.transport is not None client.close()