Fixed ThreadSensitiveContext.__aexit__ blocking the event loop.

[Arfey]

just illustration of solution

[carltongibson]

Hi @Arfey — thanks for this, and for the extra investigation!

So it's right that (now) a "cancelled" sync_to_async call completes
immediately — exec_coro.cancel() succeeds on the asyncio wrapper even though
the worker thread runs on — orphaning the sync work. (That's exactly how
ThreadSensitiveContext.__aexit__ comes to meet a still-busy executor.)

You also hit the current code silently swallowing a cancellation
(returning a normal result from a task that was asked to cancel). So the
diagnosis is right.

That said, playing locally, I think this is orthogonal to #535:

1. Cancellation becomes unbounded, so timeouts stop working. Because the except-path now always drains via ret = await exec_coro, cancellation can't complete until the sync function returns. asyncio.wait_for(sync_to_async(slow_view)(), timeout=0.2) on a 2s view raises TimeoutError after 2.0s, not 0.2s — and never, if the sync code never returns. I think the xfail → skip(reason="deadlock") change to test_sync_to_async_with_blocker_thread_sensitive in this PR is the same issue showing up in the suite: the test would now hang rather than fail, which is probably a regression.

2. A second cancellation escapes the drain, and the original deadlock comes back. While the drain is parked at await exec_coro, another cancel() — server shutdown, a nested timeout, TaskGroup teardown, etc — is thrown into that await and propagates out with the worker still parked. __aexit__'s blocking shutdown() then freezes the loop with the worker waiting on it: the identical #535 circular wait. The guarantee this PR provides only holds if cancellation arrives exactly once.

Reproducer (deadlocks with this PR applied)

import asyncio, threading, time
from asgiref.sync import ThreadSensitiveContext, sync_to_async, async_to_sync

worker_started = threading.Event()

async def inner():
    await asyncio.sleep(0.5)

def sync_code():
    worker_started.set()
    async_to_sync(inner)()   # parks worker, enqueues loop callback

async def main():
    async with ThreadSensitiveContext():
        task = asyncio.create_task(sync_to_async(sync_code)())
        await asyncio.sleep(0)
        assert worker_started.wait(5)
        time.sleep(0.1)           # hold loop: create_task callback stays queued
        task.cancel()             # 1st cancel: enters the new drain path
        await asyncio.sleep(0.1)  # drain is parked at `await exec_coro`
        task.cancel()             # 2nd cancel: escapes the drain
        try:
            await task
        except asyncio.CancelledError:
            pass
    # __aexit__ -> executor.shutdown() -> loop joins worker; worker waits on loop.
    print("never reached")

asyncio.run(main())

3. The __aexit__ boundary needs fixing regardless. A fire-and-forget asyncio.create_task(sync_to_async(...)()) still parked in async_to_sync when the context exits deadlocks the same way, with no cancellation involved anywhere. So changing cancellation semantics — your target here — doesn't make the context exit safe on its own.

Given 2 and 3, I think we need the __aexit__ fix in any case.

I'll progress #563, with some amendments, but we can keep this open to discuss some more:

• Can we handle repeated cancellation? Maybe deliver the result/re-raise only once the sync work has actually finished, however many cancels arrive) — probably in dialogue with the uncancel() machinery that TaskGroup/asyncio.timeout rely on?
• The hard one: What do we do about sync code that never returns? (Everyone has this problem. It might not be solvable.)
• Is there a change in cancellation behaviour here? If so, let's be precise about it, and plan migration.

[Arfey]

Hey 👋

Point 2 is valid, walking through it, the second cancel does re-open the #535 window. I'm convinced the drain approach doesn't hold.

My main worry is cost. Based on the issue reactions, maybe 0.001% of users ever hit this deadlock, but the fix makes 100% of requests pay for it: a new thread on every request, plus zombie threads whenever the sync function keeps running after cancellation.

As an alternative, could we keep the separate executor but only as a workaround for the cancellation case? When a CancelledError happens during loop.run_in_executor inside a ThreadSensitiveContext (or just for async_to_sync call), we set a flag on the context, and at shutdown we use the separate executor only if that flag is set, otherwise we use the same one. It won't fix the zombie-thread problem, but it should at least remove the overhead for the 99% of requests where nothing was cancelled.

What do you think?

---

On "cancellation becomes unbounded, so timeouts stop working":

Quoting my original proposal:

I want to float a different angle: fix the cause instead. We can't truly cancel running sync code anyway, so instead of cancelling mid-flight, we let it run until it finishes or reaches the async_to_sync handoff, and only then propagate the CancelledError.

The current PR only implements the first half, "let it run until it finishes", which is where the problem you found comes from. The second half, "or reaches the async_to_sync handoff", isn't implemented yet, and I think that's the important part.

On timeouts: they only fire at await points. Pure CPU-bound async code with no await blows straight through, and we accept that:

Example

import asyncio
from asgiref.timeout import timeout

async def real_cpu_task():
    total = 0
    for i in range(10**9):
        total += i
    return total

async def main():
    loop = asyncio.get_event_loop()
    start = loop.time()
    try:
        async with timeout(0.1):
            result = await real_cpu_task()
        print(f"Result: {result}, took {loop.time() - start:.3f}s")
    except asyncio.TimeoutError:
        print(f"Timeout! took {loop.time() - start:.3f}s")

asyncio.run(main())

# Result: 499999999500000000, took 28.548s

Sync code run through sync_to_async is, by definition, code with no await points, so I'd argue it's fine that the timeout doesn't cancel it mid-flight, it's the same behaviour. And if async_to_sync raised on cancel like a normal awaitable, it would follow the same default async semantics.

On test_sync_to_async_with_blocker_thread_sensitive: I think the timeout just moves the deadlock from one place to another. After the timeout fires, it parks forever in executor.shutdown() (without an event.set()), because the thread is still blocked on the event and nothing wakes it. Very strange case.