Skip to content

fix: clear leaked running loop in MCP client background thread #2111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mkmeral wants to merge 1 commit into
base: main
Choose a base branch
from
+39 −0
Open

fix: clear leaked running loop in MCP client background thread #2111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/strands/tools/mcp/mcp_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,9 @@ def _background_task(self) -> None:
This allows for a long-running event loop.
"""
self._log_debug_with_thread("setting up background task event loop")
# Clear any running-loop state leaked by OpenTelemetry's ThreadingInstrumentor, which wraps Thread.run()
# and can propagate the parent thread's event loop reference, causing run_until_complete() to fail.
asyncio._set_running_loop(None)
Copy link
Copy Markdown

@github-actions github-actions bot Apr 11, 2026

Choose a reason for hiding this comment

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

Important: asyncio._set_running_loop is a private/internal CPython API (prefixed with _). While it's been stable across Python 3.10–3.13 and is the correct low-level mechanism for this, it's worth noting the risk: this could change in a future CPython release without deprecation notice.

Suggestion: Consider adding a defensive wrapper or at minimum a brief inline comment noting the Python version range where this is known to work, so future maintainers know to verify if the minimum supported Python version changes. For example:

# asyncio._set_running_loop is a CPython internal (stable since 3.6, used by event loops themselves).
# Required to clear loop state leaked by OpenTelemetry's ThreadingInstrumentor.
asyncio._set_running_loop(None)

This is non-blocking — the existing comment is already good context about why, just suggesting adding a note about the private API nature.

self._background_thread_event_loop = asyncio.new_event_loop()
asyncio.set_event_loop(self._background_thread_event_loop)
self._background_thread_event_loop.run_until_complete(self._async_background_thread())
Expand Down
36 changes: 36 additions & 0 deletions tests/strands/tools/mcp/test_mcp_client_contextvar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,39 @@ def capturing_background_task(self):
)
# Verify it was indeed a different thread
assert background_thread_value["thread_id"] != main_thread_id, "Background task should run in a different thread"


def test_mcp_client_clears_running_loop_in_background_thread(mock_transport, mock_session):
"""Test that _background_task clears any leaked running event loop state.

When OpenTelemetry's ThreadingInstrumentor is active, Thread.run() is wrapped to propagate
trace context, which can leak the parent thread's running event loop reference into child
threads. This causes "RuntimeError: Cannot run the event loop while another loop is running"
when the background thread calls run_until_complete().

This test simulates that scenario by setting a running loop before _background_task runs
and verifying it gets cleared.
"""
import asyncio
Copy link
Copy Markdown

@github-actions github-actions bot Apr 11, 2026

Choose a reason for hiding this comment

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

Issue: import asyncio is inside the function body, but the repository coding standards (AGENTS.md) require imports at the top of the file.

Suggestion: Move import asyncio to the top-level imports alongside contextvars and threading.


cleared_running_loop = {}

original_background_task = MCPClient._background_task

def simulating_otel_leak_background_task(self):
# Simulate OTEL ThreadingInstrumentor leaking the parent's running loop
fake_loop = asyncio.new_event_loop()
asyncio._set_running_loop(fake_loop) # type: ignore[attr-defined]

# Call the real _background_task — it should clear the leaked loop and succeed
try:
return original_background_task(self)
finally:
cleared_running_loop["success"] = True
fake_loop.close()

with patch.object(MCPClient, "_background_task", simulating_otel_leak_background_task):
with MCPClient(mock_transport["transport_callable"]) as client:
assert client._background_thread is not None

assert cleared_running_loop.get("success"), "_background_task should have run successfully despite leaked loop"
Loading