Fix async websocket reader starvation and dispatch races - #249
Open
adamlogan73 wants to merge 1 commit into
Open
adamlogan73 wants to merge 1 commit into
adamlogan73 wants to merge 1 commit into
Conversation
Replace the racing-recv() polling loop with a single background reader task that is the only thing allowed to touch the socket. recv() calls now wait on a shared Condition and get notified per dispatched message, instead of each concurrently calling _async_recv() and starving each other. Errors are routed to the specific msg_id they belong to when attributable, and broadcast to all waiters only when the connection itself is dead. Bump nimax to 1.1.1 and enable id-aware WebSocket replay gating so test cassettes correctly model concurrent, out-of-order request/response correlation instead of relying on positional ordering.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
recv()polling loop with a single background reader task that is the only thing allowed to touch the socket, coordinated viaasyncio.Conditioninstead of racing_async_recv()callsmsg_idthey belong to when attributable (e.g. an HA error response for a known request), and only broadcast to every pending waiter when the connection itself is deadnimaxto>=1.1.1and enablesws_id_extractor = "id"so test cassettes correctly correlate concurrent, out-of-order request/response replay instead of relying on send positionRelated
Addresses the reader starvation / interleaved-fragment issues raised in #248 (bugs 2 & 3). Does not address bug 1 (send blocked behind a concurrent recv in the underlying
urllib3.futuretransport) — that remains an upstream issue.Test plan
uv run ruff check homeassistant_api/asyncwebsocket.pyuv run zuban check homeassistant_api/asyncwebsocket.pyuv run pytest -q— 175 passed