Skip to content

mcp: make waiting for the ioConn write lock cancelable - #1499

Merged
guglielmo-san merged 3 commits into
modelcontextprotocol:mainfrom
Tangerg:fix/ioconn-write-cancellation
Nov 6, 2026
Merged

guglielmo-san merged 3 commits into
modelcontextprotocol:mainfrom
Tangerg:fix/ioconn-write-cancellation

Conversation

@Tangerg

@Tangerg Tangerg commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

ioConn.Write checked its context only before waiting for writeMu, so a write canceled while another write was blocked kept waiting, and was sent once the lock was released.

writeMu is now a 1-element semaphore channel, like reqHeaderMu in the net/http HTTP/2 client, so a waiting write also selects on ctx.Done() and returns ctx.Err() without sending. jsonrpc2 already treats that error as a failure of the call rather than of the connection. Interrupting a write that is already in progress is left out, as discussed in review.

Without the change, the new test hangs with the canceled write blocked in sync.Mutex.Lock.

Updates #1498

AI assistance: prepared with Claude; I reviewed the change and can explain it.

ioConn.Write serialized writers with a mutex and checked ctx only before
waiting for it, so a write whose context was canceled while another write
was in progress still waited for that write and was then sent anyway. A
write blocked on a peer that stopped reading could not be interrupted at
all.

Queue writers on a one-slot channel so a canceled waiter returns without
sending. Canceling the active write closes the connection: bytes already
handed to the stream cannot be withdrawn, and a partial frame would
corrupt every later message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) label Nov 5, 2026
@github-actions github-actions Bot closed this Nov 5, 2026
@github-actions github-actions Bot reopened this Nov 5, 2026
@github-actions github-actions Bot removed the missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) label Nov 5, 2026
@guglielmo-san

Copy link
Copy Markdown
Contributor

@Tangerg let's scope this PR only to the blocking case. Exclude for now the case of the cancellation of on going call

As discussed in review, this change no longer interrupts a write that is
already in progress by closing the connection. It only stops a write
from waiting for an earlier one once its context is done.

writeMu keeps its name and becomes a 1-element semaphore channel, like
reqHeaderMu in the net/http HTTP/2 client. A canceled waiter returns
ctx.Err(), which jsonrpc2 already treats as a failure of the call rather
than of the connection. The regression test uses testing/synctest
instead of sleeping.

Updates modelcontextprotocol#1498

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Tangerg Tangerg changed the title mcp: let a canceled ioConn write leave the queue mcp: make waiting for the ioConn write lock cancelable Nov 6, 2026
@Tangerg

Tangerg commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

@guglielmo-san thanks, narrowed in a78ab1f. A write canceled while it waits for another write now returns ctx.Err() without being sent; a write already in progress is left alone. writeMu keeps its name and becomes a 1-element semaphore channel, so the diff stays small, and the test uses synctest instead of sleeping. The description now says Updates #1498, so the in-progress case stays open there. Ready for another look when you have time.

AI assistance: prepared with Claude; I reviewed it and can explain it.

@guglielmo-san
guglielmo-san merged commit 449d945 into modelcontextprotocol:main Nov 6, 2026
9 checks passed
Tangerg added a commit to Tangerg/go-sdk that referenced this pull request Nov 6, 2026
Sync 6 upstream commits through df0f295: the new
ParseURLElicitationRequiredError (modelcontextprotocol#1495), the tools/call retry on
HeaderMismatch (modelcontextprotocol#1502), notifications without Mcp-Method (modelcontextprotocol#1504), the
cancelable ioConn write lock (modelcontextprotocol#1499), invalid_token in 401 challenges
(modelcontextprotocol#1507) and the ioConn frame cap that no longer depends on read
boundaries (modelcontextprotocol#1510).

modelcontextprotocol#1499 and modelcontextprotocol#1510 are the upstream form of the fork's transport fixes, so
ioConn now matches upstream except that canceling a write already in
progress still closes the connection, because the peer may hold part of
the frame. Upstream's tests replace the fork's duplicates of them; the
fork keeps its test for the in-progress cancellation. New pkg.go.dev
links in the docs use the fork's module path.
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