Skip to content

Commit 4e2d615

Browse files
wukathcopybara-github
authored andcommitted
fix: Fix pickling lock errors in McpSessionManager
When we added the session_lock_map, it resulted in pickling errors during Agent Engine deployment. To fix, we implemented custom getstate and setstate methods to exclude the lock map lock and session lock map from pickling. Closes #4486. Co-authored-by: Kathy Wu <wukathy@google.com> PiperOrigin-RevId: 871056554
1 parent fc1f1db commit 4e2d615

File tree

2 files changed

+57
-0
lines changed

2 files changed

+57
-0
lines changed

src/google/adk/tools/mcp_tool/mcp_session_manager.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,30 @@ async def create_session(
500500
)
501501
raise ConnectionError(f'Failed to create MCP session: {e}') from e
502502

503+
def __getstate__(self):
504+
"""Custom pickling to exclude non-picklable runtime objects."""
505+
state = self.__dict__.copy()
506+
# Remove unpicklable entries or those that shouldn't persist across pickle
507+
state['_sessions'] = {}
508+
state['_session_lock_map'] = {}
509+
510+
# Locks and file-like objects cannot be pickled
511+
state.pop('_lock_map_lock', None)
512+
state.pop('_errlog', None)
513+
514+
return state
515+
516+
def __setstate__(self, state):
517+
"""Custom unpickling to restore state."""
518+
self.__dict__.update(state)
519+
# Re-initialize members that were not pickled
520+
self._sessions = {}
521+
self._session_lock_map = {}
522+
self._lock_map_lock = threading.Lock()
523+
# If _errlog was removed during pickling, default to sys.stderr
524+
if not hasattr(self, '_errlog') or self._errlog is None:
525+
self._errlog = sys.stderr
526+
503527
async def close(self):
504528
"""Closes all sessions and cleans up resources."""
505529
async with self._session_lock:

tests/unittests/tools/mcp_tool/test_mcp_session_manager.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,39 @@ async def test_close_skips_aclose_for_different_loop_sessions(self):
607607
exit_stack2.aclose.assert_not_called()
608608
assert len(manager._sessions) == 0
609609

610+
@pytest.mark.asyncio
611+
async def test_pickle_mcp_session_manager(self):
612+
"""Verify that MCPSessionManager can be pickled and unpickled."""
613+
import pickle
614+
615+
manager = MCPSessionManager(self.mock_stdio_connection_params)
616+
617+
# Access the lock to ensure it's initialized
618+
lock = manager._session_lock
619+
assert isinstance(lock, asyncio.Lock)
620+
621+
# Add a mock session to verify it's cleared on pickling
622+
manager._sessions["test"] = (Mock(), Mock(), asyncio.get_running_loop())
623+
624+
# Pickle and unpickle
625+
pickled = pickle.dumps(manager)
626+
unpickled = pickle.loads(pickled)
627+
628+
# Verify basics are restored
629+
assert unpickled._connection_params == manager._connection_params
630+
631+
# Verify transient/unpicklable members are re-initialized or cleared
632+
assert unpickled._sessions == {}
633+
assert unpickled._session_lock_map == {}
634+
assert isinstance(unpickled._lock_map_lock, type(manager._lock_map_lock))
635+
assert unpickled._lock_map_lock is not manager._lock_map_lock
636+
assert unpickled._errlog == sys.stderr
637+
638+
# Verify we can still get a lock in the new instance
639+
new_lock = unpickled._session_lock
640+
assert isinstance(new_lock, asyncio.Lock)
641+
assert new_lock is not lock
642+
610643

611644
@pytest.mark.asyncio
612645
async def test_retry_on_errors_decorator():

0 commit comments

Comments
 (0)