Skip to content

Commit 255b767

Browse files
authored
fix(mcp): skip MCPClient cleanup during interpreter finalization (#2144)
Co-authored-by: minorun365 <minorun365@users.noreply.github.com>
1 parent c723e52 commit 255b767

2 files changed

Lines changed: 37 additions & 0 deletions

File tree

src/strands/tools/mcp/mcp_client.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import contextvars
1313
import json
1414
import logging
15+
import sys
1516
import threading
1617
import uuid
1718
from asyncio import AbstractEventLoop
@@ -343,6 +344,15 @@ def stop(self, exc_type: BaseException | None, exc_val: BaseException | None, ex
343344
"""
344345
self._log_debug_with_thread("exiting MCPClient context")
345346

347+
# Skip cleanup during interpreter finalization. On Python 3.14+, joining a
348+
# non-daemon thread at shutdown raises PythonFinalizationError; even though
349+
# our background thread is a daemon and will be reclaimed automatically,
350+
# the join call itself produces noisy tracebacks on stderr when the GC
351+
# reaches Agent.__del__ during finalization. See issue #2143.
352+
if sys.is_finalizing():
353+
self._log_debug_with_thread("interpreter is finalizing, skipping MCPClient cleanup")
354+
return
355+
346356
# Only try to signal close future if we have a background thread
347357
if self._background_thread is not None:
348358
# Signal close future if event loop exists

tests/strands/tools/mcp/test_mcp_client.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,33 @@ def test_stop_closes_event_loop():
594594
assert client._background_thread_event_loop is None
595595

596596

597+
def test_stop_skips_cleanup_during_interpreter_finalization():
598+
"""Test that stop() is a no-op when the interpreter is finalizing.
599+
600+
On Python 3.14+, threading.Thread.join() raises PythonFinalizationError at
601+
shutdown. The background thread is a daemon and is reclaimed automatically,
602+
so stop() should skip join() and event loop cleanup to avoid noisy
603+
tracebacks surfaced via Agent.__del__ during GC. See issue #2143.
604+
"""
605+
client = MCPClient(MagicMock())
606+
607+
mock_thread = MagicMock()
608+
mock_event_loop = MagicMock()
609+
client._background_thread = mock_thread
610+
client._background_thread_event_loop = mock_event_loop
611+
612+
with patch("strands.tools.mcp.mcp_client.sys.is_finalizing", return_value=True):
613+
# Must not raise, and must not touch the thread or event loop.
614+
client.stop(None, None, None)
615+
616+
mock_thread.join.assert_not_called()
617+
mock_event_loop.close.assert_not_called()
618+
# State is intentionally left alone during finalization — the interpreter
619+
# is going away and cleanup is unnecessary.
620+
assert client._background_thread is mock_thread
621+
assert client._background_thread_event_loop is mock_event_loop
622+
623+
597624
def test_mcp_client_state_reset_after_timeout():
598625
"""Test that all client state is properly reset after timeout."""
599626

0 commit comments

Comments
 (0)