perf: isolate blocking file I/O from the shared default executor (#1234)

[groupthinking]

Fixes #1234

The defect

asyncio.to_thread(...) is literally loop.run_in_executor(None, ...). Both resolve to the same loop._default_executor object — verified at runtime, not assumed:

to_thread executor is run_in_executor(None) executor: True
_max_workers = 16          # min(32, cpu_count + 4)

A blocking read handed a path on NFS/FUSE/remote-backed storage can stall without limit. When it does, the worker slot is leaked permanently:

task = asyncio.create_task(asyncio.to_thread(blocking_read))
task.cancel()
# -> caller gets CancelledError
# -> worker STILL stuck (slot leaked): True     <- reproduced

concurrent.futures cannot interrupt a thread parked in a syscall. Cancellation frees the awaiting coroutine and nothing else. This is why timeouts are not the fix — a caller-side deadline bounds the coroutine, never the thread. Isolation is the only containment.

Blast radius is ~50% larger than the issue states

The issue counts 66 to_thread call sites. Measured on main:

Family	Sites	Files	Pool
asyncio.to_thread(...)	65	18	default
run_in_executor(...)	31	—	all 31 pass None -> default
Total exposed	96		one 16-worker pool

A helper that only replaces to_thread would leave 31 sites exposed. Worth scoping AC4's incremental migration to both families.

(The issue's "1 site bounded by a caller-side wait_for" claim is correct — azure_vision.py:272. It's only findable with a multiline grep, since asyncio.wait_for( and asyncio.to_thread( sit on separate lines. A single-line grep returns 0 and would wrongly suggest the issue was mistaken.)

The fix

New src/youtube_extension/utils/blocking_io.py:

• Process-wide, lazily-built, explicitly-sized ThreadPoolExecutor (BLOCKING_IO_MAX_WORKERS, default 8). Invalid values log and fall back rather than raising at import time.
• async run_blocking(func, *args, timeout=None, **kwargs).
• functools.partial binding, because run_in_executor accepts no kwargs.
• Throttled saturation warning (30s) with structured extra — AC3.
• reset_blocking_io_executor() uses shutdown(wait=False); blocking on a genuinely stuck worker would reintroduce the exact hang this module exists to contain.

One subtlety worth reviewing. In-flight accounting is attached to the concurrent future, not the asyncio one:

worker_future = executor.submit(call)
worker_future.add_done_callback(lambda _f: _exit())
awaitable = asyncio.wrap_future(worker_future)

A naive try/finally: _exit() decrements when the coroutine returns — so a caller released by timeout would stop counting a worker that is still stuck, hiding precisely the leaked slots the instrumentation exists to reveal. Cancelling the asyncio side does not fire the concurrent callback, so _in_flight reflects true worker occupancy.

Scope

Migrates only the three _read_file_bytes sites named in AC1 (Azure / Google / AWS vision). The remaining ~93 offloads are deliberately left for incremental migration — a repo-wide sweep would be unreviewable. Diff is +9 / −3 across the three providers.

azure_vision.py::_await_ocr_call is intentionally untouched: it wraps SDK calls, not file reads.

Verification

Tests pin isolation in both directions, because one direction alone is not enough:

• A (AC2 literal): saturate the I/O pool -> unrelated to_thread work still completes. Cannot fail on main — there is no I/O pool there.
• B (behavioral): saturate the default pool -> the vision reads still complete. This fails on main, so it is a genuine regression guard rather than an import smoke test.

Existing provider tests only assert reads happen off-loop. Running off-loop but still on the shared pool is this issue's failure mode, so that coverage does not subsume these tests.

Stage	Result
RED (module present, providers unmigrated)	4 failed with TimeoutError, 1 passed
GREEN	16 passed
Existing provider suites, A/B	313 passed both with and without the change
ruff, CI gate command	2 errors before and after -> pre-existing
ruff, the 3 providers	21 before and after -> zero added
ruff, new files	clean

Negative controls — each fails a distinct, targeted subset, so the suite discriminates rather than merely reddening:

Mutation	Expected	Result
NC-1 route offloads back to the default pool (the world without this fix)	isolation collapses	5 failed
NC-2 keep the helper, revert providers to to_thread	routing tests only	4 failed
NC-3 delete the saturation warning	AC3 test only	1 failed
restored	green	16 passed

NC-2 is the important one: it proves the tests assert routing, not merely that a helper exists.

Note for anyone writing similar saturation tests

Two harness bugs produced false REDs here — reds that failed for the wrong reason:

1. Event-loop starvation deadlock. asyncio.create_task(...) doesn't start the task until the loop regains control, so synchronously calling semaphore.acquire(timeout=...) right after blocks the loop and no hog ever reaches the pool. Fix: non-blocking acquire(blocking=False) in a poll loop with await asyncio.sleep(0.01). Runtime went 200s -> 0.3s.
2. loop.set_default_executor(None) raises TypeError. Teardown restoring a previously-None default executor crashes and masks the real assertion outcome. Fix: assign loop._default_executor = None directly in that case.

Both files carry an explicit positive control (assert_default_pool_is_blocked()) so they can never pass vacuously.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
v0-uvai	Ready	Preview, v0	Aug 2, 2026 3:35pm

[github-actions]

🔍 PR Validation

⚠️ Large PR detected (678 lines changed)

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 54408df.

Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

[coderabbitai]

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)

• [‘architecture-gap’, ‘bug’, ‘ci-cd’, ‘ci/cd’, ‘copilot-rabbit’, ‘documentation’, ‘duplicate’, ‘enhancement’, ‘frontend’, ‘github_actions’, ‘good first issue’, ‘help wanted’, ‘high-priority’, ‘invalid’, ‘javascript’, ‘ml-model’, ‘needs-triage’, ‘pipeline-critical’, ‘placeholder-code’, ‘priority:high’, ‘python’, ‘python:uv’, ‘question’, ‘styling’, ‘tests’, ‘v0’]

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4edff22e-686e-485e-bad0-17da2e6b4b00

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Agent Completion Truth Gate: BLOCKED

Reasons: invalid_payload

Machine-readable verdict

{
  "details": {
    "collection_errors": [
      "incomplete_linked_issue_contract",
      "missing_intent_snapshot",
      "missing_agent_run_id",
      "missing_agent_login"
    ],
    "invalid_fields": [
      "policy.agent_login",
      "policy.run_id"
    ]
  },
  "reasons": [
    "invalid_payload"
  ],
  "verdict": "blocked"
}

Workflow evidence

[groupthinking]

@copilot resolve the merge conflicts on this branch.