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

Async view cancellation freeze asgi worker when using django-allauth #3566

Closed
jrobichaud opened this issue Dec 14, 2023 · 6 comments
Closed

Comments

@jrobichaud
Copy link

Django 5.0 introduced the cancellation of Async views.

But when using django-allauth's allauth.account.middleware.AccountMiddleware the asgi worker freezes indefinitely.

This is a major problem.

Conditions:

  1. use allauth.account.middleware.AccountMiddleware
  2. use asgi (Ex: with uvicorn)
  3. call an async view then cancel it (ex curl <view url> then ctrl-c before it finishes)

I made a project to reproduce the problem:
https://github.com/jrobichaud/django-allauth-async-view-cancel

@pennersr
Copy link
Owner

allauth currently does not support async, see #3546. Likely, this needs to be applied to the middleware:

https://docs.djangoproject.com/en/4.2/topics/http/middleware/#asynchronous-support

Can you check if that fixes the issue? A PR is welcome.

@pennersr
Copy link
Owner

I have a fix for this in the pipeline...

@jrobichaud
Copy link
Author

jrobichaud commented Dec 14, 2023

This could work I think:

from asgiref.sync import iscoroutinefunction, markcoroutinefunction
from asgiref import sync
from allauth.core import context
from allauth.account import middleware


class AccountMiddleware(middleware.AccountMiddleware):
    async_capable = True
    sync_capable = True

    def __init__(self, get_response):
        self.get_response = get_response
        if iscoroutinefunction(self.get_response):
            markcoroutinefunction(self)

    def __call__(self, request):
        if iscoroutinefunction(self):
            return self.__acall__(request)
        return super().__call__(request)

    async def __acall__(self, request):
        with context.request_context(request):
            response = await self.get_response(request)
            await sync.sync_to_async(self._remove_dangling_login)(request, response)
            return response

I did it using inheritance but the classes would have to be merged.

@pennersr
Copy link
Owner

Doesn't sync_to_async spawn a thread to run the sync code? In any case, I have avoided that here #3568 -- also not very pretty, doubling code, but it's what Django recommends.

@jrobichaud
Copy link
Author

You are right, it is not required.

@jrobichaud
Copy link
Author

jrobichaud commented Dec 14, 2023

Updated:

from asgiref.sync import iscoroutinefunction, markcoroutinefunction
from allauth.core import context
from allauth.account import middleware


class AccountMiddleware(middleware.AccountMiddleware):
    async_capable = True
    sync_capable = True

    def __init__(self, get_response):
        self.get_response = get_response
        if iscoroutinefunction(self.get_response):
            markcoroutinefunction(self)

    def __call__(self, request):
        if iscoroutinefunction(self):
            return self.__acall__(request)
        return super().__call__(request)

    async def __acall__(self, request):
        with context.request_context(request):
            response = await self.get_response(request)
            self._remove_dangling_login(request, response)
            return response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants