From c9be470319704782523d913a0b2444b687a17e95 Mon Sep 17 00:00:00 2001 From: MuriloFP Date: Fri, 7 Feb 2025 17:12:39 -0300 Subject: [PATCH] fix: prevent race conditions in McpServerManager singleton initialization Added thread-safe initialization to the McpServerManager singleton pattern to prevent potential race conditions when getInstance is called concurrently. The changes include: 1. Added initializationPromise to track ongoing initialization 2. Implemented double-checked locking pattern: - First check: Return existing instance if available - Second check: Wait for any ongoing initialization - Third check: Double-check inside initialization block 3. Added proper cleanup in finally block to prevent deadlocks This ensures that: - Only one McpHub instance is ever created - Concurrent calls wait for initialization to complete - Resources are properly initialized and cleaned up - No memory leaks from incomplete initialization The fix maintains the existing functionality while making it safe for concurrent access in VS Code's multi-window environment. --- src/services/mcp/McpServerManager.ts | 33 +++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/services/mcp/McpServerManager.ts b/src/services/mcp/McpServerManager.ts index 20ed2b8322..e15f9db0a7 100644 --- a/src/services/mcp/McpServerManager.ts +++ b/src/services/mcp/McpServerManager.ts @@ -10,21 +10,44 @@ export class McpServerManager { private static instance: McpHub | null = null private static readonly GLOBAL_STATE_KEY = "mcpHubInstanceId" private static providers: Set = new Set() + private static initializationPromise: Promise | null = null /** * Get the singleton McpHub instance. * Creates a new instance if one doesn't exist. + * Thread-safe implementation using a promise-based lock. */ static async getInstance(context: vscode.ExtensionContext, provider: ClineProvider): Promise { // Register the provider this.providers.add(provider) - if (!this.instance) { - this.instance = new McpHub(provider) - // Store a unique identifier in global state to track the primary instance - await context.globalState.update(this.GLOBAL_STATE_KEY, Date.now().toString()) + // If we already have an instance, return it + if (this.instance) { + return this.instance } - return this.instance + + // If initialization is in progress, wait for it + if (this.initializationPromise) { + return this.initializationPromise + } + + // Create a new initialization promise + this.initializationPromise = (async () => { + try { + // Double-check instance in case it was created while we were waiting + if (!this.instance) { + this.instance = new McpHub(provider) + // Store a unique identifier in global state to track the primary instance + await context.globalState.update(this.GLOBAL_STATE_KEY, Date.now().toString()) + } + return this.instance + } finally { + // Clear the initialization promise after completion or error + this.initializationPromise = null + } + })() + + return this.initializationPromise } /**