Skip to content

Add regression test for stateless request memory cleanup #1140

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

Merged
merged 1 commit into from
Jul 15, 2025
Merged

Conversation

felixweinberger
Copy link
Contributor

Summary

This PR adds a regression test for the memory leak fix from PR #1116. The test ensures that transport resources are properly cleaned up after handling stateless requests in the StreamableHTTPSessionManager.

Problem

PR #1116 identified a memory leak where transport instances were not being terminated after handling stateless requests. This could lead to resource accumulation over time as streams and other resources were not properly closed.

Solution

The test verifies that:

  • The transport's terminate() method is called after each stateless request
  • The transport's internal state (_terminated flag and _request_streams) is properly cleaned up
  • The test fails if the cleanup is not performed, providing regression protection

Test Implementation

The test creates real StreamableHTTPServerTransport instances and verifies the cleanup behavior rather than relying on mocks. This provides a more robust test that validates the actual behavior of the system.

Related to #1116

@felixweinberger
Copy link
Contributor Author

@hopeful0 FYI

@hopeful0
Copy link
Contributor

Thanks, I learned that the patch can helped us track down the transport, forgive my unfamiliarity with unittest.

This test verifies that PR #1116's fix properly cleans up transport
resources for stateless requests. The test mocks StreamableHTTPServerTransport
to track when _terminate_session() is called and ensures it's invoked for
each stateless request to prevent memory leaks.

Without the fix (commenting out the _terminate_session() call), this test
fails, confirming it properly detects the memory leak issue.

Github-Issue:#1116
Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

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

Thank you

@ihrpr ihrpr merged commit bd9885f into main Jul 15, 2025
22 of 23 checks passed
@ihrpr ihrpr deleted the pr-1116 branch July 15, 2025 08:29
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.

3 participants