FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: implement size-limited LRU cache for JSON Schema validation by joaquinescalante23 · Pull Request #845 · modelcontextprotocol/java-sdk · GitHub

Repository navigation

feat: implement size-limited LRU cache for JSON Schema validation - #845

Open
joaquinescalante23 wants to merge 1 commit into
modelcontextprotocol:mainfrom
joaquinescalante23:fix/json-schema-cache-resilience
Open

joaquinescalante23 wants to merge 1 commit into
modelcontextprotocol:mainfrom
joaquinescalante23:fix/json-schema-cache-resilience

Conversation

Copy link
Copy Markdown

I noticed that the current DefaultJsonSchemaValidator implementations (Jackson 2 and 3) were using an unbounded ConcurrentHashMap for schema caching, which could lead to memory leaks in long-running processes if exposed to a large number of dynamic schemas.

This PR addresses the TODO in both modules by:

  • Replacing the unbounded map with a thread-safe LRU Cache (defaulting to 1024 entries) using LinkedHashMap.
  • Making the cache size configurable via constructors for greater flexibility.
  • Improving the generateCacheKey logic to use a stable JSON representation of the schema instead of the default hashCode(), preventing potential collisions and ensuring cache hits are reliable regardless of map order.

These changes significantly improve the resilience and resource management of the SDK when validating complex MCP tool outputs.

Copy link
Copy Markdown
Author

I've been following the architectural changes proposed in PR #749 by @tzolov. While that PR focuses on the extensibility of the validator through suppliers, this PR addresses a critical resource management issue in the current default implementation.

Specifically, the existing DefaultJsonSchemaValidator (Jackson 2 and 3) lacks a cache eviction policy, which poses a memory exhaustion risk in long-running applications or when dealing with dynamic tool schemas. My proposal introduces a size-configurable LRU Cache that would make the new supplier-based architecture much more robust by default.

I'm happy to rebase or adjust this PR to align with the new supplier patterns once #749 is merged. Let me know if you'd like me to integrate these resilience improvements into the new supplier structure!

This branch has not been deployed

No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL