Skip to content

Fix resource leak in session storage cleanup#4014

Open
yrobla wants to merge 1 commit intomainfrom
issue-3871
Open

Fix resource leak in session storage cleanup#4014
yrobla wants to merge 1 commit intomainfrom
issue-3871

Conversation

@yrobla
Copy link
Contributor

@yrobla yrobla commented Mar 5, 2026

Implements fix by calling Close() on sessions that implement io.Closer before deleting them from storage during cleanup. This prevents connection leaks when vMCP sessions with backend clients expire.

Closes: #3871

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d8dad4ce9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes session-expiry resource leaks by ensuring expiring sessions that own resources are closed before removal from in-memory storage, with accompanying unit tests validating the new cleanup behavior.

Changes:

  • Update LocalStorage.DeleteExpired() to call Close() on sessions that implement io.Closer before deleting them.
  • Add unit tests covering io.Closer sessions, non-closer sessions, and Close() error handling.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
pkg/transport/session/storage_local.go Adds io.Closer detection and Close() invocation during expired-session cleanup, with warning logs on close failure.
pkg/transport/session/storage_test.go Introduces a closable mock session and adds test cases for closer/non-closer deletion and close-error behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Mar 5, 2026

Codecov Report

❌ Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.56%. Comparing base (f0f55c1) to head (b2e839b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/transport/session/storage_local.go 89.47% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4014      +/-   ##
==========================================
- Coverage   68.60%   68.56%   -0.05%     
==========================================
  Files         444      444              
  Lines       45184    45202      +18     
==========================================
- Hits        31000    30993       -7     
- Misses      11783    11812      +29     
+ Partials     2401     2397       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added the size/M Medium PR: 300-599 lines changed label Mar 5, 2026
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Mar 6, 2026
@yrobla yrobla requested a review from Copilot March 6, 2026 10:42
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Mar 6, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Implements fix by calling Close() on sessions that implement
io.Closer before deleting them from storage during cleanup.
This prevents connection leaks when vMCP sessions with backend clients
expire.

Closes: #3871
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[vMCP] Fix resource leak in session storage cleanup (Phase 3)

3 participants