Fix orphaned WebSocket IO tasks logging 'Task was destroyed but it is pending!' on abrupt disconnect - #3184
Conversation
…eak them data_received/eof_received create fire-and-forget asyncio.Task objects (async_data_received/async_eof_received) with no reference kept anywhere. On an abrupt client disconnect, these can still be pending when nothing else holds them, so the event loop's GC destroys them and logs "Task was destroyed but it is pending!" -- harmless but log-polluting on every normal disconnect during in-flight data/eof handling. keepalive_ping_task and auto_closer_task already avoid this by being stored on the protocol instance; apply the same pattern to these IO tasks via a small _schedule_io helper that tracks them in self.io_tasks (pruned via a done-callback) and cancels any still-pending ones in connection_lost. Fixes sanic-org#3175.
|
The 3 test_asgi.py failures in "Check coverage" are unrelated to this PR's change (`sanic/server/websockets/impl.py`, exercised only by the WebSocket tests, which pass): ``` This is a dependency-version mismatch between the freshly-resolved `uvicorn` (a test dependency here) and `websockets` packages in this run's environment, not anything in this diff. Confirmed by checking main's own CI on the exact commit this branch is based on (`5ffc7b37`) — it passed cleanly two days ago with the same test file untouched. The `type-checking` job failures are also pre-existing and unrelated: 2 mypy errors in `sanic/pages/error.py`, present on `main` before this PR. The websocket-focused test run (`pytest tests/ -k "websocket or ws_"`) passes clean, including the new regression test. |
Description
Closes #3175.
data_received()andeof_received()inWebsocketImplProtocolcreate fire-and-forget tasks (async_data_received/async_eof_received) via a bareasyncio.create_task(...), with nothing keeping a reference to them:On an abrupt client disconnect (e.g. closing a browser tab mid-WebSocket-session), one of these tasks can still be pending with nothing else referencing it. The event loop's task-GC then destroys it and logs:
It doesn't crash anything, but it's a normal-disconnect-path log pollution issue, especially noisy for long-lived WebSocket connections (e.g. GraphQL subscriptions).
keepalive_ping_taskandauto_closer_taskalready avoid this exact problem by being stored as attributes on the protocol instance. This PR applies the same "keep a strong reference" pattern to the IO tasks created fromdata_received/eof_received.Changes
self.io_tasks: set[asyncio.Task]toWebsocketImplProtocol._schedule_io()helper that creates the task, adds it toio_tasks, and registers a done-callback to discard it once it completes — the standard "keep a strong reference" pattern for fire-and-forget tasks (see the asyncio docs'create_taskexample).data_received/eof_receivednow go through_schedule_io()instead of a bareasyncio.create_task(...).connection_lost()now cancels any IO tasks still pending, so they can't be silently GC'd mid-flight, and don't run forever past the point the connection is gone.Testing
test_connection_lost_cancels_pending_io_tasksintests/test_websockets.py, which schedules a never-completing coroutine via_schedule_io, callsconnection_lost, and asserts the task is actually cancelled and pruned fromio_tasks— verified this fails against the pre-fix code (_schedule_iodoesn't exist yet).pytest tests/ -k "websocket or ws_"— 71 passed, no new warnings.ruff check/ruff format --check— clean.mypy sanic/server/websockets/impl.py— no new errors (the file itself reports none; the 3 project-wide mypy errors are pre-existing and unrelated, insanic/pages/error.pyandsanic/mixins/startup.py).Checklist