Missed request cleanup for async views

[Hugo Heyman]

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:

1. Concurrent tasks are started for
   • disconnect listener (listen for "http.disconnect" from webserver)
   • process_request
2. If disconnect happens first (e.g. browser refresh) cancel() is run on process_request task and request_finished may not have been triggered
3. 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

[Mariusz Felisiak]

Duplicate of #35059.

[Mariusz Felisiak]

Hugo, Does that the proposed ​patch work for you? Testing would be a huge help.

[Hugo Heyman]

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.

[Mariusz Felisiak]

Thanks for checking.