tornadoweb / tornado

Tornado is a Python web framework and asynchronous networking library, originally developed at FriendFeed.
http://www.tornadoweb.org/
Apache License 2.0
21.69k stars 5.5k forks source link

websocket: Async version of WebSocketHandler.close is needed #2914

Open benediamond opened 4 years ago

benediamond commented 4 years ago

NOTE: this is different from https://github.com/tornadoweb/tornado/issues/2763. possibly related to https://github.com/tornadoweb/tornado/issues/2448, but also distinct.

After starting a websocket server, I am getting

ERROR:asyncio:Task was destroyed but it is pending!
task: <Task pending name='Task-5' coro=<RequestHandler._execute() running at /usr/local/lib/python3.8/site-packages/tornado/web.py:1703> wait_for=<Future pending cb=[<TaskWakeupMethWrapper object at 0x10acb2970>()]> cb=[_HandlerDelegate.execute.<locals>.<lambda>() at /usr/local/lib/python3.8/site-packages/tornado/web.py:2333]>

even after appropriately closing all connections. this is different from https://github.com/tornadoweb/tornado/issues/2763, where the unresolved task was WebSocketProtocol13._receive_frame_loop, and not all connections were closed.

to reproduce this easily, run the following tiny python3 program:

import asyncio
from abc import ABC
import tornado.websocket
import tornado.httpserver
import tornado.ioloop
import tornado.web

handlers = []
class Handler(tornado.websocket.WebSocketHandler, ABC):
    def open(self):  # making this async and awaiting write_message doesn't silence the tornado warnings.
        handlers.append(self)
    def on_message(self, message):
        pass
    def on_close(self):
        pass
    def check_origin(self, origin):
        return True
application = tornado.web.Application([
    (r'/', Handler),
])
http_server = tornado.httpserver.HTTPServer(application)
loop = tornado.ioloop.IOLoop.current()
async def my_function():  # closes the connections, stops the server, stops the loop after 20s.
    await asyncio.sleep(20.0)
    for handler in handlers:
        handler.close()
    http_server.stop()
    loop.stop()
http_server.listen(8080)
loop.add_callback(my_function)
loop.start()
loop.close()

in a separate Node.js console (for example), run:

const WebSocket = require('ws');
new WebSocket('ws://localhost:8080')

the culprit appears to be the future fut here, which is never awaited: https://github.com/tornadoweb/tornado/blob/79b9c4fcbb3728f1325c4f6120bec45dd5df9fd8/tornado/web.py#L2323-L2326 tornado version: tornado-6.0.4. Python version: 3.8.5.

i am unable to further diagnose the issue, or fix it. thanks for your attention and time.

benediamond commented 4 years ago

@garenchan sorry to bother—not sure if you have any thoughts on this. thanks.

ebrightfield commented 4 years ago

Also having this problem. Do you know where might a safe (and close-to-the-source) place be to catch this with a try/except block until this is fixed?

infime commented 3 years ago

Also running into this after upgrading from python 3.7.6 to 3.8.6

AlanW0ng commented 2 years ago

Also running into this after upgrading from python 3.7.6 to 3.8.6

Thanks. I solved this problem by downgrading python to 3.6

graingert commented 2 years ago

you can avoid this by using an asyncio.Event() to wait for the on_close( method to be called

Here I created an async def aclose( method, which you can gather when shutting down your server gracefully:

import functools
import asyncio
import tornado.websocket
import tornado.httpserver
import tornado.ioloop
import tornado.web

class Handler(tornado.websocket.WebSocketHandler):
    def initialize(self, *, handlers):
        self._handlers = handlers
        self._closed = asyncio.Event()

    def open(self):
        self._handlers.add(self)

    def on_message(self, message):
        pass

    def on_close(self):
        self._handlers.remove(self)
        self._closed.set()

    def check_origin(self, origin):
        return True

    async def aclose(self):
        self.close()
        await self._closed.wait()
        return self.close_code, self.close_reason

async def my_function():
    handlers = set()
    application = tornado.web.Application(
        [
            (r"/", Handler, {"handlers": handlers}),
        ]
    )
    http_server = tornado.httpserver.HTTPServer(application)
    http_server.listen(8080)

    try:
        await asyncio.sleep(20.0)
    finally:
        http_server.stop()
        await asyncio.gather(*(handler.aclose() for handler in handlers))

asyncio.run(my_function())
bdarnell commented 1 year ago

The root cause of this issue is that WebSocketHandler.close is not immediate - it begins the process of shutting down the websocket connection, but this takes a little time (the server sends a close frame to the client, then waits for the client to respond with its own close frame before closing the TCP connection). We don't currently have a clean way to wait for this process to complete - @graingert 's use of the on_close hook is the best workaround for now. We should incorporate something like that into WebSocketHandler to give you a good way to close all your connections and wait for them to shut down cleanly.