#35148 closed Bug (duplicate)
Missed request cleanup for async views
Reported by: | Hugo Heyman | Owned by: | nobody |
---|---|---|---|
Component: | HTTP handling | Version: | 5.0 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Hello!
I posted this on django-dev mailing list at first, which was probably not the correct place for this (sorry), so I'm posting it here instead.
I've been troubleshooting an issue with lingering idle db connections (postgres) since upgrading from django 4.2 to 5.0. The issue manifested by db raising OperationalError FATAL: sorry, too many clients already not long after deploying the new version. We're running asgi with uvicorn + gunicorn.
After extensive troubleshooting I installed django from a local repo and ran a git bisect to pin down the commit where this first occured and ended up on this commit https://github.com/django/django/commit/64cea1e48f285ea2162c669208d95188b32bbc82
Since I've recently built something quite async heavy I decided to dig into how the ASGIHandler
code works and see if I can understand it and possibly make a fix if there really is some issue there (I have never found a real bug in django so I'm still having doubts :D).
The change introduced in Django 5.0 to allow handling of asyncio.CancelledError
in views seems to also allow async views with normal HttpResponse
handling client disconnects. As a consequence though the request_finished
signal may not run leading to things like db connection not being closed.
What currently seems to be happening in ASGIHandler
in django/core/handlers/asgi.py
goes something like this:
- Concurrent tasks are started for
- disconnect listener (listen for "http.disconnect" from webserver)
- process_request
- If disconnect happens first (e.g. browser refresh)
cancel()
is run onprocess_request
task andrequest_finished
may not have been triggered - db connection for the request is not closed :(
Here are the lines of code where all this happens, marked: https://github.com/django/django/blob/9c6d7b4a678b7bbc6a1a14420f686162ba9016f5/django/core/handlers/asgi.py#L191-L232
Possible fix? I'm thinking only views that return a HTTPStreamingResponse
would need to allow for cancellation cleanup handling via the view. If so response = await self.run_get_response(request)
could run before and outside of any task so we could check response.streaming
attribute and only run listen_for_disconnect
task when there's a streaming response.
I've made a patch to try the above out and it seems to fix the issue:
https://github.com/HeyHugo/django/commit/e25a1525654e00dcd5b483689ef16e0dc74d32d1
Here's a minimal setup to reproduce the issue via print debugging:
https://github.com/HeyHugo/django_async_issue
Change History (4)
comment:1 by , 9 months ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
comment:2 by , 9 months ago
Hugo, Does that the proposed patch work for you? Testing would be a huge help.
comment:3 by , 9 months ago
Aha oops there was already a ticket and a patch for this.
That patch works great. Tested on both my minimal setup and real project.
Duplicate of #35059.