Fix Django ASGI HTTP concurrency issues

[Archmonger]

This PR changes thread_sensitive=False for view rendering.

Testable by creating a view_1 with time.sleep(30) then accessing a different view_2 without any sleeps.
With thread_sensitive=True, Django would be completely blocking on view_1 to complete sleeping then return a HTTP response.

I highly recommend also merging django/channels#1587 for anyone using the legacy django/channels ASGIHandler

fix Archmonger/Conreq#16
fix django/channels#1587
fix django/asgiref#218
fix django/daphne#350
fix django/channels#1626

[github-actions]

Hello @Archmonger! Thank you for your contribution 💪

As it's your first contribution be sure to check out the patch review checklist.

If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!

If you have any design or process questions then you can ask in the Django forum.

Welcome aboard ⛵️!

[Archmonger]

I'm unsure how to create a unit test for this. Need to send a two http requests concurrently, but I'm not sure how to emulate that in tests.py.

Here's my working draft for a test function. Unfortunately it will always throw an exception, and additionally will cause all other tests to fail.

django/tests/asgi/tests.py:ASGITest

........

    async def test_async_rendering(self):

        application = get_asgi_application()

        async def ping_endpoint(endpoint):
            """Generic to ping a certain endpoint."""
            scope = self.async_request_factory._base_scope(path=endpoint)
            communicator = ApplicationCommunicator(application, scope)
            await communicator.send_input({'type': 'http.request'})
            response_start = await communicator.receive_output()
            response_body = await communicator.receive_output()
            return response_body['body']

        slow_task = asyncio.create_task(ping_endpoint('/sleepy/'))
        fast_task = asyncio.create_task(ping_endpoint('/nosleep/'))

        try:
            await asyncio.wait_for(fast_task, timeout=3.0)
        except asyncio.exceptions.TimeoutError:
            print('This should NOT have failed')

        try:
            await asyncio.wait_for(slow_task, timeout=5.0)
        except asyncio.exceptions.TimeoutError:
            print('This should have failed')
            
........

``django/tests/asgi/urls.py`

........

from time import sleep

def sleeper(request):
    sleep(10)
    return HttpResponse('Slow (Body)')

def nosleep(request):
    return HttpResponse('Fast (Body)')

........

urlpatterns = [
    path('', hello),
    path('file/', lambda x: FileResponse(open(test_filename, 'rb'))),
    path('meta/', hello_meta),
    path('sleepy/', sleeper),
    path('nosleep/', nosleep),
]

[carltongibson]

Hi @Archmonger -- you seem to have ignored all the conversations here around WHY we switched thread_sensitive to True in the first place... — ORM operations must be performed on the main thread. We can't accept this. (Work on making the ORM async aware is ongoing, but until then this is a non-starter.)

Maybe we could allow Channels to use thread_sensitive=False when passing down to Django's ASGIHandler, which would allow one Django view per thread, but whether that's even safe still needs demonstration.

This is just to repeat the points from the other threads you've been active on... 🤔

[Archmonger]

@carltongibson Got it, looks like I misunderstood the last comment on that asgrief thread.

I'll take a look at making the ORM async aware. I see an early comment about "subtle bugs". Is there an ongoing bug list for async ORM issues? For example, does Django suffer from half transmitted SQL queries (or intermingled queries) under these conditions?

Perhaps @andrewgodwin could chime in on the current issues/behaviors to look out for.

[carltongibson]

The best place to look is the thread on the forum.

https://forum.djangoproject.com/t/asynchronous-orm/5925

There's a lot of discussion there.
I think this is the best place to follow up with questions too.

Then Andrew has given various talks at DjangoCons, which are available on YouTube now. That gives a good idea of his current thinking.

Super if you want to get involved here!

[andrewgodwin]

Right, we had to deliberately switch this on because of the way middleware works. I have expanded a bit on what I believe originally caused us to switch it on https://forum.djangoproject.com/t/asynchronous-orm/5925/32, but in general people are just going to pass ORM and connection objects from middlewares to views so we unfortunately need to keep them aligned.

A potential fix that could be done is using the new scoping feature of thread_sensitive in the new asgiref releases, that lets you shrink the scope of what must all run in a single thread so it can be done per-request.