Skip to content

fix: Preserve thread interrupt status after catching InterruptedException #234

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

rohitdutt-04
Copy link

Motivation and Context

When the sendMessage method catches an InterruptedException, it previously swallowed the interrupt without re-asserting the thread’s interrupt status. This can break cooperative interruption in multi-threaded environments and was flagged by SonarQube as a violation of best practices. Re-asserting the interrupt ensures that higher-level code can still detect and respond to the interruption.

How Has This Been Tested?

The change has been verified by running a full SonarQube scan on the project after the update. No new issues or warnings were reported, confirming that the change resolves the previously flagged issue related to interrupt handling without introducing regressions.

Breaking Changes

No. This change only affects internal error handling in the sendMessage method and does not modify any public APIs or require changes in user code or configurations.

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

This change makes the SDK more robust by properly handling InterruptedException. According to standard Java practices, when an InterruptedException is caught, the thread’s interrupt status should be preserved — which we now do by calling Thread.currentThread().interrupt(). This also resolves a SonarQube warning related to rule java:S2142.

@chemicL
Copy link
Member

chemicL commented May 15, 2025

Although in principle the recommendation is right, I view this situation rather as a problem with the implementation of HttpClientSsseClientTransport. It is making a blocking call that can get interrupted inside a non-blocking, reactive method. That is something we should address and it has been raised as #115.

@chemicL chemicL closed this May 15, 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