Skip to content

sse: Close does not drain incoming; Read can deliver stale messages after Close and POST can return false 202 #1191

Description

@junnhwan

What did you do?

I was reading the HTTP+SSE transport (mcp/sse.go) while evaluating the SDK, and noticed that sseServerConn.Close() closes the done channel but does not drain the buffered incoming channel. incoming has capacity 100 and is documented as intentionally "never closed" (sse.go:126). Both Read and the POST handler (ServeHTTP) use a select over incoming and done, so when a message is buffered and done is closed, the selection is non-deterministic.

I wrote a small reproduction (details below) to confirm.

What did you see?

  1. Read after Close returns stale messages. With one message buffered in incoming before Close, calling Read returns the buffered message instead of io.EOF roughly 50% of the time, because Go's select picks randomly among ready cases. I measured 1000/2000 deliveries.
  2. POST after Close returns 202 Accepted for messages that will never be processed. After Close, incoming (cap 100) is never drained, so the t.incoming <- msg case stays ready alongside <-t.done, and the POST handler sometimes returns 202. I measured 100/2000 false 202 responses.

What did you expect to see?

After Close, a Read should return io.EOF (the session is terminated — no post-close message delivery), and a POST should consistently return 400 session closed, since there is no live reader to consume the message. The current behavior gives the client a 202 ack for a message that is silently dropped.

Root cause

sse.go:348-356:

func (s *sseServerConn) Close() error {
	s.t.mu.Lock()
	defer s.t.mu.Unlock()
	if !s.t.closed {
		s.t.closed = true
		close(s.t.done)
	}
	return nil
}

Close closes done but leaves buffered messages in incoming. Read (sse.go:307-316):

func (s *sseServerConn) Read(ctx context.Context) (jsonrpc.Message, error) {
	select {
	case <-ctx.Done():
		return nil, ctx.Err()
	case msg := <-s.t.incoming:
		return msg, nil
	case <-s.t.done:
		return nil, io.EOF
	}
}

When both incoming and done are ready, the choice is random. The POST handler (sse.go:166-171) has the same shape:

select {
case t.incoming <- msg:
	w.WriteHeader(http.StatusAccepted)
case <-t.done:
	http.Error(w, "session closed", http.StatusBadRequest)
}

Reproduction

Both are deterministic (no -race needed):

// 1. Post-close stale Read delivery (~50%)
w := httptest.NewRecorder()
tr := &SSEServerTransport{Endpoint: "/messages?sessionid=x", Response: w}
conn, _ := tr.Connect(context.Background())
srv := conn.(*sseServerConn)
srv.t.incoming <- &jsonrpc.Request{Method: "ping"}
conn.Close()
// Loop Read 2000x, refilling incoming each time -> ~1000 stale deliveries.

// 2. Post-close false 202 (~5%)
// Close the transport, then POST 2000 messages -> ~100 return 202.

On current main (3d6450f), I measured:

  • stale deliveries after Close: 1000/2000 (~50%, matching the random select)
  • false 202 responses after Close: 100/2000 (~5%)

Suggested fix

Drain incoming in Close so no buffered message can be observed after close:

if !s.t.closed {
	s.t.closed = true
	close(s.t.done)
	// drain any buffered messages so Read cannot observe them after Close
	for {
		select {
		case <-s.t.incoming:
		default:
			return
		}
	}
}

This makes done the only ready case in Read after Close, and forces the POST handler's incoming <- msg to block so the <-t.done case fires and the client gets 400 session closed. An alternative is to check closed in Read/ServeHTTP before selecting on incoming; draining is the smaller change that closes both paths.

I'm happy to send a PR with a regression test, but wanted to file this first and get a read on the approach before working on it (per the contributing guide for changes not related to an existing issue).

Versions

  • github.com/modelcontextprotocol/go-sdk at main (3d6450f, 2026-08-21)
  • Go: go1.26.1 windows/amd64

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3Nice to haves, rare edge cases

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions