miguelgrinberg / python-socketio

Python Socket.IO server and client
MIT License
4.01k stars 589 forks source link

_ping background task not cancelled by async_server.shutdown() #1301

Closed khalo-sa closed 5 months ago

khalo-sa commented 10 months ago

Bug Description

Pretty much what the title says. I'm using python-socketio version 5.11.0.

Here is code to reproduce:

import asyncio
from contextlib import asynccontextmanager

import socketio
from uvicorn import Config, Server

@asynccontextmanager
async def run_uvicorn_server(config: Config, sockets=None):
    server = Server(config=config)
    cancel_handle = asyncio.ensure_future(server.serve(sockets=sockets))
    await asyncio.sleep(0.1)
    try:
        yield server
    finally:
        await server.shutdown()
        cancel_handle.cancel()

def check_ping_task():
    for task in asyncio.all_tasks():
        if task.get_coro().__name__ == "_send_ping":
            print(f"found ping task. Done: {task.done()}")
            return
    print("no ping task found")

async def __main__():
    sio_server = socketio.AsyncServer(
        async_mode="asgi", cors_allowed_origins=[], logger=True
    )
    sio_client = socketio.AsyncClient()

    app = socketio.ASGIApp(sio_server, socketio_path="/")

    config = Config(app=app, log_level="error")

    print("\n==before server start==\n")

    check_ping_task()

    async with run_uvicorn_server(config=config):
        print("\n==after server start, before user connect==\n")
        check_ping_task()

        # connect a user
        await sio_client.connect("http://localhost:8000")

        print("\n==after server start, after user connect==\n")

        check_ping_task()

        sleep_sec = 1

        await asyncio.sleep(sleep_sec)

        # user disconnects (shouldn't this already cancel the ping task?)
        await sio_client.disconnect()

        # shutting down sio server before web server shutdown
        await sio_server.shutdown()

    print("\n==after server shutdown==\n")

    print("server killed")

    await asyncio.sleep(1)

    check_ping_task()

if __name__ == "__main__":
    asyncio.run(__main__())

Output

==before server start==

no ping task found

==after server start, before user connect==

no ping task found

==after server start, after user connect==

found ping task. Done: False
Socket.IO is shutting down

==after server shutdown==

server killed
found ping task. Done: False

Expected behavior I would expect sio_server.shutdown to clean up all python-socketio related background tasks. In that case, I'm wondering if the _ping task should have even be cancelled already after the user disconnected, since it is only created after the user connects.

Notes I only noticed this in the tests of a more complex project, where I always got annoying "task was destroyed but it is pending" warnings regarding the _ping coroutine, whenever the pytest fixture for the sio-server was torn down.

miguelgrinberg commented 10 months ago

The _send_ping task is not a background task. There is one of these per connected client. These tasks are short lived, they end on their own. It may take up to ping_interval seconds for the task corresponding to a client to end gracefully from the time of disconnection.