Skip to content

fix: make MCPManager singleton thread-safe with double-checked locking#813

Open
voidborne-d wants to merge 1 commit intoQwenLM:mainfrom
voidborne-d:fix/thread-safe-mcp-singleton
Open

fix: make MCPManager singleton thread-safe with double-checked locking#813
voidborne-d wants to merge 1 commit intoQwenLM:mainfrom
voidborne-d:fix/thread-safe-mcp-singleton

Conversation

@voidborne-d
Copy link
Copy Markdown

Summary

Fixes #812 — Makes the MCPManager singleton pattern thread-safe.

Problem

The MCPManager.__new__ method uses a singleton pattern that is not thread-safe:

def __new__(cls, *args, **kwargs):
    if cls._instance is None:
        cls._instance = super(MCPManager, cls).__new__(cls, *args, **kwargs)
    return cls._instance

In multi-threaded environments (e.g., when serving multiple users via Gradio WebUI or any ASGI server), two threads can both evaluate cls._instance is None as True simultaneously, creating duplicate instances. This breaks the singleton guarantee and can lead to:

  • Duplicated MCP server connections
  • Inconsistent state
  • Resource leaks

Solution

Add a class-level threading.Lock with double-checked locking:

_lock = threading.Lock()

def __new__(cls, *args, **kwargs):
    if cls._instance is None:           # Fast path: no lock needed
        with cls._lock:
            if cls._instance is None:   # Re-check after acquiring lock
                cls._instance = super(MCPManager, cls).__new__(cls, *args, **kwargs)
    return cls._instance

Why double-checked locking?

  • The outer check avoids lock acquisition on every call (fast path when instance exists)
  • The inner check prevents race conditions during first creation
  • threading is already imported in the module — zero new dependencies

Note: While Python's GIL prevents true parallel execution of bytecode, the GIL can release between the is None check and super().__new__() call (e.g., during I/O or C extension calls), making this race condition possible in practice.

The MCPManager.__new__ method uses a singleton pattern that is not
thread-safe. In multi-threaded environments (e.g., Gradio WebUI or
ASGI servers), concurrent threads can both evaluate _instance is None
as True, creating duplicate instances. This leads to duplicated MCP
server connections, inconsistent state, or resource leaks.

Fix: Add a class-level threading.Lock with double-checked locking
pattern - the fast path (instance already exists) remains lock-free,
while the slow path (first creation) is protected by the lock.

Fixes QwenLM#812
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Thread unsafe singleton pattern in MCPManager

1 participant