Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #7306 - Set ClientWebSocketResponse.close_code correctly in concurrent closing scenario #7680

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/7306.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed ``ClientWebSocketResponse.close_code`` being erroneously set to ``None`` when there are concurrent async tasks receiving data and closing the connection.
5 changes: 3 additions & 2 deletions aiohttp/client_ws.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ async def send_json(
async def close(self, *, code: int = WSCloseCode.OK, message: bytes = b"") -> bool:
# we need to break `receive()` cycle first,
# `close()` may be called from different task
if self._waiting is not None and not self._closed:
if self._waiting is not None and not self._closing:
self._closing = True
self._reader.feed_data(WS_CLOSING_MESSAGE, 0)
await self._waiting

Expand All @@ -210,7 +211,7 @@ async def close(self, *, code: int = WSCloseCode.OK, message: bytes = b"") -> bo
self._response.close()
return True

if self._closing:
if self._close_code:
self._response.close()
return True

Expand Down
27 changes: 27 additions & 0 deletions tests/test_client_ws_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,33 @@
assert msg.type == aiohttp.WSMsgType.CLOSED


async def test_concurrent_task_close(aiohttp_client: Any) -> None:
async def handler(request):
ws = web.WebSocketResponse()
await ws.prepare(request)
await ws.receive()
return ws

app = web.Application()
app.router.add_route("GET", "/", handler)

async with aiohttp_client(app) as client:
async with client.ws_connect("/") as resp:
# wait for the message in a separate task
task = asyncio.create_task(resp.receive())

# Make sure we start to wait on receiving message before closing the connection
await asyncio.sleep(0.1)

closed = await resp.close()

await task
Fixed Show fixed Hide fixed

assert closed
assert resp.closed
assert resp.close_code == 1000
Dreamsorcerer marked this conversation as resolved.
Show resolved Hide resolved


async def test_concurrent_close(aiohttp_client: Any) -> None:
client_ws = None

Expand Down
Loading