Skip to content

[Server] Give FileSessionStore a time budget for replacing a held session file on Windows - #726

Merged
chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:windows-failures
Nov 10, 2026
Merged

chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:windows-failures

Conversation

@chr-hertel

@chr-hertel chr-hertel commented Nov 10, 2026 •

Copy link
Copy Markdown
Member

Follow-up to #702 and #717. On Windows, rename() can't replace a file anyone has open, readers included. Under the concurrent writer test the target stayed held for up to ~240 ms, longer than the 10 attempts covered. Windows now retries the move for up to a second.

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The review identifies no active issues with the Windows session-file replacement retry changes.

No open issues 🎉

What changed in this PR

Improves Windows session-file replacement reliability by allowing retries long enough to cover the file contention observed after #702 and #717.

Changes:

  • Replaces the fixed 10-attempt limit with a one-second retry budget on Windows.
  • Preserves single-attempt replacement on other platforms and updates the changelog.
File Description
src/​Server/​Session/​FileSessionStore.php Uses a monotonic deadline for Windows replacement retries, with 1 ms sleeps between attempts.
CHANGELOG.md Documents the extended retry window for held session files on Windows.

Review effort: Balanced

@chr-hertel
chr-hertel merged commit bfc40f5 into modelcontextprotocol:main Nov 10, 2026
30 checks passed
@chr-hertel chr-hertel added the Server Issues & PRs related to the Server component label Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants