You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
McpTransportContext.create documents an unmodifiable context but stores the caller's map by reference #1077
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
Build a mutable Map<String, Object> and put an entry in it.
Pass it to McpTransportContext.create(...).
Mutate the map: overwrite the entry, add another, or clear it.
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.
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.
Activity