Skip to content

Conversation

rahulchittimalla
Copy link

@rahulchittimalla rahulchittimalla commented Jun 6, 2025

Motivation and Context

Currently, there is no explicit cleanup of the session_id from the _read_stream_writers. When the connection closes (client disconnects) the code closes the streams, but does not remove the session_id key from _read_stream_writers dictionary when the connection is closed.

How Has This Been Tested?

Breaking Changes

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

Copy link
Contributor

@felixweinberger felixweinberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @rahulchittimalla - thank you for your contribution! And apologies for the delay in getting back to you.

Could you provide some additional detail what problem you were running into without this? I understand the session_id not being removed from the _read_stream_writers but I'm curious whether this was actually causing any issues for you?

If this was causing bugs in production for you or if there's some reproducible problem as a result of this, would be great to add a test that demonstrates the broken behavior?

Happy to get back to this quickly if you have the bandwidth to update the PR here!

@felixweinberger felixweinberger added needs confirmation Needs confirmation that the PR is actually required or needed. needs more work Not ready to be merged yet, needs additional changes. labels Sep 17, 2025
@felixweinberger
Copy link
Contributor

Closing this one for now as it's been a while - we're currently going through older PRs to ensure we prioritize limited maintainer time appropriately.

Happy to reopen this if you come back to this at a later time to continue the discussion - in that case feel free to ping here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs confirmation Needs confirmation that the PR is actually required or needed. needs more work Not ready to be merged yet, needs additional changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants