Problem
Every asyncio.to_thread(...) call in this repo runs on the shared default executor, whose worker count is bounded (min(32, cpu_count + 4) — 16 on a 12-core box). If a work item blocks indefinitely, that worker slot is leaked permanently, because cancelling the awaiting coroutine does not reclaim the thread.
Demonstrated:
stuck = threading.Event()
def blocker():
entered.set(); stuck.wait() # simulates an uninterruptible NFS/FUSE read
t = asyncio.create_task(asyncio.to_thread(blocker))
t.cancel()
# -> caller cancel raised CancelledError -> caller is free
# -> non-main threads AFTER cancel : 1 ['asyncio_0']
# >>> worker STILL stuck (slot leaked): TrueEnough concurrent stalls exhaust the pool and every to_thread user in the process stops making progress — including unrelated subsystems.
Scope
| Metric |
Count |
asyncio.to_thread call sites in src/ |
66 |
Bounded by a caller-side wait_for |
1 |
So 65 of 66 offloads are unbounded.
Why a timeout is not the fix
asyncio.wait_for around to_thread bounds the caller, not the worker. The proof above shows the thread stays alive and stuck after the caller has already been released. A timeout converts an indefinite caller hang into a prompt error — worth having for responsiveness — but it does not return the slot to the pool, so it does not address starvation. Adding one and calling it a mitigation would be misleading.
Proposed fix
Isolate blast radius rather than trying to cancel the uncancellable:
- Introduce a dedicated, explicitly-sized
ThreadPoolExecutor for file/blocking I/O offloads, so a stall can exhaust only that pool and never the shared default one the rest of the process depends on.
- Route offloads through a small helper (
run_blocking(...)) instead of calling asyncio.to_thread directly, giving one place to attach the executor, a caller-side deadline, and instrumentation.
- Emit a metric/log when the pool's queue depth or active count approaches capacity, so exhaustion is observable before it becomes an outage.
- Migrate call sites incrementally, highest-risk first (paths that read from potentially remote-backed mounts).
Acceptance criteria
Context
Raised during review of #1233 by the Linear agent, which correctly challenged the claim that a "local" file read has no unbounded-wait failure mode. A path under open() may resolve to NFS/FUSE/remote-backed storage and stall without limit.
Deliberately not fixed inside #1233: that PR's canonical issue (#1232) is scoped to moving the read off the event loop, and it is a strict improvement on this axis regardless (pre-change an NFS stall blocks the entire event loop; post-change it blocks one of 16 pool workers). Executor isolation is a separate, repo-wide architectural concern affecting all 66 call sites, and fixing it only for image reads would be arbitrary.
Problem
Every
asyncio.to_thread(...)call in this repo runs on the shared default executor, whose worker count is bounded (min(32, cpu_count + 4)— 16 on a 12-core box). If a work item blocks indefinitely, that worker slot is leaked permanently, because cancelling the awaiting coroutine does not reclaim the thread.Demonstrated:
Enough concurrent stalls exhaust the pool and every
to_threaduser in the process stops making progress — including unrelated subsystems.Scope
asyncio.to_threadcall sites insrc/wait_forSo 65 of 66 offloads are unbounded.
Why a timeout is not the fix
asyncio.wait_foraroundto_threadbounds the caller, not the worker. The proof above shows the thread stays alive and stuck after the caller has already been released. A timeout converts an indefinite caller hang into a prompt error — worth having for responsiveness — but it does not return the slot to the pool, so it does not address starvation. Adding one and calling it a mitigation would be misleading.Proposed fix
Isolate blast radius rather than trying to cancel the uncancellable:
ThreadPoolExecutorfor file/blocking I/O offloads, so a stall can exhaust only that pool and never the shared default one the rest of the process depends on.run_blocking(...)) instead of callingasyncio.to_threaddirectly, giving one place to attach the executor, a caller-side deadline, and instrumentation.Acceptance criteria
_read_file_bytesconsumption in all three ofaws_rekognition.py,azure_vision.py,google_cloud.py.to_threadcall from completing.Context
Raised during review of #1233 by the Linear agent, which correctly challenged the claim that a "local" file read has no unbounded-wait failure mode. A path under
open()may resolve to NFS/FUSE/remote-backed storage and stall without limit.Deliberately not fixed inside #1233: that PR's canonical issue (#1232) is scoped to moving the read off the event loop, and it is a strict improvement on this axis regardless (pre-change an NFS stall blocks the entire event loop; post-change it blocks one of 16 pool workers). Executor isolation is a separate, repo-wide architectural concern affecting all 66 call sites, and fixing it only for image reads would be arbitrary.