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

ClientSession Object does not have its .close() method called in the event that its .get() method returns an error #7299

Closed
1 task done
DanielCKL opened this issue May 25, 2023 · 3 comments
Labels

Comments

@DanielCKL
Copy link

DanielCKL commented May 25, 2023

Describe the bug

If an aiohttp.ClientSession() object's get() method results in a timeout error, and it is called by 'async with', the .close() method is supposed to be automatically called by the 'async with'. However, that does not happen, and the session will remain unclosed if not explicitly closed.

To Reproduce

To replicate the issue, run this in Python 3.10.11:

import aiohttp
import asyncio
import time
import requests
MaxRange=300
import gc

async def async_multi_query_noInterrupt(URLs: dict, query_function)->dict:
    
    tasks={key_id:asyncio.create_task(query_function(**URLs[key_id])) for key_id in URLs} 
    
    for each_task in tasks: 
        await tasks[each_task]
    
    errors={}
    for key_id in tasks:
        if not isinstance(tasks[key_id].result(),BaseException): 
            tasks[key_id]=tasks[key_id].result()
        else: 
            errors[key_id]=URLs[key_id]
            
    #Remove all urls with errors from tasks
    for key_id in errors:
        del tasks[key_id]

    gc.collect() #To prevent memory leaks

    return {'results':tasks,'errors':errors}

async def get_url(**kwargs):
    async with aiohttp.ClientSession() as session:
        try: 
            async with session.get(**kwargs) as response: 
                results = await response.json()
        except Exception as e:
            if not session.closed:
                print("Issue: Status of session.closed is: ", session.closed)  
            #await asyncio.sleep(0)
            #await session.close() #This will NOT be called by 'async with session.get(**kwargs) as response'! Must be done here.
            #print(session.closed)  #If you run session.closed()  now, it will be closed.
            return e
    #print(session.closed) # The session will be closed if there was no timeout.
    return results


input_dict={}
for number in range(1, MaxRange):
    pokemon_url = f'https://pokeapi.co/api/v2/pokemon/{number}'
    input_dict[number]={'url':pokemon_url,'timeout':0.1} #Intentionally give a very short timeout here 

now=time.time()
results=asyncio.run(async_multi_query_noInterrupt(input_dict,get_url))
print(time.time()-now)

Expected behavior

Of course, the session will be correctly closed if there was no timeout, but if I'm not mistaken, by rights the 'async with' clause should call the ClientSession() object's .close() or .aexit() method regardless of whether an error was caught or not:
https://superfastpython.com/asyncio-async-with/

If the ClientSession objects were closed, we should not be seeing even a single 'Issue: Status of session.closed is: False' being printed out.

Note: I am not sure if this issue is related to #1925

Logs/tracebacks

Issue: Status of session.closed is:  False
Issue: Status of session.closed is:  False
 . . .
0.24727106094360352

Python Version

$ python --version
3.10.11

aiohttp Version

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.8.4
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author:
Author-email:
License: Apache 2
Location: c:\users\study\desktop\github\discordbot\venv\lib\site-packages
Requires: aiosignal, async-timeout, attrs, charset-normalizer, frozenlist, multidict, yarl
Required-by:

multidict Version

$ python -m pip show multidict
Name: multidict
Version: 6.0.4
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: [email protected]
License: Apache 2
Location: c:\users\study\desktop\github\discordbot\venv\lib\site-packages
Requires:
Required-by: aiohttp, yarl

yarl Version

$ python -m pip show yarl
Name: yarl
Version: 1.9.2
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl/
Author: Andrew Svetlov
Author-email: [email protected]
License: Apache-2.0
Location: c:\users\study\desktop\github\discordbot\venv\lib\site-packages
Requires: idna, multidict
Required-by: aiohttp

OS

Windows 10

Related component

Client

Additional context

No response

Code of Conduct

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

#await session.close() #This will NOT be called by 'async with session.get(**kwargs) as response'! Must be done here.

This line appears inside the session context. You are explicitly closing the session before the context is exited.
If the session is still open after the context, that'd be unexpected.

@Dreamsorcerer
Copy link
Member

The context you have in the try block is the request context. The request should be considered closed (and depending on the status, may result in the connection being closed), but the session is obviously still open.

@DanielCKL
Copy link
Author

The context you have in the try block is the request context. The request should be considered closed (and depending on the status, may result in the connection being closed), but the session is obviously still open.

You are right, my bad, that was a serious brain fart. Now only do I see the line 'async with aiohttp.ClientSession() as session' and think 'oh, maybe THAT is responsible for closing the session instead of 'async with session.get(**kwargs) as response''.

Thank you so much for taking the time to point my mistake out to me. The Internet is truly an amazing place!

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