Skip to content

fix: wake MCP session::recv when SSE stream closes - #69

Merged
ruoka merged 2 commits into
masterfrom
cursor/critical-bug-management-c962
Aug 2, 2026
Merged

fix: wake MCP session::recv when SSE stream closes#69
ruoka merged 2 commits into
masterfrom
cursor/critical-bug-management-c962

Conversation

@cursor

@cursor cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Bug and impact

http::mcp::session::recv() with the default unbounded timeout used cv.wait. That wait is only notified by POST enqueue or erase_session. When the SSE stream becomes not good (client disconnect after a write sets failbit, or any path that makes sse::session::closed() true), ready()'s m_stream.closed() clause was never re-checked.

Custom sse_transport::attach handlers that block in recv() can hang forever after the stream dies, leaving the SSE handler thread alive so http::server::stop()wait_for_handlers() never completes.

mcp::server::run_session already avoids this with a 1s timed recv + send_comment probe; the public session::recv() default did not.

Root cause

cv.wait only re-evaluates its predicate on notify. Stream close/failbit never notifies the session CV.

Fix

Slice the unbounded wait with wait_for (50ms) so m_stream.closed() is re-checked. Timed recv(timeout > 0) was already correct.

Complementary to open PR #68 (shutdown latch for poll-only closed()): #68 makes stop() visible without a write; this PR makes recv() notice once closed() is true.

Validation

  • Unit regression: unbounded recv() returns nullopt within 2s after badbit is set on the underlying stream (no CV notify).
  • Full MCP suite: test_case.*MCP — 25/25 passed (89 assertions).
Open in Web View Automation 

Note

Medium Risk
Changes blocking/wakeup semantics on the MCP SSE session path; incorrect behavior could still strand handler threads during shutdown, but the change is narrow and covered by a regression test.

Overview
Fixes a hang when session::recv() is called with the default unbounded timeout while the SSE stream dies without a condition-variable notify (client disconnect / failbit).

recv() no longer uses a single cv.wait for that path. It loops on wait_for(50ms) so ready() periodically re-evaluates m_stream.closed(), matching the behavior callers already get with a positive timeout. Timed recv is unchanged.

Adds a regression test: unbounded recv() on a stringstream-backed SSE session returns nullopt within 2s after badbit is set, with no POST or session erase.

Reviewed by Cursor Bugbot for commit d0e7e6c. Bugbot is set up for automated code reviews on this repo. Configure here.

@ruoka
ruoka force-pushed the cursor/critical-bug-management-c962 branch from b4034e0 to 970209c Compare August 2, 2026 01:36
@ruoka
ruoka marked this pull request as ready for review August 2, 2026 01:36
cursoragent and others added 2 commits August 2, 2026 04:37
Unbounded recv() used cv.wait, which is only notified by POST enqueue
or erase_session. Stream failure (client disconnect after write / failbit)
never re-checked m_stream.closed(), so attach handlers blocked forever
and http::server stop/join could hang. Slice the wait and add a unit
regression that sets badbit while recv is blocked.

Co-authored-by: Kaius  Ruokonen <ruoka@users.noreply.github.com>
Avoid ambiguous lookup against net::detail from structured_log_stream.

Co-authored-by: Kaius  Ruokonen <ruoka@users.noreply.github.com>
@ruoka
ruoka force-pushed the cursor/critical-bug-management-c962 branch from 970209c to d0e7e6c Compare August 2, 2026 01:37
@ruoka
ruoka merged commit 29d90c9 into master Aug 2, 2026
5 checks passed
@ruoka
ruoka deleted the cursor/critical-bug-management-c962 branch August 2, 2026 01:39
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