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

WebSocketResponse requires closing/closed checks before sending something #3391

Open
achimnol opened this issue Nov 16, 2018 · 3 comments
Open
Labels
Milestone

Comments

@achimnol
Copy link
Member

achimnol commented Nov 16, 2018

Long story short

In the server-side websocket handlers written with aiohttp, WebSocketResponse's send_xxx() methods "ignores" closing/closed state of the connection.

So in my code, I had to add if ws.closed: break everywhere that calls send_xxx().

Maybe this issue is related to #2025.

Expected behaviour

The send_xxx() methods should raise an explicit exception so that a server-side task can notice if the connection is closed.

Actual behaviour

The send_xxx() methods just emit an warning to the logger and continue.
This behavior leads to a memory leak as the unsent messages accumulates in the buffer.
I get a stream of log messages like:

socket.send() raised exception.
websocket connection is closing.
socket.send() raised exception.
websocket connection is closing.
socket.send() raised exception.
websocket connection is closing.
socket.send() raised exception.
websocket connection is closing.
socket.send() raised exception.
websocket connection is closing.
socket.send() raised exception.
websocket connection is closing.
socket.send() raised exception.
...

until my server-side task finishes (which runs without noticing the connection is closed!).

Steps to reproduce

  1. Write a simple websocket handler like:
async def handler(request):
  ws = web.WebSocketResponse()
  await ws.prepare(request)
  for _ in range(10):
    await asyncio.sleep(1)
    await ws.send_str('test')
  return ws
  1. Open this handler in a web browser using Javascript that reads only one message and closes the connection actively.
    (NOTE: closing the page abruptly in the browser makes aiohttp to raise asyncio.CancelledError)

Your environment

Linux & macOS.

@aio-libs-bot
Copy link

GitMate.io thinks the contributor most likely able to help you is @asvetlov.

Possibly related issues are #370 (Server closes connection before sending data), #3363 (How send "WebSocket Connection Close[FIN]"???), #2202 (Send explicit RuntimeError on response operations after session closing), #2309 (WebSocketResponse does not throw/close when connection is abruptly cut), and #172 (Question about ServerHttpProtocol.closing()).

@asvetlov
Copy link
Member

asvetlov commented Dec 3, 2018

I like the proposal but it is a backward incompatible change: a server code can get an unexpected exception on await ws.send_str('data').

Let's postpone the fix to aiohttp 4.0 (next version after 3.5 release)

@redradist
Copy link

I like the proposal but it is a backward incompatible change: a server code can get an unexpected exception on await ws.send_str('data').

Let's postpone the fix to aiohttp 4.0 (next version after 3.5 release)

@asvetlov Is there any updates for this issue ?
It is very annoying to have this workaround for socket.send() raised exception., because of silent affect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants