Skip to content

fix(mcp): catch RuntimeError when cancelling tasks on a closed event loop - #60032

Open
B416-JAFLY wants to merge 1 commit into
NousResearch:mainfrom
B416-JAFLY:fix/mcp-shutdown-runtime-error
Open

fix(mcp): catch RuntimeError when cancelling tasks on a closed event loop#60032
B416-JAFLY wants to merge 1 commit into
NousResearch:mainfrom
B416-JAFLY:fix/mcp-shutdown-runtime-error

Conversation

@B416-JAFLY

@B416-JAFLY B416-JAFLY commented Jul 7, 2026

Copy link
Copy Markdown

Problem

When Hermes exits and an MCP server is in the parked state (initial connection failed), the _wait_for_reconnect_or_shutdown and _wait_for_lifecycle_event methods' finally blocks call t.cancel() on pending asyncio tasks. A race in shutdown_mcp_servers() can cause the event loop to be closed by _stop_mcp_loop() before the finally block finishes executing:

  1. shutdown_mcp_servers() schedules _shutdown() on the MCP loop, with a 15-second timeout
  2. Inside _shutdown(), all servers are shut down in parallel via asyncio.gather
  3. If shutdown takes longer than 15s (e.g. a parked server is mid-cycle), future.result(timeout=15) times out
  4. _stop_mcp_loop() closes the event loop while the server's finally block is mid-execution
  5. t.cancel()call_soon()_check_closed() raises RuntimeError: Event loop is closed

This surfaces as an Exception ignored in warning during Python GC at exit:

Exception ignored in: <coroutine object MCPServerTask.run at 0x...>
Traceback (most recent call last):
  File "mcp_tool.py", line 2590, in run
    parked = await self._wait_for_reconnect_or_shutdown(...)
  File "mcp_tool.py", line 1884, in _wait_for_reconnect_or_shutdown
    t.cancel()
  File "base_events.py", line 762, in call_soon
    self._check_closed()
RuntimeError: Event loop is closed

The program shuts down correctly; the traceback is just noisy.

Fix

Wrap t.cancel() in try/except RuntimeError in both _wait_for_lifecycle_event and _wait_for_reconnect_or_shutdown. When the loop is already closed there is nothing to clean up, so silently passing is correct. The subsequent await t is already guarded by except (CancelledError, Exception) which catches RuntimeError as well.

Testing

Verified on macOS 15.7 with Python 3.11.15 — the RuntimeError traceback no longer appears on exit.

…loop

When the MCP event loop is closed during Hermes shutdown, the finally
blocks in _wait_for_lifecycle_event and _wait_for_reconnect_or_shutdown
call t.cancel() which schedules a callback via call_soon(). If the loop
has already been closed by _stop_mcp_loop(), this raises:

  RuntimeError: Event loop is closed

The exception propagates up through the parked coroutine and surfaces as
an 'Exception ignored in' warning during Python GC at exit. While the
program shuts down correctly, the traceback is noisy and confusing.

Catch RuntimeError around t.cancel() since there is nothing to clean up
when the loop is already closed.
Copilot AI review requested due to automatic review settings July 7, 2026 06:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces shutdown noise from the MCP background loop by preventing RuntimeError: Event loop is closed from surfacing during cleanup when cancelling pending asyncio tasks, particularly in the “parked” reconnect-wait paths.

Changes:

  • Wrap pending-task t.cancel() calls in _wait_for_lifecycle_event() and _wait_for_reconnect_or_shutdown() with try/except RuntimeError to avoid shutdown-time tracebacks.
  • Add inline comments clarifying the “loop already closed” shutdown race.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tools/mcp_tool.py
Comment on lines +1840 to +1843
try:
t.cancel()
except RuntimeError:
pass # Event loop already closed — nothing to clean up
Comment thread tools/mcp_tool.py
Comment on lines +1887 to +1890
try:
t.cancel()
except RuntimeError:
pass # Event loop already closed — nothing to clean up
@B416-JAFLY

Copy link
Copy Markdown
Author

撤回,等彻底解决后再提

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating the cancellation site. Current main still has the unguarded cancellation in the two targeted waiters, so the report is grounded.

Problems

  • The same pending-task cleanup remains unguarded in _wait_for_lazy_reconnect() at tools/mcp_tool.py:3120; the PR changes only the two other waiters.
  • The race originates higher in the shutdown lifecycle: shutdown_mcp_servers() proceeds to _stop_mcp_loop() after its bounded wait (tools/mcp_tool.py:5663-5667), and _stop_mcp_loop() closes the shared event loop (tools/mcp_tool.py:5822-5826). Catching at two leaf sites does not ensure pending MCP tasks are drained before that close.
  • The diff contains no regression test for this shutdown sequence.

Suggested changes

  • Use a bounded task-drain/cancellation step before closing the MCP loop, or cover all equivalent waiter cleanup paths consistently.
  • Add a regression that parks a waiter, triggers shutdown/loop close, and verifies cleanup produces no closed-loop traceback.

Automated hermes-sweeper review.

@shaiful-hisham

Copy link
Copy Markdown

Hi! The triage bot flagged that our PRs overlap — this is the closed-loop cancellation family (#60032, #60104, #60380, #80955).

My PR #80955 is a superset of this one: it covers your two call sites (_wait_for_lifecycle_event, _wait_for_reconnect_or_shutdown) plus two more (_wait_for_lazy_reconnect, _stop_mcp_loop drain path). The maintainers asked us to consolidate the preferred shutdown strategy.

Proposal: consolidate into #80955 so there's a single fix for the whole bug class. Would you be open to closing this PR in favor of it (or pointing me at anything in your change I should absorb)?

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/mcp MCP client and OAuth P2 Medium — degraded but workaround exists needs-decision Awaiting maintainer decision before any implementation and removed sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants