AcademySoftwareFoundation / AcademySoftwareFoundation/xstudio
`Connection.response()` raises `TimeoutError` when the response has already arrived
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 773
- Forks
- 129
- Avg merge
- 9d 1m
- Merged PRs (30d)
- 10
Description
Connection.response() raises TimeoutError when the response has already arrived
Describe the bug
Connection.response() can report a timeout for a request whose answer has
already been received and correctly stored.
_dequeue_messages files every response it dequeues into self.responses,
whatever request it was watching for — only the break out of its loop is
specific to watch_for (python/src/xstudio/connection/__init__.py:490-493):
req_id = msg[-2]
if req_id in self.responses:
self.responses[req_id] = msg # filed regardless of watch_for
if req_id == watch_for:
break # only the break is specific
else:
self.handle_broadcast(msg)
response() then lets TimeoutError propagate without consulting
self.responses (:401-406):
def response(self, req_id, timeout_milli=None):
if timeout_milli is not None:
self._dequeue_messages(timeout_milli, req_id) # may raise
return self._response(req_id) # never reached
So when any other consumer of the connection's queue takes our response first,
the answer is stored correctly and the caller is told the request timed out.
Not having dequeued it ourselves is being treated as no answer arriving.
To Reproduce
from xstudio.connection import Connection
from xstudio.core import version_atom
conn = Connection(auto_connect=True)
req_id = conn.request(conn.remote(), version_atom())
# Any other consumer of this connection's queue. Doing it on this thread
# removes the race without changing the mechanism: dequeue_messages() pumps
# with watch_for unset, so it files the response and carries on rather than
# breaking on it.
conn.dequeue_messages(300)
assert conn.responses[req_id] is not None # the answer IS here
conn.response(req_id, 300) # ...and this raises TimeoutError
Expected behavior
TimeoutError should be raised only when no response for that request has been
recorded. Where one has been recorded — by any consumer of the queue — it should
be returned.
Impact
Three ways this shows up, all measured on develop @ cb0e9b6b:
Connection(background_processing=True)is effectively unusable. It
starts exactly such a consumer viastart_background_processing→
process_events_forever. With one running, 20 of 20 bounded reads raised
TimeoutErrorwith the answer already sitting inself.responses.- A request issued from inside a broadcast callback, with no threads
involved at all. The nestedrequest_receivere-enters the same pump on the
calling thread, so an outer frame files the answer while the inner call
concludes it timed out. Measured 1 in 20. - The deterministic single-threaded case above.
It matters beyond a spurious exception: callers treat TimeoutError as evidence
that an actor is unresponsive and act on it — dropping cached handles,
re-acquiring, marking a peer unhealthy.
Desktop
- OS: macOS 15 (arm64)
- xSTUDIO version:
develop@cb0e9b6b, built from source - Python 3.11 (bundled interpreter)
Additional context
A fix and a regression test are in the linked PR. The same investigation turned
up a second, independent defect on the same code path — a read's latency being
charged for dispatching unrelated broadcasts — filed separately, since it is
architectural and has no small fix.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in python/src/xstudio/connection/init.py at response() around lines 401-406 and _dequeue_messages around lines 490-493, then run the deterministic reproduction from the issue. Done means a response already stored in self.responses is returned rather than reported as a timeout, with the regression test mentioned in the linked PR passing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend, networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100