Skip to content

McpTransportContext.create documents an unmodifiable context but stores the caller's map by reference #1077

Description

@iuliiasobolevska

Bug description

McpTransportContext.create(Map) is documented to "create an unmodifiable context containing the given metadata":
https://www.javadocs.dev/io.modelcontextprotocol.sdk/mcp-core/2.0.0/io/modelcontextprotocol/common/McpTransportContext.html
It stores the caller's map by reference instead, with no copy and no unmodifiable wrapper. Mutating the map after the call changes the context, and because equals and hashCode delegate to the map, the context's identity moves with it.

Environment

  • MCP Java SDK 2.0.0, and main at fd00498 (2.0.1-SNAPSHOT). The two are identical here
  • Java 17 target, reproduced on Java 21
  • mcp-core only, no transport or vector store involved

Steps to reproduce

  1. Build a mutable Map<String, Object> and put an entry in it.
  2. Pass it to McpTransportContext.create(...).
  3. Mutate the map: overwrite the entry, add another, or clear it.
  4. Read the context back with get(...).

The context reflects every mutation.

Expected behavior

create(...) returns a context that is unmodifiable, as the javadoc says. One line does it:

DefaultMcpTransportContext(Map<String, Object> metadata) {
    Assert.notNull(metadata, "The metadata cannot be null");
    this.metadata = Map.copyOf(metadata);
}

Map.copyOf copies and makes the result unmodifiable. It also rejects null keys and values, so it is a behavior change for anyone who puts a null value in the map today. If that needs to keep working, Collections.unmodifiableMap(new HashMap<>(metadata)) does the same job without the restriction.

If storing by reference is deliberate, so a caller can update a context in place, then the javadoc is the thing to fix rather than the code. In that case the thread safety expectation is worth spelling out, because the context is read from transport worker threads and the field is neither volatile nor guarded.

Minimal Complete Reproducible example

Tests at #1076.

The SDK's test suite doesn't catch this because every caller of create(...) in the repo passes Map.of(...), which is already immutable.

Practical case: The context is captured on one thread and read on the HTTP client's worker threads during the SSE reconnect and the follow-up POST. A caller who builds a HashMap, passes it to create(...) and then reuses or clears it has a cross thread data race on a field with no volatile and no synchronization.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions