-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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! timeout #7072
Comments
Also having a lot of Sentry errors because of this. Could you please ping us once this has been added to a new version. |
It would be nice if we could have a patch that we can implement directly in Python while we wait for a proper fix? |
Any news on this ? |
Could you write a test that reproduces the problem (and submit that as a PR)? |
@Dreamsorcerer Sorry it took so long! I suspect the real issue is some DNS lookup failure occurring at random times. In order to simulate this, I "patched" the This yields something close to the same error, which leads me to think I'm on the right path. The solution suggested by @michaelzus does indeed resolve the issue. Here's a test that reproduces the problem: from aiohttp import ClientError, ClientSession, ClientTimeout, TCPConnector
from asyncio import TimeoutError
import asyncio
# Patch to simulate a DNS lookup failure
# I believe this is what causing the random errors
async def delay(self, host, port, traces):
await asyncio.sleep(20)
TCPConnector._resolve_host_with_throttle = delay
async def main(url):
async with ClientSession() as session:
try:
response = await session.get(url, timeout=ClientTimeout(total=1))
response.close()
except (ClientError, TimeoutError):
# All exceptions thrown by aiohttp is handled here
print('Timeout error handled, but not pending task')
return False
return True
if __name__ == "__main__":
asyncio.get_event_loop().run_until_complete(main('http://httpstat.us/200?sleep=5000'))
asyncio.get_event_loop().close() Running it as-is yields the following output in the console:
Applying the modification from @michaelzus returns the following:
(The expected result ; Only the print statement is displayed, no exception is raised) Tested with Note that the exception says "coro=<delay() done...". The original exception is "coro=<TCPConnector._resolve_host() running at". (running at vs done). I suspect this is because of the patch and can be disregarded. Having the suggestion from @michaelzus resolving it makes me believe it. And finally, here's a full stacktrace via Sentry: I'll push a PR. |
see #7608 |
OK, I see what's happening. The reference to the task gets garbage collected at the end of the function, so if no other connector has a reference to the task, then the task ends up getting dropped. I think we need to add a set to TCPConnector to track the tasks and then cancel and await them in .close(). |
Describe the bug
In case of setting a ClientTimout, we are getting the following issue:
Task was destroyed but it is pending!
task: <Task pending name='Task-3' coro=<TCPConnector._resolve_host() running at /my_env/git/nlastic-connections/venv/lib/python3.10/site-packages/aiohttp/connector.py:874> wait_for=<Future pending cb=[_chain_future.._call_check_cancel() at /labhome/python_3.10.4/lib/python3.10/asyncio/futures.py:384, Task.task_wakeup()]> cb=[TCPConnector._create_direct_connection..drop_exception() at /my_env/git/nlastic-connections/venv/lib/python3.10/site-packages/aiohttp/connector.py:1155]>
To Reproduce
Expected behavior
not to see the issue
Logs/tracebacks
Python Version
aiohttp Version
multidict Version
yarl Version
OS
CentOS Linux release 7.9.2009 (Core)
Related component
Client
Additional context
problem was solved by the following refactor:
adding "host_resolved.cancel()" to aiohttp/connector.py at line 1160
code block:
`
except asyncio.CancelledError:
`
Code of Conduct
The text was updated successfully, but these errors were encountered: