asyncio SSL transport error log after session.close() called from atexit #7328
Comments
|
I don't think what you're asking for is possible in a reasonable manner. The well-engineered solution is to get the user to manage the lifecycle (if it is tied to something they are using). That doesn't mean you need to expose ClientSession to them directly, but your API (Client in your example) should implement Using |
I can see how this is appealing from a low-level library creator's standpoint, but it is definitely not good engineering from the perspective of software higher up in the stack. Exposing all the specific resource management requirements of (transitive) libraries breaks encapsulation, complicates usability, and renders some software architectures impossible. Practically no software component takes 100% explicit responsibility for managing all the resources it uses. (What if Python required you to pass in a logger and clean it up? What if Python required you to have an allocator handy every time you instantiate an object?) Allocating resources from a global source and having them released automatically at process shutdown is an extremely practical, time-honored, and widely-used mechanism for dealing with many resources. May I please ask, what is the technical obstacle to this in aiohttp? I'm using a shared session as the docs instruct, I'm causing its |
I'd argue that using atexit to cleanup library details is breaking encapsulation and complicating usability. If you want a system where the user doesn't handle the lifecycle of the application, then you want to build a framework, at which point you have full control of the application lifecycle and can easily close the session at application shutdown (e.g. like what aiohttp does for the server component, using
You seem to be talking about memory allocation, which is handled by garbage collection, not OS resources. If you use a FileHandler, for example, in your logging, you are expected to close() it. For example, see the end of this example:
Nothing. The problem is the architecture of your library conflicting with the architecture of asyncio. Again, a well-engineered solution would ensure that the session is created and closed within a single |
|
I respectfully disagree. An async context manager is only one of many legitimate models for resource management; there are others, including free-on-shutdown, and many others. I am not willing to impose this model on all libraries that use HTTP or might possibly use it in the future, and on their own transitive users. I also respectfully ask that you stop implying that anything other than your preferred architecture is poorly engineered. This is simply not the case.
Not really; it conflicts with aiohttp. I'll choose a different HTTP library. Thanks for your guidance, in any case. |
Calling
You are actually removing control of the lifecycle from the user, which is restricting valid software architectures the user can use, instead of giving them freedom. If an async context manager is a valid way to handle the life cycle, why are you so insistent that a user should not be allowed to do this with your library?
No, it is asyncio. All other async HTTP libraries will still have a If you can find any popular async library, or anything in stdlib, that does not work like this, I'm happy to be proven wrong and learn about new architectures, but I'm pretty sure what you are asking for does not work well with the architecture of asyncio. I'm curious why you're so bothered with this in the async version though, I noticed in your PR that you never bothered to close the |
HTTPX works fine for me; it was a simple replacement. I admit that graceful shutdown of external resources on process termination is difficult, especially when faced with poorly-defined shutdown order. It is, however, possible; I've done it myself in various contexts. Part of it is the fine art of choosing which errors are safely ignorable at shutdown (only). |
I can assure you with the same example, that you are not closing the httpx session correctly. It is using asyncio sockets/transports, and those are tied to an event loop. Some implementation detail may mean that you don't see errors when calling
Many of us like to close things cleanly and not have any errors. This requires a library to expose controls for it's lifecycle, which is why I keep suggesting you do this. Many users who just don't care about this will just ignore it and not close it explicitly (such as your original Exposing the controls gives users the freedom to manage the lifecycle however they like, or not at all. |
|
If I write a library that uses a library that uses a library that may one day decide to use aiohttp for something (I donno, log metrics), then all these libraries have to be async context managers. It follows pretty quickly that every library has to be an async context manager. I don't wish to go there. I don't consider abruptly closing a socket on process shutdown necessarily to be a problem; Linux, TCP, TLS, and every server handle this just fine. We're unlikely to converge, so I'm signing off. Thanks again for responding, anyway. |
Or any async HTTP library, then yes. But, that library would also need to change it's API to become async, so it's not like it'll just suddenly happen without you noticing. asyncio and sync libraries are sort of like separate ecosystems that can't be mixed easily.
Then don't bother trying to close the session at all? I'm still not sure why you refuse to let any of your users be concerned about this though. Again, you're only restricting users, which seems to be the opposite of what you were claiming originally. |
Then I get other asyncio shutdown errors. I am a user of my library, I own |
The warnings are related to not closing the session properly. If you don't care, then ignore the warnings...
If you are the user, then you can call If you were insistent on doing it with |
|
One last thought, you might be able to do it like this (though I still strongly discourage it): The closure should ensure you are using the same loop to run the close() method. atexit should only occur outside of a running loop, so it should work OK. |
|
Thanks. This works only if the loop was dropped without closing, not if it was already closed. So, it works with something like, if __name__ == "__main__":
loop = asyncio.get_event_loop()
loop.run_until_complete(main())but not with if __name__ == "__main__":
asyncio.run(main()) # this closes the loop :(I did some more reading; my understanding of the issue is that the fds are registered to a particular loop, so any other loop won't get wakeups for them, and therefore can't really do much with them. Is that correct? Seems to me what we really need is lifecycle callbacks for event loops. I don't see any standard way to do it, but here's a monkey solution. I'm sure there are all sorts of problems with it, so consider it a sketch. # Installs a custom event loop policy to add `on_close()` lifecycle functions to
# an event loop.
import asyncio
def _install():
"""
Monkeypatches the event loop policy to augment new event loops with
`on_close` behavior.
"""
policy = asyncio.get_event_loop_policy()
old_new_event_loop = policy.new_event_loop
def new_event_loop():
"""
Monkeypatches a new event loop with an `on_close()` method to
register awaitables to call on close (in reverse order), and wraps its
`close()` method to call these.
"""
loop = old_new_event_loop()
loop.__on_close = []
old_close = loop.close
def on_close(aw):
loop.__on_close.append(aw)
def close():
for aw in reversed(loop.__on_close):
loop.run_until_complete(aw)
return old_close()
loop.on_close = on_close
loop.close = close
return loop
policy.new_event_loop = new_event_loop
asyncio.set_event_loop_policy(policy)
_install()
#-------------------------------------------------------------------------------
# Client wrapper. Would be in its own module. Use of aiohttp is
# implementation detail.
import aiohttp
import functools
@functools.cache
def _get_session():
session = aiohttp.ClientSession()
asyncio.get_event_loop().on_close(session.close())
return session
class Client:
async def get(self):
async with _get_session().get("https://google.com") as rsp:
return rsp.status
#-------------------------------------------------------------------------------
# Top-level app.
import logging
async def main():
client = Client()
print(await client.get())
if __name__ == "__main__":
logging.basicConfig()
asyncio.run(main()) |
Yes, that'd a be a problem, wasn't sure if it would definitely work or not.
I'm not sure there's any effort to add anything like this to asyncio, but you're welcome to ask in cpython if there's interest. Monkey patching asyncio is definitely a lot more likely to cause issues to end users than the other solutions I've suggested ( |
Describe the bug
asyncio logs
Fatal error on SSL transportat level ERROR on Python shutdown, aftersession.close()is called from an atexit handler.To Reproduce
Expected behavior
No logging on shutdown.
Logs/tracebacks
Python Version
aiohttp Version
multidict Version
yarl Version
OS
Arch Linux
Related component
Client
Additional context
My intent is to use aiohttp in a client library as implementation detail. Moving management of the
ClientSessionto the library's users is not an option. If there is a better way to shut down aiohttp cleanly at program exit without involving the caller, I'm not tied to atexit.I've read #1925 and tried adding the all_is_lost mechanism described here after shutdown in my atexit function. This detects one transport and then hangs indefinitely; neither
connection_lostnoreof_receivedis ever entered.One additional, rather mysterious point: in my larger application (though not with the sample code above), I see different behavior depending on whether .pyc files are present/fresh or not. No joke—I've verified this pretty carefully.
Thank you in advance for your help!
Code of Conduct
The text was updated successfully, but these errors were encountered: