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

Task was destroyed but it is pending — on the server side for orphan request handlers #7551

Closed
1 task done
nolar opened this issue Aug 25, 2023 · 1 comment
Closed
1 task done
Labels

Comments

@nolar
Copy link

nolar commented Aug 25, 2023

Describe the bug

The setup:

  • I use aresponses, but essentially any server might work e.g. RawTestServer.
  • a request hander that sleeps long enough (5 seconds) before returning an empty response ({}).
  • an aiohttp client that is forced to time out fast enough (0.1 seconds) AND that closes its own connections also fast enough, before the server begins the shutdown (<0.2s).

Because this happens in a test suite, everything typically happens very fast: the server spin up, the request & handling, and the server shutdown via context manager — there is no time for "idle" connection closing.

The client behaves as expected. The server, however, leaves 2 tasks pending after the server itself has "exited" via the content manager:

Task was destroyed but it is pending!
task: <Task pending name='Task-10' coro=<RequestHandler._handle_request() running at …/site-packages/aiohttp/web_protocol.py:442> wait_for=<Future pending cb=[Task.task_wakeup()]> cb=[Task.task_wakeup()]>
Task was destroyed but it is pending!
task: <Task pending name='Task-8' coro=<RequestHandler.start() running at …/site-packages/aiohttp/web_protocol.py:521> wait_for=<Task pending name='Task-10' coro=<RequestHandler._handle_request() running at …/site-packages/aiohttp/web_protocol.py:442> wait_for=<Future pending cb=[Task.task_wakeup()]> cb=[Task.task_wakeup()]>>

After some deep dive, I figured out that the task in RequestHandler._task_handler is created but never cancelled nor awaited when the connection is lost. More on that, the field is set to None and is not shut down later when the server shuts down (via [conn.shutdown() for conn in self._connections]) — because the self._connections is empty by that time.

If I do the manual hacking in RequestHandler.connection_lost() to add cancelling:

        if self._task_handler is not None:  # ADDED
            self._task_handler.cancel()     # ADDED
        self._task_handler = None

… then it helps to avoid pending/orphan tasks in the end.

I am not sure what would be a proper fix — all these connection_made/connection_lost methods are sync, so I cannot use await there to properly cancel-and-wait for the task.

Can you please hint where/how to fix this the best way?

To Reproduce

The timing is only for convenicence — to see the difference between 0.1s timeout and 5.0s shutdown.

The repro is standalone and works in Python 3.11.4, 3.7.9, and basically everywhere where I tested. The aiohttp is the most recent one.

from aiohttp.web import *
from aiohttp.test_utils import *

t0 = None


async def handler(request):
    await asyncio.sleep(5)
    return json_response({})


async def client_side(host, port):
    global t0

    # Simulate the typical client-side behaviour:
    session = aiohttp.ClientSession(connector=aiohttp.TCPConnector(limit=0))
    timeout = aiohttp.ClientTimeout(total=0.1)
    async with session:
        # t1 = asyncio.get_running_loop().time()
        # print(f"⏰ starting the request = {t1-t0:.6f}")

        with contextlib.suppress(asyncio.TimeoutError):
            await session.get(f'http://{host}:{port}/', timeout=timeout)

        # t2 = asyncio.get_running_loop().time()
        # print(f"⏰ finished the request = {t2-t0:.6f} / {t2-t1:.6f}")

    # t3 = asyncio.get_running_loop().time()
    # print(f"⏰ exited the inner ctx mgr = {t3-t0:.6f}")


async def main():
    global t0, t1
    before = asyncio.all_tasks()

    t0 = asyncio.get_running_loop().time()
    async with RawTestServer(handler) as server:

        await client_side(host=server.host, port=server.port)

        # Let the client-side session close its connections (server-side: "connection lost"),
        # but stay within the request handler's time before server shutdown (0.1+0.2 < 5).
        await asyncio.sleep(0.2)

    # t4 = asyncio.get_running_loop().time()
    # print(f"⏰ exited the outer mgr = {t4-t0:.6f}")

    after = asyncio.all_tasks()
    for t in after - before:
        print(f"💥Unattended asyncio task: {t!r}")


if __name__ == '__main__':
    asyncio.run(main())

Expected behavior

No unattended tasks are left after exiting the server.

In the logs below, I've added a few manual prints in aiohttp's code — notice that the CONN LOST happens BEFORE the SERVER SHUTDOWN. That is important. Without the 0.2s sleep, they happen in the proper order: server shutdown, then the client goes away — and there is no issue.

Logs/tracebacks

>>>> SERVER CONN MADE  len(self._connections)=1 handler=<RequestHandler connected>
>>>> SERVER CONN LOST handler=<RequestHandler connected> exc=None
>>> ReqHand cancelling the task
>>>> SERVER shutdown len(self._connections)=0

💥Unattended asyncio task: <Task pending name='Task-6' coro=<RequestHandler._handle_request() running at /Users/nolar/.pyenv/versions/kopf3114/lib/python3.11/site-packages/aiohttp/web_protocol.py:443> wait_for=<Future pending cb=[Task.task_wakeup()]> cb=[Task.task_wakeup()]>
💥Unattended asyncio task: <Task pending name='Task-4' coro=<RequestHandler.start() running at /Users/nolar/.pyenv/versions/kopf3114/lib/python3.11/site-packages/aiohttp/web_protocol.py:522> wait_for=<Task pending name='Task-6' coro=<RequestHandler._handle_request() running at /Users/nolar/.pyenv/versions/kopf3114/lib/python3.11/site-packages/aiohttp/web_protocol.py:443> wait_for=<Future pending cb=[Task.task_wakeup()]> cb=[Task.task_wakeup()]>>

Python Version

3.11.4

aiohttp Version

Name: aiohttp
Version: 3.8.5
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: 
Author-email: 
License: Apache 2
Location: /Users/nolar/.pyenv/versions/kopf3114/lib/python3.11/site-packages
Requires: aiosignal, async-timeout, attrs, charset-normalizer, frozenlist, multidict, yarl
Required-by: aresponses, kmock, kopf, pytest-aiohttp

multidict Version

Name: multidict
Version: 6.0.4
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: /Users/nolar/.pyenv/versions/kopf3114/lib/python3.11/site-packages
Requires: 
Required-by: aiohttp, yarl

yarl Version

Name: yarl
Version: 1.9.2
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl/
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache-2.0
Location: /Users/nolar/.pyenv/versions/kopf3114/lib/python3.11/site-packages
Requires: idna, multidict
Required-by: aiohttp

OS

MacOS

Related component

Server

Additional context

No response

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@nolar nolar added the bug label Aug 25, 2023
@Dreamsorcerer
Copy link
Member

Looks exactly the same as #6978.

As you already have a reproducer, if you could convert that into a test and put it into a PR that would be a massive help.

@Dreamsorcerer Dreamsorcerer closed this as not planned Won't fix, can't repro, duplicate, stale Aug 25, 2023
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