Merge pull request #62 from MarcSkovMadsen/copilot/sub-pr-61

Fix race condition in singleton client initialization

Author: MarcSkovMadsen

src/holoviz_mcp/client.py

@@ -14,6 +14,7 @@
 >>> result = await call_tool("docs_search", {"query": "Button"})
 """
 
+import asyncio
 from typing import Any
 
 from fastmcp import Client
@@ -25,6 +26,7 @@
 __all__ = ["call_tool"]
 
 _CLIENT: Client | None = None
+_CLIENT_LOCK = asyncio.Lock()
 
 
 async def _setup_composed_server() -> None:
@@ -55,6 +57,9 @@ async def call_tool(tool_name: str, parameters: dict[str, Any]) -> CallToolResul
     server initialization. The first call will initialize the server and
     create a client; subsequent calls reuse the same client.
 
+    The client initialization is protected by an asyncio.Lock to prevent
+    race conditions when multiple tasks call this function concurrently.
+
     Parameters
     ----------
     tool_name : str
@@ -81,8 +86,9 @@ async def call_tool(tool_name: str, parameters: dict[str, Any]) -> CallToolResul
     >>> result = await call_tool("docs_get_best_practices", {"project": "panel"})
     """
     global _CLIENT
-    if _CLIENT is None:
-        _CLIENT = await _create_client()
+    async with _CLIENT_LOCK:
+        if _CLIENT is None:
+            _CLIENT = await _create_client()
 
     async with _CLIENT:
         return await _CLIENT.call_tool(tool_name, parameters)

tests/test_client.py

@@ -0,0 +1,84 @@
+"""
+Integration tests for the client module.
+
+These tests verify the functionality of the client module, including
+the singleton client pattern and its thread safety under concurrent access.
+"""
+
+import asyncio
+
+import pytest
+
+from holoviz_mcp.client import call_tool
+
+
+class TestClientSingleton:
+    """Tests for the singleton client pattern."""
+
+    @pytest.mark.asyncio
+    async def test_concurrent_client_initialization(self):
+        """Test that concurrent calls to call_tool don't create multiple clients.
+
+        This test verifies that the singleton client pattern handles race
+        conditions properly when multiple async tasks call call_tool
+        concurrently before the client is initialized. The asyncio.Lock
+        ensures only one task initializes the client.
+        """
+        # Reset the client state for testing
+        # Note: This directly manipulates module state for testing purposes
+        import holoviz_mcp.client as client_module
+
+        client_module._CLIENT = None
+
+        # Create multiple concurrent tasks that all call call_tool
+        tasks = [
+            call_tool("panel_list_components", {}),
+            call_tool("panel_list_components", {}),
+            call_tool("panel_list_components", {}),
+            call_tool("panel_list_components", {}),
+            call_tool("panel_list_components", {}),
+        ]
+
+        # Run all tasks concurrently
+        results = await asyncio.gather(*tasks)
+
+        # All tasks should complete successfully
+        assert len(results) == 5
+        for result in results:
+            assert result is not None
+            # Results should contain component data
+            assert isinstance(result.data, list)
+
+    @pytest.mark.asyncio
+    async def test_client_reuse(self):
+        """Test that the client is reused across multiple calls."""
+        # First call
+        result1 = await call_tool("panel_list_components", {})
+        assert result1 is not None
+
+        # Second call should reuse the same client
+        result2 = await call_tool("panel_list_components", {})
+        assert result2 is not None
+
+        # Both should return valid data
+        assert isinstance(result1.data, list)
+        assert isinstance(result2.data, list)
+
+    @pytest.mark.asyncio
+    async def test_concurrent_tool_calls(self):
+        """Test multiple concurrent tool calls using the singleton client."""
+        # Create tasks that call different tools concurrently
+        tasks = [
+            call_tool("panel_list_components", {}),
+            call_tool("panel_list_packages", {}),
+            call_tool("panel_list_components", {}),
+        ]
+
+        # Run all tasks concurrently
+        results = await asyncio.gather(*tasks)
+
+        # All tasks should complete successfully
+        assert len(results) == 3
+        for result in results:
+            assert result is not None
+            assert isinstance(result.data, list)