Possible async view regression in django 5.0?

[Hugo Heyman]

Hello!

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 occurred 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.

Now for the possible regression with django:

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 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 issues. https://github.com/HeyHugo/django/commit/e25a1525654e00dcd5b483689ef16e0dc74d32d1

Here's a minimal setup to demonstrate the issue via print debugging: https://github.com/HeyHugo/django_async_issue

(I have never found a real bug in django so I'm still having doubts :D)

Best regards
/Hugo

[Natalia Bidart]

Hello Hugo, thanks for this email.

Just for completeness sake and for future readers, I'm leaving a note here that a ticket was created for this issue (https://code.djangoproject.com/ticket/35148) which was closed as a duplicate of https://code.djangoproject.com/ticket/35059.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/4c7a761b-1420-4997-900f-b8f0d59f4f31n%40googlegroups.com.