Skip to content

Discard the late response of a cancelled request - #3076

Open
sankalpsthakur wants to merge 1 commit into
pymodbus-dev:devfrom
sankalpsthakur:fix/3070-late-response
Open

sankalpsthakur wants to merge 1 commit into
pymodbus-dev:devfrom
sankalpsthakur:fix/3070-late-response

Conversation

@sankalpsthakur

Copy link
Copy Markdown
Contributor

When an async execute() is cancelled while it waits, the device can still answer after the next request is sent. With RTU/ASCII, which have no transaction id, and the same function code, the next request then returns that stale data, as in #3070. #3072 drops a late response only if it arrives before the next request or has another function code. On cancel, execute() now keeps a fresh response future, and the next execute() waits for it before sending, at most timeout_connect after the cancel. Socket framers are unchanged (handleFrame already skips an old transaction id), CancelledError still propagates as in #3010, and there is no API change.

Validation

  • test_transaction_execute_cancel (new, FramerRTU): cancel a 4-register read, then read 5 registers. With late_response=True the 4-register reply arrives during the second call: on dev (c37dd72, with Partial solve ext. cancel sideeffects. #3072) it fails with assert [1, 2, 3, 4] == [1, 2, 3, 4, 5], and with this change it passes. late_response=False (the reply never comes) passes on both.
  • pytest test/transaction: 47 passed on Python 3.10.18 and 3.14.7 (macOS), with 100% line and branch coverage of pymodbus/transaction and test/transaction (3.14).
  • ruff check ., ruff format --check ., codespell and zuban check on the touched files are clean, and pylint reports nothing new. execute() was already at the C901 limit of 10, so it gets # noqa: C901, like convert_from_registers.

Fixes #3070

AI disclosure: written with an AI coding agent and tested as described above.

When execute() is cancelled while it waits, the response can still
arrive after the next request is sent. RTU and ASCII responses carry
no transaction id, so the next request returned it as its own.

On cancel, keep a fresh response future, and let the next execute()
wait for it before it sends, at most timeout_connect after the cancel.
Socket framers are unchanged: handleFrame already skips a response
with an old transaction id.
@janiversen

Copy link
Copy Markdown
Collaborator

I just tried your test case, as expected it shows the problem, so with that I will try

  • revert the 2 PRs that have changed cancelation, and ensure the test passes.
  • if so, then fix the PRs.

I do suspect however that the problem might be older, because I can produce the problem without cancelling:

  1. App sends a request with timeout.
  2. No response is received before timeout, and app receives a ModbusIOException.
  3. App sends new request
  4. Device responds to first request, which are returned as response to second request
  5. Device responds to second request, which are discarded

This is of course a configuration problem (timeout too small), but the effect seems identical to cancelling.

I will keep you informed of my findings, because your method to solve might be the best one.

@janiversen

janiversen commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Used your test case on v3.15.0

modified line 321:

   with pytest.raises(ModbusIOException):

to adapt to the non passing of cancel exception.

Your test FAILS, meaning this is as suspected a very old problem.

Your fix raises a big problem, because it should be allowed to send a request to another device on the same line, without delay:

Ex 1:

  • App sends request1, dev_id 1
  • App cancels request1
  • App sends request2, dev_id 2
  • receive response1 (discarded because dev_id not matching)
  • receive response2, passed to app

Ex 2:

  • App sends request1, dev_id 1
  • App cancels request1
  • App sends request2, dev_id 2
  • receive response2, passed to app
  • App sends request1, dev_id 1
  • wait for timeout
  • receive response1 within wait time (discarded because no request outstanding)
  • receive response2, passed to app

If we get those 2 examples working, the only outstanding problem is if timeout is configured too low:

  • App sends request1, dev_id 1
  • App cancels request1
  • App sends request2, dev_id 1
  • wait for timeout
  • receive response1, after timeout, passed to app as response2

This seems theoretically insolvable, but it is a configuration problem.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Async cancellation can lead to incorrect register data being returned on subsequent calls

2 participants