diff --git a/.changeset/fix-validator-memory-leak.md b/.changeset/fix-validator-memory-leak.md new file mode 100644 index 0000000000..93213a629b --- /dev/null +++ b/.changeset/fix-validator-memory-leak.md @@ -0,0 +1,5 @@ +--- +"@modelcontextprotocol/core": patch +--- + +Cache compiled Ajv validator instances to prevent memory leak from repeated schema compilation diff --git a/packages/core-internal/src/validators/ajvProvider.ts b/packages/core-internal/src/validators/ajvProvider.ts index e33adb741f..0ef61e84d8 100644 --- a/packages/core-internal/src/validators/ajvProvider.ts +++ b/packages/core-internal/src/validators/ajvProvider.ts @@ -10,6 +10,8 @@ import _addFormats from 'ajv-formats'; import { declaredDialect } from './dialects'; import type { JsonSchemaType, JsonSchemaValidator, jsonSchemaValidator, JsonSchemaValidatorResult } from './types'; +const MAX_CACHE_SIZE = 1000; + /** Structural subset of the AJV interface used by {@link AjvJsonSchemaValidator}. */ interface AjvLike { compile: (schema: unknown) => AjvValidateFunction; @@ -83,6 +85,19 @@ export class AjvJsonSchemaValidator implements jsonSchemaValidator { private _ajv2019: AjvLike | undefined; /** True iff the constructor received a caller-supplied engine; the `$schema` dispatch is skipped. */ private readonly _userAjv: boolean; + /** + * Content-keyed cache for compiled validators of schemas without `$id`. + * Prevents the memory leak where `engine.compile(schema)` is called unconditionally, + * causing Ajv's internal scope to grow without bound in long-running processes. + * + * Capped at {@link MAX_CACHE_SIZE} entries. Servers with a fixed tool set (the + * overwhelmingly common shape) converge well below this limit. Beyond the cap, + * new schemas are compiled but not stored in this Map. Note that Ajv's internal + * scope still retains compilation metadata in that case — the cap bounds our + * external allocation, not Ajv's internals. Truly-distinct-per-request schemas + * are an antipattern regardless; this cache targets the fixed-set case. + */ + private readonly _compiledCache: Map = new Map(); /** * @param ajv - Optional pre-configured AJV-compatible instance. When supplied, this instance is @@ -128,12 +143,41 @@ export class AjvJsonSchemaValidator implements jsonSchemaValidator { return (this._ajvDraft7 ??= createDefaultAjvInstance(Draft7Ajv)); } + /** + * Compile or retrieve a cached validator for the given schema. + * + * Schemas with `$id` use Ajv's built-in identity cache (`engine.getSchema`). + * Schemas without `$id` are cached by their JSON-serialised content to prevent + * unbounded growth of Ajv's internal scope in long-running processes (see #2605). + */ + private _getCompiled(schema: JsonSchemaType, engine: AjvLike): AjvValidateFunction { + if ('$id' in schema && typeof schema.$id === 'string') { + return engine.getSchema(schema.$id) ?? engine.compile(schema); + } + + let key: string; + try { + key = JSON.stringify(schema); + } catch { + // Non-serialisable schema (e.g. cyclic): fall back to uncached compilation. + return engine.compile(schema); + } + + const cached = this._compiledCache.get(key); + if (cached !== undefined) { + return cached; + } + + const compiled = engine.compile(JSON.parse(key)); + if (this._compiledCache.size < MAX_CACHE_SIZE) { + this._compiledCache.set(key, compiled); + } + return compiled; + } + getValidator(schema: JsonSchemaType): JsonSchemaValidator { const engine = this._engineFor(schema); - const ajvValidator = - '$id' in schema && typeof schema.$id === 'string' - ? (engine.getSchema(schema.$id) ?? engine.compile(schema)) - : engine.compile(schema); + const ajvValidator = this._getCompiled(schema, engine); return (input: unknown): JsonSchemaValidatorResult => { const valid = ajvValidator(input); diff --git a/packages/core-internal/src/validators/cfWorkerProvider.ts b/packages/core-internal/src/validators/cfWorkerProvider.ts index fe876bf9b6..c2ce887826 100644 --- a/packages/core-internal/src/validators/cfWorkerProvider.ts +++ b/packages/core-internal/src/validators/cfWorkerProvider.ts @@ -13,6 +13,8 @@ import { Validator } from '@cfworker/json-schema'; import { declaredDialect } from './dialects'; import type { JsonSchemaType, JsonSchemaValidator, jsonSchemaValidator, JsonSchemaValidatorResult } from './types'; +const MAX_CACHE_SIZE = 1000; + /** * JSON Schema draft version supported by `@cfworker/json-schema`. */ @@ -52,6 +54,12 @@ export class CfWorkerJsonSchemaValidator implements jsonSchemaValidator { private readonly shortcircuit: boolean; /** Caller-supplied draft; when set, the `$schema` check is skipped (caller owns dialect). */ private readonly draft?: CfWorkerSchemaDraft; + /** + * Content-keyed cache for compiled validators of schemas without `$id`. + * Prevents redundant Validator instantiation in long-running processes (see #2605). + * Capped at {@link MAX_CACHE_SIZE} entries. + */ + private readonly _compiledCache: Map = new Map(); /** * Create a validator @@ -77,18 +85,45 @@ export class CfWorkerJsonSchemaValidator implements jsonSchemaValidator { return dialect === 'draft-7' ? '7' : dialect; } + /** + * Retrieve or create a cached Validator instance for the given schema. + * Schemas are cached by their JSON-serialised content to prevent redundant + * instantiation in long-running processes (see #2605). + */ + private _getValidator(schema: JsonSchemaType, draft: CfWorkerSchemaDraft): Validator { + let key: string; + try { + key = JSON.stringify(schema); + } catch { + // Non-serialisable schema: fall back to uncached instantiation. + return new Validator(schema as ConstructorParameters[0], draft, this.shortcircuit); + } + + // Include draft in the key since the same schema content under different drafts + // may validate differently. + const cacheKey = `${draft}:${key}`; + const cached = this._compiledCache.get(cacheKey); + if (cached !== undefined) { + return cached; + } + + const compiled = new Validator(JSON.parse(key) as ConstructorParameters[0], draft, this.shortcircuit); + if (this._compiledCache.size < MAX_CACHE_SIZE) { + this._compiledCache.set(cacheKey, compiled); + } + return compiled; + } + /** * Create a validator for the given JSON Schema * - * Unlike AJV, this validator is not cached internally - * * @param schema - Standard JSON Schema object * @returns A validator function that validates input data */ getValidator(schema: JsonSchemaType): JsonSchemaValidator { const draft = this.draft ?? this._draftFor(schema); // Cast to the cfworker Schema type - our JsonSchemaType is structurally compatible - const validator = new Validator(schema as ConstructorParameters[0], draft, this.shortcircuit); + const validator = this._getValidator(schema, draft); return (input: unknown): JsonSchemaValidatorResult => { const result = validator.validate(input); diff --git a/packages/core-internal/test/validators/validatorCaching.test.ts b/packages/core-internal/test/validators/validatorCaching.test.ts new file mode 100644 index 0000000000..43b4b2f8a5 --- /dev/null +++ b/packages/core-internal/test/validators/validatorCaching.test.ts @@ -0,0 +1,206 @@ +/** + * Tests for validator caching behaviour (fixes #2605: memory leak from + * unconditional recompilation of schemas without `$id`). + */ + +import { describe, expect, it } from 'vitest'; + +import { AjvJsonSchemaValidator } from '../../src/validators/ajvProvider'; +import { CfWorkerJsonSchemaValidator } from '../../src/validators/cfWorkerProvider'; +import type { JsonSchemaType } from '../../src/validators/types'; + +describe('AjvJsonSchemaValidator caching (#2605)', () => { + it('returns the same validator function for identical schemas without $id', () => { + const provider = new AjvJsonSchemaValidator(); + const schema: JsonSchemaType = { + type: 'object', + properties: { name: { type: 'string' } }, + required: ['name'] + }; + + // Call getValidator twice with structurally identical (but different object) schemas + const v1 = provider.getValidator(schema); + const v2 = provider.getValidator({ ...schema }); + + // Both should validate correctly + expect(v1({ name: 'Alice' }).valid).toBe(true); + expect(v2({ name: 'Alice' }).valid).toBe(true); + expect(v1({}).valid).toBe(false); + expect(v2({}).valid).toBe(false); + }); + + it('does not recompile when called repeatedly with the same schema content', () => { + let compileCount = 0; + const fakeEngine = { + compile: (schema: unknown) => { + compileCount++; + return Object.assign(() => true, { errors: undefined }); + }, + getSchema: () => undefined, + errorsText: () => '' + }; + const provider = new AjvJsonSchemaValidator(fakeEngine); + const schema: JsonSchemaType = { type: 'string' }; + + provider.getValidator(schema); + provider.getValidator(schema); + provider.getValidator({ type: 'string' }); + + // Should compile only once — subsequent calls use the cache + expect(compileCount).toBe(1); + }); + + it('recompiles when schema content changes', () => { + const provider = new AjvJsonSchemaValidator(); + + const v1 = provider.getValidator({ type: 'string' } as JsonSchemaType); + const v2 = provider.getValidator({ type: 'number' } as JsonSchemaType); + + expect(v1('hello').valid).toBe(true); + expect(v1(42).valid).toBe(false); + expect(v2(42).valid).toBe(true); + expect(v2('hello').valid).toBe(false); + }); + + it('schemas with $id still use Ajv built-in identity cache', () => { + const provider = new AjvJsonSchemaValidator(); + const schema: JsonSchemaType = { + $id: 'https://example.com/test-schema', + type: 'object', + properties: { x: { type: 'number' } } + }; + + const v1 = provider.getValidator(schema); + const v2 = provider.getValidator(schema); + + expect(v1({ x: 1 }).valid).toBe(true); + expect(v2({ x: 1 }).valid).toBe(true); + expect(v1({ x: 'nope' }).valid).toBe(false); + }); + + it('a mutated schema object produces a validator for the new content', () => { + const provider = new AjvJsonSchemaValidator(); + const schema: JsonSchemaType = { type: 'string' }; + + const v1 = provider.getValidator(schema); + expect(v1('hello').valid).toBe(true); + expect(v1(42).valid).toBe(false); + + // Mutate in place + (schema as Record).type = 'number'; + + const v2 = provider.getValidator(schema); + expect(v2(42).valid).toBe(true); + expect(v2('hello').valid).toBe(false); + }); + + it('shared cached validator does not have cross-call error pollution', () => { + const provider = new AjvJsonSchemaValidator(); + const schema: JsonSchemaType = { + type: 'object', + properties: { name: { type: 'string' } }, + required: ['name'] + }; + + const v1 = provider.getValidator(schema); + const v2 = provider.getValidator(schema); + + // Validate with invalid data on v1 + const r1 = v1({}); + expect(r1.valid).toBe(false); + expect(r1.errorMessage).toBeDefined(); + + // v2 should still validate correctly (not inheriting errors from v1) + const r2 = v2({ name: 'Bob' }); + expect(r2.valid).toBe(true); + }); + + it('works correctly across different dialect schemas', () => { + const provider = new AjvJsonSchemaValidator(); + + const schema2020: JsonSchemaType = { + $schema: 'https://json-schema.org/draft/2020-12/schema', + type: 'string' + }; + const schema07: JsonSchemaType = { + $schema: 'http://json-schema.org/draft-07/schema#', + type: 'string' + }; + + const v2020 = provider.getValidator(schema2020); + const v07 = provider.getValidator(schema07); + + expect(v2020('hello').valid).toBe(true); + expect(v07('hello').valid).toBe(true); + }); + + it('degrades gracefully when cache size cap is exceeded', () => { + const provider = new AjvJsonSchemaValidator(); + + // Generate 1010 distinct schemas (exceeding the 1000-entry cap) + for (let i = 0; i < 1010; i++) { + const schema: JsonSchemaType = { + type: 'object', + properties: { [`field_${i}`]: { type: 'string' } } + }; + const v = provider.getValidator(schema); + expect(v({ [`field_${i}`]: 'val' }).valid).toBe(true); + } + + // The first 1000 should still be cached (validate correctly) + const earlySchema: JsonSchemaType = { + type: 'object', + properties: { field_0: { type: 'string' } } + }; + const v = provider.getValidator(earlySchema); + expect(v({ field_0: 'hello' }).valid).toBe(true); + expect(v({ field_0: 123 }).valid).toBe(false); + }); +}); + +describe('CfWorkerJsonSchemaValidator caching (#2605)', () => { + it('returns the same validation result for identical schemas without $id', () => { + const provider = new CfWorkerJsonSchemaValidator(); + const schema: JsonSchemaType = { + type: 'object', + properties: { name: { type: 'string' } }, + required: ['name'] + }; + + const v1 = provider.getValidator(schema); + const v2 = provider.getValidator({ ...schema }); + + expect(v1({ name: 'Alice' }).valid).toBe(true); + expect(v2({ name: 'Alice' }).valid).toBe(true); + expect(v1({}).valid).toBe(false); + expect(v2({}).valid).toBe(false); + }); + + it('recompiles when schema content changes', () => { + const provider = new CfWorkerJsonSchemaValidator(); + + const v1 = provider.getValidator({ type: 'string' } as JsonSchemaType); + const v2 = provider.getValidator({ type: 'number' } as JsonSchemaType); + + expect(v1('hello').valid).toBe(true); + expect(v1(42).valid).toBe(false); + expect(v2(42).valid).toBe(true); + expect(v2('hello').valid).toBe(false); + }); + + it('caches per draft — same content with different drafts validates differently', () => { + const provider = new CfWorkerJsonSchemaValidator(); + + // A schema with prefixItems: under 2020-12 it's enforced, under draft-07 it's ignored + const schema2020: JsonSchemaType = { + $schema: 'https://json-schema.org/draft/2020-12/schema', + type: 'array', + prefixItems: [{ type: 'number' }, { type: 'string' }] + }; + + const v2020 = provider.getValidator(schema2020); + // prefixItems is enforced under 2020-12 + expect(v2020([1, 'x']).valid).toBe(true); + expect(v2020(['x', 1]).valid).toBe(false); + }); +});