Skip to content

fix: replace blocking time.sleep with asyncio.sleep in async context (closes #125)#149

Open
geored wants to merge 2 commits into
mainfrom
fix/async-sleep-125
Open

fix: replace blocking time.sleep with asyncio.sleep in async context (closes #125)#149
geored wants to merge 2 commits into
mainfrom
fix/async-sleep-125

Conversation

@geored

@geored geored commented Jun 24, 2026

Copy link
Copy Markdown
Member

Root Cause

time.sleep(2) in _setup_port_forward (line 520) is a synchronous C-level blocking call inside an async def function. It freezes the entire asyncio event loop for 2 seconds, stalling all concurrent MCP tool requests during local development port-forwarding.

Fix

Approach 2 (Defensive) — selected from 3 simulated approaches:

  1. Replaced time.sleep(2) with await asyncio.sleep(2) — yields control to the event loop during the wait
  2. Added import asyncio (was not previously imported)
  3. Removed import time (verified: only usage in file was this sleep call)
  4. Added explanatory comment documenting the async constraint

Why This Approach

  • Approach 1 (Minimal) rejected: leaves dead import time and no documentation
  • Approach 3 (Refactor) rejected: converting subprocess.Popen to asyncio.create_subprocess_exec breaks stop_port_forward (sync method calling .wait(timeout=5)) and the type annotation at line 93

Investigation Trace

  1. Confirmed import time at line 19, asyncio NOT imported
  2. Found time.sleep(2) at line 520 inside async def _setup_port_forward
  3. Searched all time. usages — only one (line 520). Lines 1652/1660/1667 use datetime, not time
  4. Verified removing import time is safe
  5. Verified subprocess import must stay (15 usages across the file)

Closes #125

geored and others added 2 commits June 24, 2026 15:45
…loses #125)

time.sleep(2) in _setup_port_forward blocks the entire asyncio event loop,
freezing all concurrent MCP tool requests for 2 seconds. Replaced with
await asyncio.sleep(2) which yields control back to the event loop.

Also removed unused `import time` (only usage was this sleep call)
and added explanatory comment for future maintainers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[LOW] Blocking time.sleep(2) in async context

1 participant