Skip to content

fix: resolve duplicate requests caused by concurrent message sending #333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

JooKS-me
Copy link
Contributor

@JooKS-me JooKS-me commented Jun 20, 2025

Motivation and Context

When calling the tools/call method of the same mcp async-client in parallel, we found that the client occasionally sent duplicate messages. We located that the same requestBuilder was used each time a request was made in the SDK, which was thread-unsafe.

How Has This Been Tested?

        ExecutorService threadPool = Executors.newFixedThreadPool(5);

        var transport = HttpClientSseClientTransport.builder("xxxxxx")
                .sseEndpoint("xxxxxx")
                .build();
        var asyncClient = McpClient.async(transport).build();

        asyncClient.initialize().block(Duration.ofSeconds(10));
        Map<String, Object> arguments = new HashMap<>();
        McpSchema.CallToolRequest request = new McpSchema.CallToolRequest("xxxxxx", arguments);

        Mono<McpSchema.CallToolResult> mono1 = asyncClient.callTool(request)
                .timeout(Duration.ofSeconds(30))
                .doOnSuccess(result -> {
                    System.out.println("--> get success response" + result);
                })
                .doOnError(error -> {
                    System.out.println("failed");
                    System.out.println(error.getMessage());
                });

        Mono<McpSchema.CallToolResult> mono2 = asyncClient.callTool(request)
                .timeout(Duration.ofSeconds(30))
                .doOnSuccess(result -> {
                    System.out.println("--> get success response" + result);
                })
                .doOnError(error -> {
                    System.out.println("failed");
                    System.out.println(error.getMessage());
                });

        Mono<McpSchema.CallToolResult> mono3 = asyncClient.callTool(request)
                .timeout(Duration.ofSeconds(30))
                .doOnSuccess(result -> {
                    System.out.println("--> get success response" + result);
                })
                .doOnError(error -> {
                    System.out.println("failed");
                    System.out.println(error.getMessage());
                });

        Mono<McpSchema.CallToolResult> mono4 = asyncClient.callTool(request)
                .timeout(Duration.ofSeconds(30))
                .doOnSuccess(result -> {
                    System.out.println("--> get success response" + result);
                })
                .doOnError(error -> {
                    System.out.println("failed");
                    System.out.println(error.getMessage());
                });

        Thread.sleep(10000);

        threadPool.submit(() -> {
            mono1.subscribe();
        });

        threadPool.submit(() -> {
            mono2.subscribe();
        });

        threadPool.submit(() -> {
            mono3.subscribe();
        });

        threadPool.submit(() -> {
            mono4.subscribe();
        });

Breaking Changes

No

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@JooKS-me JooKS-me force-pushed the feature/fix_conncurrent_build_request branch from 8066995 to 5fd0744 Compare June 20, 2025 09:11
@tzolov tzolov added this to the 0.11.0 milestone Jun 23, 2025
@tzolov tzolov self-assigned this Jun 23, 2025
@tzolov
Copy link
Contributor

tzolov commented Jun 23, 2025

Nice catch @JooKS-me , thank you.
Can you please run ./mvnw spring-javaformat:apply to resolve the formatting issues.

@JooKS-me
Copy link
Contributor Author

Nice catch @JooKS-me , thank you. Can you please run ./mvnw spring-javaformat:apply to resolve the formatting issues.

OK,it's done~

tzolov pushed a commit that referenced this pull request Jun 24, 2025
…333)

- Add .copy() call before modifying requestBuilder in FlowSseClient
- Add .copy() call before modifying requestBuilder in HttpClientSseClientTransport
- Prevents unintended mutations of shared builder instances
@tzolov
Copy link
Contributor

tzolov commented Jun 24, 2025

Thank you @JooKS-me

Rebased, squashed and merged at fce7a6b

@tzolov tzolov closed this Jun 24, 2025
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.

2 participants