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

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

Open
khalo-sa opened this issue Jan 23, 2024 · 1 comment
Open

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

khalo-sa opened this issue Jan 23, 2024 · 1 comment
Labels

Comments

@khalo-sa
Copy link

khalo-sa commented Jan 23, 2024

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
Copy link
Owner

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.

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

2 participants