Repository navigation
Fix concurrent first requests to one schema failing with HTTP 500 - #10525
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved issues were identified that would block approval.
0 open findings
What changed in this PR
Fixes concurrent first-request failures by moving per-executor sessions into schema services and removing unsynchronized feature collection writes.
Changes:
- Registers
ExecutorSessionas a schema-scoped singleton for Core and Fusion. - Resolves HTTP and MCP handlers from schema services.
- Adds concurrency tests and removes
McpExecutorSession.
| File | Description |
|---|---|
| src/HotChocolate/Fusion/src/Fusion.AspNetCore/DependencyInjection/FusionServerServiceCollectionExtensions.cs | Updated as part of this pull request. |
| src/HotChocolate/AspNetCore/test/AspNetCore.Tests/HttpRequestExecutorProxyTests.cs | Updated as part of this pull request. |
| src/HotChocolate/AspNetCore/src/AspNetCore/Extensions/HotChocolateAspNetCoreServiceCollectionExtensions.cs | Updated as part of this pull request. |
| src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/HttpRequestExecutorProxy.cs | Updated as part of this pull request. |
| src/HotChocolate/Adapters/src/Adapters.Mcp.Core/Proxies/StreamableHttpHandlerProxy.cs | Updated as part of this pull request. |
| src/HotChocolate/Adapters/src/Adapters.Mcp.Core/Proxies/McpRequestExecutorProxy.cs | Updated as part of this pull request. |
| src/HotChocolate/Adapters/src/Adapters.Mcp.Core/Proxies/McpExecutorSession.cs | Updated as part of this pull request. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Patch coverage96.2% of changed lines covered (25/26)
Uncovered changed lines (JSON){
"sha": "23564430fc8f44a1e47bfae695ffd3f820bbdd53",
"files": [
{ "path": "src/HotChocolate/Adapters/src/Adapters.Mcp.Core/Proxies/StreamableHttpHandlerProxy.cs", "ranges": [[49, 49]] }
]
}Project coverage: 58.6% (314246/536469 lines) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ExecutorSession, registered as a singleton in the executor's schema services for Core and Fusion. Proxies no longer store the session inexecutor.Features, an unsynchronized collection shared by all endpoints of a schema. Concurrent first requests to two endpoints, such asMapGraphQLHttpandMapGraphQLSchema, no longer fail withOperations that change non-concurrent collections must have exclusive accessorFeature 'HotChocolate.AspNetCore.ExecutorSession' is not present, and executor swaps no longer write to the new executor from every proxy at once.StreamableHttpHandlerfrom schema services, and after a swap it notifies the previous executor's MCP sessions by reading their dictionary from that executor directly.McpExecutorSessionis removed.Test plan
HttpRequestExecutorProxyTests: proxies that share an executor return the same session, and 2 or 16 proxies starting together on each of 20 fresh executors return one session per executor. All three cases fail without the fix, and the concurrent case also reproduces the reported errors.ListTools_AfterSchemaUpdate_ReturnsUpdatedToolsfor the swap notification), Adapters.OpenApi.Tests, Fusion.AspNetCore.Tests, and both Azure Functions test projects pass on net10.0.Closes #10480