Data race in ioConn.protocolVersion (concurrent read/write without synchronization)

[zesty-clawd]

Summary

A data race exists in ioConn.protocolVersion between sessionUpdated() and Read(), violating Go's memory model and potentially causing undefined behavior.

Location

mcp/transport.go

Details

The Race Condition

Write path (unsynchronized):

• ServerSession.updateState() → ioConn.sessionUpdated() → writes to c.protocolVersion (line 435)

Read path (unsynchronized):

• ioConn.Read() → reads t.protocolVersion (line 536)

Evidence

The ioConn struct has a writeMu mutex and batchMu mutex for other concurrent access:

type ioConn struct {
    protocolVersion string // ❌ NO SYNCHRONIZATION

    writeMu sync.Mutex         // ✅ guards Write
    // ...
    batchMu sync.Mutex         // ✅ guards batches map
    batches map[jsonrpc2.ID]*msgBatch
    // ...
}

The code comment at line 370-371 explicitly acknowledges concurrency:

"Since writes may be concurrent to reads, we need to guard this with a mutex."

Yet protocolVersion is accessed without any synchronization.

Impact

1. Go memory model violation: Concurrent read/write of non-atomic variables without synchronization is undefined behavior.
2. Potential corruption: String reads/writes in Go are not atomic; concurrent access can yield corrupted/torn values.
3. Race detector alerts: Running with -race flag will catch this.

Reproduction

The race occurs when:

1. A server calls updateState() (which triggers sessionUpdated())
2. Concurrently, Read() checks protocolVersion for batch validation

This is realistic because the Connection interface explicitly allows concurrent Read() and Close() (per interface docs).

Proposed Fix

Option 1: Use sync/atomic for protocolVersion:

type ioConn struct {
    protocolVersion atomic.Value // stores string
    // ...
}

func (c *ioConn) sessionUpdated(state ServerSessionState) {
    // ...
    c.protocolVersion.Store(negotiatedVersion(protocolVersion))
}

func (t *ioConn) Read(ctx context.Context) (jsonrpc.Message, error) {
    // ...
    pv := t.protocolVersion.Load().(string)
    if batch && pv >= protocolVersion20250618 {
        return nil, fmt.Errorf("JSON-RPC batching is not supported...")
    }
    // ...
}

Option 2: Reuse batchMu to guard protocolVersion reads/writes (simpler but broader lock scope).

References

• Go Memory Model: https://go.dev/ref/mem
• Related code comment acknowledging concurrent access: mcp/transport.go:370-371