New thread_sensitive=True default effectively makes Channels single-threaded

[AlexHill]

Hi all,

We're running Django 1.11 but this applies at least up to 2.2.

We didn't have asgiref pinned, and so at some point it was updated to 3.3. We found that a bunch of requests were timing out and we tracked it down to the changed default of thread_sensitive to True in SyncToAsync.

Since Django views are run inside this decorator, all Django code is treated as thread sensitive, which means none of it is run on the asgiref threadpool. ASGI_THREADS has no effect. In addition to reducing concurrency to zero, this introduces a lot of latency if you have any long-running requests. The uvicorn worker accepts connections as fast as it receives them, so one slow request in a worker will cause a backlog of unhandled async tasks inside that worker, even if other worker processes are doing nothing.

I'm not sure what the answer is here, just wanted to raise the issue. From what I can see it looks as though the change was made to accommodate things in Django 3. We've fixed the issue by pinning the older version of asgiref. Perhaps there could be a new point release of Channels 2.4 that requires asgiref<3.3?

[andrewgodwin]

This is an unfortunate side-effect of the change, but it is deliberate - we were seeing a lot of subtle bugs caused by the multi-threaded nature of things that didn't have thread_sensitive, and so the decision was made to be safe by default rather than faster by default.

Channels is particularly hard-hit because of the way it works, but what's needed there is a Channels setting to decide how to run Django code; it's not something we can really fix on the asgiref side of the fence. If you're using sync_to_async directly you can just throw thread_sensitive=False back in there to get the performance boost back, but in the case of Channels, that'll need to get exposed there somehow.

[AlexHill]

Thanks for the response.

Just to check my understanding - considering only a normal Django app with no explicit use of async functionality, would serving under channels with thread_sensitive=False be pose any additional risk compared to running in a traditional multi-threading application server? Is the answer to that question any different between Channels 3 and 2.x and/or Django 3 and 2.x?

My instinct would be that without any async views or use of the async decorators, the risk is the same - when run in asgiref's threadpool the entire request/response cycle will happen on the same thread, so anything that worked in e.g. uWSGI should work there too. But I haven't dug too deep here.

Could changing Channels to decorate AsgiHandler.handle() with @partial(sync_to_async, thread_sensitive=False) be safe? If every call below that top-level there defaulted to True, then everything within a request should be run in the same thread, while still allowing different requests to be run in different threads...

[andrewgodwin]

I think Channels would mostly be safe as there's nothing synchronous on the main thread to lock to, but I'd have to go back into the codebase to be sure, and it's been a loooong time since I actively developed in there.