Track and cancel WebSocket reconnect tasks cleanly (#73)#81
Merged
Conversation
The aiohttp WebSocket adapter started its reconnect loop with a fire-and-forget asyncio.create_task call, so close() could not cancel or await it. If close() ran while the loop was sleeping in its exponential back-off branch, the reconnect task survived shutdown and could touch a closing session. Reconnect also accumulated stale subscription ids in _subscriptions because resubscribe registered fresh ids without removing the dropped ones. Fix: - Store the reconnect task in self._reconnect_task and refuse to spawn a second one while one is active. - In close(), cancel and await the reconnect task so no background task survives shutdown. - Drop stale subscription ids before resubscribing so _subscriptions only ever contains ids that are valid on the live session. - Re-raise CancelledError from the reconnect loop so close() can unblock the sleep deterministically. Tests added: - close() cancels a reconnect task that is sleeping in back-off. - Only one reconnect task is active at a time. - _subscriptions contains only live ids after reconnect.
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.
Closes #73.
Validity assessment
Valid bug.
AiohttpWebSocketAdapter._reader_loopspawned the reconnectloop with a fire-and-forget
asyncio.create_taskcall (ws_aiohttp.py:399-400),so
close()(ws_aiohttp.py:142-182) had no handle to cancel or await.If
close()ran while the loop was blocked in its 1-60s back-off sleep(
ws_aiohttp.py:473-476), the reconnect task survived shutdown and couldrace against the closing session.
Reconnect also accumulated stale ids in
_subscriptionsbecausesubscribe_events()adds the new id (ws_aiohttp.py:325) but the_reconnect_loopresubscribe path never removed the old one(
ws_aiohttp.py:484-486).Fix
self._reconnect_task: asyncio.Task[None] | Noneand store thetask when reconnect starts.
close()now cancels and awaits_reconnect_taskafter the readertask, so a sleeping reconnect loop is woken and shut down
deterministically.
_reconnect_loopre-raisesCancelledErrorand clearsself._reconnect_task = Nonein afinallyblock._subscriptionsso it only ever contains ids that are valid on thelive session.
Tests added
test_close_cancels_reconnect_while_sleeping— forces reconnect intothe back-off sleep, calls
close(), and asserts the task is done andno reference leaks.
test_only_one_reconnect_task_active— asserts a single reconnecttask reference is held.
test_reconnect_clears_stale_subscription_ids— after a forced dropand reconnect, the old id is gone from
_subscriptionsand only thefresh id from resubscribe remains.
Checks run
pytest tests/ --cov=haclient --cov-report=term --cov-fail-under=95— 294 passed, 97.09% coverage.
ruff check src tests— passed.ruff format --check src tests— passed.mypy src—Success: no issues found in 37 source files.