Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions mcp/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,10 @@ type ioConn struct {
protocolVersion string
dropped map[jsonrpc2.ID]bool // lazily allocated

writeMu sync.Mutex // guards Write, which must be concurrency safe.
// writeMu is a 1-element semaphore guarding Write, which must be
// concurrency safe. Send to it to lock and receive from it to unlock.
// Unlike a sync.Mutex, waiting for it can stop when the context is done.
writeMu chan struct{}
rwc io.ReadWriteCloser // the underlying stream

// incoming receives messages from the read loop started in [newIOConn].
Expand Down Expand Up @@ -639,6 +642,7 @@ func newIOConnLimited(rwc io.ReadWriteCloser, maxLineLength int) *ioConn {
}
}()
return &ioConn{
writeMu: make(chan struct{}, 1),
rwc: rwc,
incoming: incoming,
closed: closed,
Expand Down Expand Up @@ -876,8 +880,12 @@ func (t *ioConn) Write(ctx context.Context, msg jsonrpc.Message) error {
default:
}

t.writeMu.Lock()
defer t.writeMu.Unlock()
select {
case t.writeMu <- struct{}{}:
case <-ctx.Done():
return ctx.Err()
}
defer func() { <-t.writeMu }()

// Batching support: if msg is a Response, it may have completed a batch, so
// check that first. Otherwise, it is a request or notification, and we may
Expand Down
52 changes: 52 additions & 0 deletions mcp/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ import (
"fmt"
"io"
"log/slog"
"net"
"strings"
"testing"
"testing/synctest"
"time"

"github.com/google/jsonschema-go/jsonschema"
Expand Down Expand Up @@ -886,3 +888,53 @@ func TestLoggingConnForwardsOptionalInterfaces(t *testing.T) {
t.Error("propagateCancellation() = true for a delegate without the interface, want false")
}
}

// TestIOConnCanceledQueuedWrite checks that a Write canceled while it waits for
// another Write returns without sending its message, and that the connection
// stays usable.
func TestIOConnCanceledQueuedWrite(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
local, peer := net.Pipe()
conn := newIOConn(local)
defer conn.Close()
defer peer.Close()

write := func(ctx context.Context, method string) <-chan error {
errc := make(chan error, 1)
go func() { errc <- conn.Write(ctx, &jsonrpc.Request{Method: method}) }()
return errc
}

// The peer is not reading yet, so the first Write blocks in the pipe.
first := write(context.Background(), "first")
synctest.Wait()

ctx, cancel := context.WithCancel(context.Background())
canceled := write(ctx, "canceled")
synctest.Wait()
cancel()
if err := <-canceled; !errors.Is(err, context.Canceled) {
t.Fatalf("canceled Write() = %v, want %v", err, context.Canceled)
}

second := write(context.Background(), "second")
frames := bufio.NewScanner(peer)
for _, want := range []string{"first", "second"} {
if !frames.Scan() {
t.Fatalf("peer read no frame, want %q: %v", want, frames.Err())
}
msg, err := jsonrpc2.DecodeMessage(frames.Bytes())
if err != nil {
t.Fatalf("decoding %q: %v", frames.Bytes(), err)
}
if got := msg.(*jsonrpc.Request).Method; got != want {
t.Fatalf("peer read %q, want %q", got, want)
}
}
for _, errc := range []<-chan error{first, second} {
if err := <-errc; err != nil {
t.Errorf("Write() = %v, want nil", err)
}
}
})
}
Loading