diff --git a/agentscope-core/src/main/java/io/agentscope/core/util/JsonSchemaUtils.java b/agentscope-core/src/main/java/io/agentscope/core/util/JsonSchemaUtils.java index f15efe5984..30e620d2db 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/util/JsonSchemaUtils.java +++ b/agentscope-core/src/main/java/io/agentscope/core/util/JsonSchemaUtils.java @@ -29,6 +29,7 @@ import io.agentscope.core.tool.ToolSchemaModule; import java.lang.reflect.Type; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * Utility class for JSON Schema operations. @@ -53,6 +54,11 @@ *
  • {@code @JsonClassDescription(...)} - add class description
  • * * + *

    All public methods are thread-safe. Schema generation through the shared victools + * {@code SchemaGenerator} is serialized by an internal lock, because the generator itself + * is not designed for concurrent use. Generated schemas are cached per {@link Class}/{@link Type} + * so that the lock is only needed the first time a given class or type is seen.

    + * * @hidden */ public class JsonSchemaUtils { @@ -61,6 +67,29 @@ public class JsonSchemaUtils { private static final SchemaGenerator schemaGenerator; + /** + * Guards the shared victools {@link SchemaGenerator}, which is not thread-safe: its + * JacksonModule keeps an unsynchronized introspection cache, so concurrent schema + * generation must be serialized. Only cache misses in {@link #CLASS_SCHEMA_CACHE} and + * {@link #TYPE_SCHEMA_CACHE} take this lock. + */ + private static final Object SCHEMA_LOCK = new Object(); + + /** + * Caches the schema {@link JsonNode} generated for each class, since it is a deterministic + * function of the class and the static, never-changing generator config, so no invalidation + * is needed. Values are never mutated after being cached; every call still converts a fresh, + * independently mutable {@code Map} from the cached node. Unbounded, but keys are the + * compile-time-fixed structured-output and tool-parameter classes declared by application + * code, so the entry count is bounded by the (small, finite) set of classes the JVM loads for + * that purpose, not by request volume or untrusted input. + */ + private static final Map, JsonNode> CLASS_SCHEMA_CACHE = new ConcurrentHashMap<>(); + + /** Same caching strategy and bound rationale as {@link #CLASS_SCHEMA_CACHE}, keyed by + * generic {@link Type}. */ + private static final Map TYPE_SCHEMA_CACHE = new ConcurrentHashMap<>(); + static { // JacksonModule to support @JsonProperty, @JsonPropertyDescription annotations JacksonModule jacksonModule = @@ -95,7 +124,14 @@ public class JsonSchemaUtils { */ public static Map generateSchemaFromClass(Class clazz) { try { - JsonNode schemaNode = schemaGenerator.generateSchema(clazz); + JsonNode schemaNode = + CLASS_SCHEMA_CACHE.computeIfAbsent( + clazz, + c -> { + synchronized (SCHEMA_LOCK) { + return schemaGenerator.generateSchema(c); + } + }); return JsonUtils.getJsonCodec() .convertValue(schemaNode, new TypeReference>() {}); } catch (Exception e) { @@ -130,7 +166,14 @@ public static Map generateSchemaFromJsonNode(JsonNode schema) { */ public static Map generateSchemaFromType(Type type) { try { - JsonNode schemaNode = schemaGenerator.generateSchema(type); + JsonNode schemaNode = + TYPE_SCHEMA_CACHE.computeIfAbsent( + type, + t -> { + synchronized (SCHEMA_LOCK) { + return schemaGenerator.generateSchema(t); + } + }); return JsonUtils.getJsonCodec() .convertValue(schemaNode, new TypeReference>() {}); } catch (Exception e) { diff --git a/agentscope-core/src/test/java/io/agentscope/core/util/JsonSchemaUtilsTest.java b/agentscope-core/src/test/java/io/agentscope/core/util/JsonSchemaUtilsTest.java index c5efd261b9..bd3d341f65 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/util/JsonSchemaUtilsTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/util/JsonSchemaUtilsTest.java @@ -23,12 +23,23 @@ import com.fasterxml.jackson.core.type.TypeReference; import java.lang.reflect.Type; +import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.function.IntFunction; import org.junit.jupiter.api.Test; class JsonSchemaUtilsTest { + private static final int CONCURRENT_THREAD_COUNT = 12; + + private static final int CONCURRENT_CALL_COUNT = 240; + static class SimpleModel { public String name; public int age; @@ -138,6 +149,52 @@ void testConvertToObjectInvalidData() { () -> JsonSchemaUtils.convertToObject(invalidData, SimpleModel.class)); } + @Test + void testGenerateSchemaFromClassRepeatedCallsReturnEqualIndependentMaps() { + Map first = JsonSchemaUtils.generateSchemaFromClass(SimpleModel.class); + Map second = JsonSchemaUtils.generateSchemaFromClass(SimpleModel.class); + + assertEquals(first, second); + + // Mutating a schema returned from one call must not leak into a later call, matching + // callers (e.g. ToolSchemaGenerator) that mutate the returned map in place. + first.put("description", "mutated"); + assertTrue(!second.containsKey("description")); + + Map third = JsonSchemaUtils.generateSchemaFromClass(SimpleModel.class); + assertTrue(!third.containsKey("description")); + assertEquals(second, third); + } + + @Test + void testGenerateSchemaFromTypeRepeatedCallsReturnEqualIndependentMaps() { + Type listType = new TypeReference>() {}.getType(); + + Map first = JsonSchemaUtils.generateSchemaFromType(listType); + Map second = JsonSchemaUtils.generateSchemaFromType(listType); + + assertEquals(first, second); + + first.put("description", "mutated"); + assertTrue(!second.containsKey("description")); + + Map third = JsonSchemaUtils.generateSchemaFromType(listType); + assertTrue(!third.containsKey("description")); + assertEquals(second, third); + } + + @Test + void testGenerateSchemaFromClassNullThrowsNullPointerException() { + assertThrows( + NullPointerException.class, () -> JsonSchemaUtils.generateSchemaFromClass(null)); + } + + @Test + void testGenerateSchemaFromTypeNullThrowsNullPointerException() { + assertThrows( + NullPointerException.class, () -> JsonSchemaUtils.generateSchemaFromType(null)); + } + @Test void testGenerateSchemaFromType() { // Test List @@ -156,4 +213,92 @@ void testGenerateSchemaFromType() { assertNotNull(mapSchema); assertEquals("object", mapSchema.get("type")); } + + static class ConcurrentClassA { + public String name; + public int age; + } + + static class ConcurrentClassB { + public String title; + public List tags; + } + + static class ConcurrentClassC { + public String id; + public boolean active; + } + + static class ConcurrentClassD { + public double score; + } + + @Test + void testGenerateSchemaFromClassConcurrently() throws Exception { + List> targetClasses = List.of(ConcurrentClassA.class, ConcurrentClassB.class); + + List> schemas = + generateConcurrently( + index -> + JsonSchemaUtils.generateSchemaFromClass( + targetClasses.get(index % targetClasses.size()))); + + assertEquals(CONCURRENT_CALL_COUNT, schemas.size()); + for (Map schema : schemas) { + assertNotNull(schema); + assertEquals("object", schema.get("type")); + assertNotNull(schema.get("properties")); + } + } + + @Test + void testGenerateSchemaFromTypeConcurrently() throws Exception { + List targetTypes = + List.of( + new TypeReference() {}.getType(), + new TypeReference>() {}.getType()); + + List> schemas = + generateConcurrently( + index -> + JsonSchemaUtils.generateSchemaFromType( + targetTypes.get(index % targetTypes.size()))); + + assertEquals(CONCURRENT_CALL_COUNT, schemas.size()); + for (Map schema : schemas) { + assertNotNull(schema); + assertNotNull(schema.get("type")); + } + } + + /** + * Runs the given generator on a fixed thread pool, with all tasks released at the same + * time to maximize the chance of overlapping schema generation. Any exception thrown + * inside a task propagates through {@code Future#get} and fails the test. + */ + private static List> generateConcurrently( + IntFunction> generator) throws Exception { + ExecutorService executor = Executors.newFixedThreadPool(CONCURRENT_THREAD_COUNT); + CountDownLatch start = new CountDownLatch(1); + try { + List>> futures = new ArrayList<>(); + for (int i = 0; i < CONCURRENT_CALL_COUNT; i++) { + final int index = i; + futures.add( + executor.submit( + () -> { + start.await(); + return generator.apply(index); + })); + } + start.countDown(); + List> results = new ArrayList<>(); + for (Future> future : futures) { + results.add(future.get(30, TimeUnit.SECONDS)); + } + return results; + } finally { + executor.shutdownNow(); + } + } }