[jira] [Commented] (CAMEL-23957) camel-openai: MCP reconnect mutates shared endpoint tool state without synchronization

["Matteo Minin (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CAMEL-23957?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18095302#comment-18095302 ] 

Matteo Minin commented on CAMEL-23957:
--------------------------------------

I've been working on this issue and submitted a PR on GitHub. 
I hope it will be useful.

> camel-openai: MCP reconnect mutates shared endpoint tool state without synchronization
> --------------------------------------------------------------------------------------
>
>                 Key: CAMEL-23957
>                 URL: https://issues.apache.org/jira/browse/CAMEL-23957
>             Project: Camel
>          Issue Type: Bug
>          Components: camel-openai
>    Affects Versions: 4.21.0
>            Reporter: Federico Mariani
>            Priority: Major
>              Labels: ai
>
> {{OpenAIEndpoint}} holds the MCP tool state in plain unsynchronized collections shared by all concurrent exchanges (lines 89-93): {{cachedMcpTools}} ({{ArrayList}}), {{toolClientMap}} ({{HashMap}}), {{returnDirectTools}} ({{HashSet}}), {{toolToServerName}} ({{HashMap}}).
> When {{mcpReconnect=true}} (the default), {{reconnectMcpServer}} (lines 288-352) structurally modifies all of them at runtime — {{toolClientMap.entrySet().removeIf(...)}}, {{cachedMcpTools.removeIf(...)}} / {{addAll(...)}}, {{toolClientMap.put(...)}} — while other exchanges concurrently:
> * iterate {{getMcpTools()}} in {{OpenAIProducer.processInternal}} (for-each over the same {{ArrayList}}) to build the request → {{ConcurrentModificationException}} or a request with a partially rebuilt tool list;
> * read {{toolClientMap}} (unsynchronized {{HashMap}} read during structural modification is undefined behavior);
> * potentially enter {{reconnectMcpServer}} for the same server simultaneously (no mutual exclusion), closing each other's fresh clients and leaking connections/stdio subprocesses.
> Additional inconsistencies in the same code path:
> * {{returnDirectTools}} entries of the old server are never removed on reconnect — if a tool dropped its {{returnDirect}} annotation (or disappeared), the stale entry keeps short-circuiting the loop;
> * {{toolToServerName}} keeps entries for tools that no longer exist after re-listing.
> This is the same defect class as CAMEL-23945 (producer state race in camel-langchain4j-agent). Producers are endpoint singletons and {{process()}} runs concurrently, so this is reachable in any multi-threaded route that hits an MCP transport error.
> h3. Suggested fix
> Guard the reconnect with a per-server lock ({{ReentrantLock}} per the project's virtual-thread guidance, not {{synchronized}}), swap the collections atomically (build new immutable snapshots and replace {{volatile}} references instead of mutating in place), and clean {{returnDirectTools}}/{{toolToServerName}} for the reconnected server. This fits naturally into the {{McpServerManager}} extraction proposed by CAMEL-23396 and should be coordinated with the parallel-tool-execution work in CAMEL-23078, which will widen the concurrency window.
> _This issue was drafted by Claude Code on behalf of Federico Mariani_



--
This message was sent by Atlassian Jira
(v8.20.10#820010)