Add option for immediate execution in McpSyncServer#371
Merged
tzolov merged 1 commit intomodelcontextprotocol:mainfrom Jul 8, 2025
Merged
Conversation
Member
|
Thanks for this, @Kehrlann :) Let me think tomorrow whether we can add a test for it. |
29f7c2a to
ab675d1
Compare
29e17d3 to
eabc24a
Compare
- The McpSyncServer wraps an async server. By default, reactive operations are scheduled on a bounded-elastic scheduler, to offload blocking work and prevent accidental blocking of non-blocking operations. - With the default behavior, there will be thead ops, even in a blocking context, which means thread-locals from the request thread will be lost. This is inconenvient for frameworks that store state in thread-locals. - This commit adds the ability to avoid offloading, when the user is sure they are executing code in a blocking environment. Work happens in the calling thread, and thread-locals are available throughout the execution.
eabc24a to
46a8562
Compare
chemicL
approved these changes
Jul 8, 2025
Member
|
LGTM :) In the future we could provide Schedulers.immediate() instead of the flag, but accepting a Scheduler should not be in the interfaces of an SPI module, but an implementation that uses Reactor. For the current state of things, it looks great, thanks! |
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.
Add the ability to decide whether or not to use blocking code offloading in
McpSyncServer. Turning it off allows for the propagation ofThreadLocals.Motivation and Context
I use this with Spring Security. Authentication data about the JWT that was used in the request is stored in a ThreadLocal. With
.subscribeOn(Schedulers.boundedElastic())), thread hops are introduced in blocking scenarios, and thread locals are lost.In select case, when the user is sure they are NOT in a reactive context, they might want to enable "immediate execution", on the thread that calls
.block(), for example in theMcpSyncServerExchange, and preserve thread locals.How Has This Been Tested?
Trying this change with a Spring-based sample, using Spring Security to pass the
SecurityContextand authentication information.Breaking Changes
None
Types of changes
Checklist
Additional context