Skip to content

Add option for immediate execution in McpSyncServer #371

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Kehrlann
Copy link

@Kehrlann Kehrlann commented Jul 3, 2025

Add the ability to decide whether or not to use blocking code offloading in McpSyncServer. Turning it off allows for the propagation of ThreadLocals.

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 the McpSyncServerExchange, and preserve thread locals.

How Has This Been Tested?

Trying this change with a Spring-based sample, using Spring Security to pass the SecurityContext and authentication information.

Breaking Changes

None

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

@chemicL
Copy link
Member

chemicL commented Jul 3, 2025

Thanks for this, @Kehrlann :) Let me think tomorrow whether we can add a test for it.
@tzolov this is a change me and @Kehrlann worked on together. Instead of providing hooks for propagating ThreadLocal state, we instead just don't lose the ThreadLocal state by not offloading to the boundedElastic Scheduler. When using a sync server this should be a completely desired situation.

@Kehrlann Kehrlann force-pushed the dgarnier/server-context-propagation branch 2 times, most recently from 29f7c2a to ab675d1 Compare July 4, 2025 13:39
@Kehrlann Kehrlann marked this pull request as ready for review July 4, 2025 14:00
@Kehrlann Kehrlann force-pushed the dgarnier/server-context-propagation branch 2 times, most recently from 29e17d3 to eabc24a Compare July 4, 2025 14:08
- 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.
@Kehrlann Kehrlann force-pushed the dgarnier/server-context-propagation branch from eabc24a to 46a8562 Compare July 4, 2025 14:09
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