From db251e7f85a22e7215454bef5c050fb164ff68f9 Mon Sep 17 00:00:00 2001 From: Steven Schlansker Date: Fri, 23 Oct 2026 12:01:55 -0700 Subject: [PATCH 1/9] fix(java): run descriptor warm-up on Fory's compiler pool and drop its shutdown hook Descriptor warm-up used ForkJoinPool.commonPool(), whose threads serve the whole JVM. When Fory is loaded by a classloader other than the system loader, such as a plugin or per-application loader, warm-up tasks do not run with that loader as their context classloader (TCCL). On JDK 9+, common-pool threads use the system loader. On JDK 8, a thread inherits the TCCL of whichever thread started it, which can be another application's loader, and the shared thread keeps that loader reachable. Warm-up now uses CodeGenerator.getCompilationService(), Fory's own pool. Before, only async compilation created that pool. Now the first descriptor build that submits warm-up work also creates it, including in the default configuration. ForyJitCompilerThreadFactory sets each pool thread's TCCL to the loader that defines Fory, instead of inheriting the TCCL of the submitting thread. Fory already falls back to that loader when the TCCL is null. The pool's JVM shutdown hook (#3138) is removed. The JVM keeps a registered hook until exit, so the hook kept Fory's classloader reachable, and its thread kept the TCCL of the thread that created the pool. With the pool now created in the default configuration, that hold would apply to every application. The pool threads are daemons and idle threads time out after 5 seconds, so the pool does not delay JVM exit. Without the hook, in-flight tasks no longer get up to 1 second at exit, and tasks submitted during shutdown are accepted instead of rejected with a warning. This removes only the compiler pool's hold on the loader: other Fory state, such as the MultiKeyWeakMap cleaner thread, still keeps the loader reachable after a Fory instance is created. seMaxCompilationThreadPoolSize now rejects values below 1 when it is called, instead of failing later at pool creation. A call after the pool exists logs a warning and has no effect. The default size is computed when the pool is created, because CodeGenerator is initialized at native-image build time and its static initializer sized the pool for the build machine. Native-image build time keeps the common pool for warm-up, because using the compiler service there would store a direct executor in the image. --- .agents/languages/java.md | 11 +++ .../apache/fory/codegen/CodeGenerator.java | 48 ++++++----- .../java/org/apache/fory/type/Descriptor.java | 42 ++++++---- .../ForyJitCompilerThreadFactory.java | 4 + .../fory/GuavaOptionalDependencyTest.java | 18 +---- .../test/java/org/apache/fory/TestUtils.java | 16 ++++ .../org/apache/fory/type/DescriptorTest.java | 81 ++++++++++++++++++- .../ForyJitCompilerThreadFactoryTest.java | 43 ++++++++++ 8 files changed, 211 insertions(+), 52 deletions(-) create mode 100644 java/fory-core/src/test/java/org/apache/fory/util/concurrency/ForyJitCompilerThreadFactoryTest.java diff --git a/.agents/languages/java.md b/.agents/languages/java.md index 98d0caff4b..8e19d7f254 100644 --- a/.agents/languages/java.md +++ b/.agents/languages/java.md @@ -49,6 +49,17 @@ Load this file when changing anything under `java/` or when Java drives a cross- values; use qualified names only when a real name conflict requires it. - If you run temporary tests with `java -cp`, run `mvn -T16 install -DskipTests` first so local Fory jars are current. - `WriteContext`, `ReadContext`, and `CopyContext` must stay explicit. Do not reintroduce `ThreadLocal` or ambient runtime-context patterns. +- Do not register JVM shutdown hooks from Fory runtime code. A registered hook stays reachable + until JVM exit and keeps Fory's classloader loaded. Compiler pool threads are daemons that exit + when idle, so the pool needs no hook. +- Submit Fory background work, such as async JIT compilation and descriptor warm-up, to + `CodeGenerator.getCompilationService()`, not to a JVM-global executor such as + `ForkJoinPool.commonPool()` or the default `CompletableFuture` async executor. A global pool's + threads serve the whole JVM, so their context classloader may not be the loader that loaded Fory. + `ForyJitCompilerThreadFactory` pins each compiler thread's context classloader to + Fory's defining loader. Native-image build time is the only exception: `CodeGenerator` is + initialized at build time, so using its compiler service there would store a direct executor in + the image, and descriptor warm-up keeps the common pool. - Java root deserialization graph memory budgeting belongs to `ReadContext` and is initialized by `Fory` root APIs. Public config is `maxGraphMemoryBytes` with fixed `128 MiB` default. Positive explicit values override the default; diff --git a/java/fory-core/src/main/java/org/apache/fory/codegen/CodeGenerator.java b/java/fory-core/src/main/java/org/apache/fory/codegen/CodeGenerator.java index 7777e23446..d3d393cfb1 100644 --- a/java/fory-core/src/main/java/org/apache/fory/codegen/CodeGenerator.java +++ b/java/fory-core/src/main/java/org/apache/fory/codegen/CodeGenerator.java @@ -81,7 +81,9 @@ public class CodeGenerator { // use this package when bean class name starts with java. private static final String FALLBACK_PACKAGE = Generated.class.getPackage().getName(); public static final boolean ENABLE_FORY_GENERATED_CLASS_UNIQUE_ID; - private static int maxPoolSize = Math.max(1, Runtime.getRuntime().availableProcessors() / 2); + // Zero selects a default from the CPU count when the pool is created. This class is initialized + // at native-image build time, so a static initializer would size the pool for the build machine. + private static int maxPoolSize; private static ExecutorService compilationExecutorService; static { @@ -256,7 +258,24 @@ private static void checkRuntimeCodegenSupported() { } } - public static void seMaxCompilationThreadPoolSize(int maxCompilationThreadPoolSize) { + /** + * Sets the thread count of the compiler pool that runs async compilation and descriptor warm-up. + * The default is half the available processors, at least 1. Call this method before the pool is + * created, which happens at the first async compilation or the first descriptor warm-up. A later + * call logs a warning and has no effect. + * + * @param maxCompilationThreadPoolSize thread count, at least 1 + */ + public static synchronized void seMaxCompilationThreadPoolSize(int maxCompilationThreadPoolSize) { + Preconditions.checkArgument( + maxCompilationThreadPoolSize >= 1, + "Compilation thread pool size must be at least 1: " + maxCompilationThreadPoolSize); + if (compilationExecutorService != null) { + LOG.warn( + "Ignoring compilation thread pool size {}: the pool already exists.", + maxCompilationThreadPoolSize); + return; + } maxPoolSize = maxCompilationThreadPoolSize; } @@ -266,10 +285,14 @@ public static synchronized ExecutorService getCompilationService() { // GraalVM build time can't reachable thread. return compilationExecutorService = new DirectExecutorService(); } + int poolSize = + maxPoolSize > 0 + ? maxPoolSize + : Math.max(1, Runtime.getRuntime().availableProcessors() / 2); ThreadPoolExecutor executor = new ThreadPoolExecutor( - maxPoolSize, - maxPoolSize, + poolSize, + poolSize, 5L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), @@ -278,23 +301,10 @@ public static synchronized ExecutorService getCompilationService() { // Normally task won't be rejected by executor, since we used an unbound queue. // But when we shut down executor for debug, it'll be rejected by executor, // in such cases we just ignore the reject exception by log it. + // Idle threads time out and all threads are daemons, so the pool needs no shutdown hook. A + // hook would keep Fory's classloader reachable until JVM exit. executor.allowCoreThreadTimeOut(true); compilationExecutorService = executor; - Runtime.getRuntime() - .addShutdownHook( - new Thread( - () -> { - executor.shutdown(); - try { - if (!executor.awaitTermination(1, TimeUnit.SECONDS)) { - executor.shutdownNow(); - } - } catch (InterruptedException e) { - executor.shutdownNow(); - Thread.currentThread().interrupt(); - } - }, - "fory-jit-compiler-shutdown")); } return compilationExecutorService; } diff --git a/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java b/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java index de2f95c4a2..aebd413def 100644 --- a/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java +++ b/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java @@ -56,12 +56,14 @@ import org.apache.fory.annotation.UInt32Type; import org.apache.fory.annotation.UInt64Type; import org.apache.fory.annotation.UInt8Type; +import org.apache.fory.codegen.CodeGenerator; import org.apache.fory.collection.Cache; import org.apache.fory.collection.CacheBuilder; import org.apache.fory.collection.ClassValueCache; import org.apache.fory.collection.Collections; import org.apache.fory.collection.Tuple2; import org.apache.fory.meta.TypeExtMeta; +import org.apache.fory.platform.GraalvmSupport; import org.apache.fory.reflect.TypeRef; import org.apache.fory.reflect.TypeUseMetadata; import org.apache.fory.serializer.converter.FieldConverter; @@ -843,8 +845,6 @@ public static SortedMap getAllDescriptorsMap( TreeMap descriptorMap = new TreeMap<>(memberComparator); TreeMap currentDescriptorMap = new TreeMap<>(memberComparator); Class clazz = clz; - // TODO(chaokunyang) use fory compiler thread pool - ExecutorService compilationService = ForkJoinPool.commonPool(); if (RecordUtils.isRecord(clz)) { RecordComponent[] components = RecordUtils.getRecordComponents(clazz); assert components != null; @@ -883,7 +883,7 @@ public static SortedMap getAllDescriptorsMap( Field[] fields = clazz.getDeclaredFields(); boolean haveExpose = false, haveIgnore = false; for (Field field : fields) { - warmField(clz, field, compilationService); + warmField(clz, field); if (field.isAnnotationPresent(Expose.class)) { haveExpose = true; } @@ -918,11 +918,23 @@ public static SortedMap getAllDescriptorsMap( return Tuple2.of(descriptorMap, currentDescriptorMap); } + /** + * Warm-up runs on Fory's compiler pool, not a JVM-global pool. A global pool's threads serve the + * whole JVM, so their context classloader may not be the loader that loaded Fory. Native-image + * build time keeps the common pool: {@link CodeGenerator} is initialized at build time, so its + * compiler service would store a direct executor in the image and make warm-up serial at runtime. + */ + private static ExecutorService warmUpExecutor() { + return GraalvmSupport.isGraalBuildTime() + ? ForkJoinPool.commonPool() + : CodeGenerator.getCompilationService(); + } + /** * Speedup generics resolve by multi-thread since {@link Field#getGenericType()} is slow and * nested Descriptor is slow in single thread. */ - static void warmField(Class context, Field field, ExecutorService compilationService) { + static void warmField(Class context, Field field) { Class fieldRawType = field.getType(); if (fieldRawType.isPrimitive() || fieldRawType == String.class @@ -937,22 +949,24 @@ static void warmField(Class context, Field field, ExecutorService compilation return; } if (!fieldRawType.getName().startsWith("java")) { - compilationService.submit( - () -> { - // use a flag to avoid blocking thread. - AtomicBoolean flag = flags.computeIfAbsent(fieldRawType, k -> new AtomicBoolean(false)); - if (flag.compareAndSet(false, true)) { - getAllDescriptorsMap(fieldRawType); - } - }); + warmUpExecutor() + .submit( + () -> { + // use a flag to avoid blocking thread. + AtomicBoolean flag = + flags.computeIfAbsent(fieldRawType, k -> new AtomicBoolean(false)); + if (flag.compareAndSet(false, true)) { + getAllDescriptorsMap(fieldRawType); + } + }); } else if (TypeUtils.isCollection(fieldRawType) || TypeUtils.isMap(fieldRawType)) { // warm up generic type, sun.reflect.generics.repository.FieldRepository // is expensive. - compilationService.submit(() -> warmGenericTask(TypeUtils.getFieldTypeRef(field))); + warmUpExecutor().submit(() -> warmGenericTask(TypeUtils.getFieldTypeRef(field))); } else if (fieldRawType.isArray()) { Class componentType = fieldRawType.getComponentType(); if (!componentType.isPrimitive()) { - compilationService.submit(() -> warmGenericTask(TypeUtils.getFieldTypeRef(field))); + warmUpExecutor().submit(() -> warmGenericTask(TypeUtils.getFieldTypeRef(field))); } } } diff --git a/java/fory-core/src/main/java/org/apache/fory/util/concurrency/ForyJitCompilerThreadFactory.java b/java/fory-core/src/main/java/org/apache/fory/util/concurrency/ForyJitCompilerThreadFactory.java index 72cf564efe..812160e275 100644 --- a/java/fory-core/src/main/java/org/apache/fory/util/concurrency/ForyJitCompilerThreadFactory.java +++ b/java/fory-core/src/main/java/org/apache/fory/util/concurrency/ForyJitCompilerThreadFactory.java @@ -32,6 +32,10 @@ public Thread newThread(Runnable task) { Thread thread = backingThreadFactory.newThread(task); thread.setName("fory-jit-compiler-" + threadNumber.incrementAndGet()); thread.setDaemon(true); + // Pool threads start lazily on whichever thread submits a task and would inherit its context + // classloader. Pin the loader that defines Fory so every task sees the same context + // classloader on every JDK and a pool thread never keeps a caller's context classloader. + thread.setContextClassLoader(ForyJitCompilerThreadFactory.class.getClassLoader()); return thread; } } diff --git a/java/fory-core/src/test/java/org/apache/fory/GuavaOptionalDependencyTest.java b/java/fory-core/src/test/java/org/apache/fory/GuavaOptionalDependencyTest.java index e43a545285..74afaad3e0 100644 --- a/java/fory-core/src/test/java/org/apache/fory/GuavaOptionalDependencyTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/GuavaOptionalDependencyTest.java @@ -26,7 +26,6 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; -import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; import java.nio.charset.StandardCharsets; @@ -102,7 +101,8 @@ private static RegistrationIds runWithoutGuava() throws Exception { } private static RegistrationIds runWithPartialGuava() throws Exception { - try (PartialGuavaClassLoader loader = new PartialGuavaClassLoader(classPathUrls())) { + try (PartialGuavaClassLoader loader = + new PartialGuavaClassLoader(TestUtils.forkClassPathUrls())) { Class main = Class.forName(PartialGuavaMain.class.getName(), true, loader); String output = (String) main.getMethod("run").invoke(null); return parseResult(output); @@ -125,20 +125,6 @@ private static String removeGuavaFromClasspath(String classPath) { .collect(Collectors.joining(File.pathSeparator)); } - private static URL[] classPathUrls() { - return Arrays.stream(System.getProperty("java.class.path").split(File.pathSeparator)) - .map(GuavaOptionalDependencyTest::toUrl) - .toArray(URL[]::new); - } - - private static URL toUrl(String path) { - try { - return new File(path).toURI().toURL(); - } catch (MalformedURLException e) { - throw new IllegalArgumentException(e); - } - } - private static String readFully(InputStream inputStream) throws IOException { ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); byte[] buffer = new byte[1024]; diff --git a/java/fory-core/src/test/java/org/apache/fory/TestUtils.java b/java/fory-core/src/test/java/org/apache/fory/TestUtils.java index 81cfec51a8..575f5f2943 100644 --- a/java/fory-core/src/test/java/org/apache/fory/TestUtils.java +++ b/java/fory-core/src/test/java/org/apache/fory/TestUtils.java @@ -28,6 +28,8 @@ import java.lang.management.ManagementFactory; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.net.MalformedURLException; +import java.net.URL; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -99,6 +101,20 @@ public static String forkClassPath() { return String.join(File.pathSeparator, elements); } + public static URL[] forkClassPathUrls() { + return Arrays.stream(forkClassPath().split(java.util.regex.Pattern.quote(File.pathSeparator))) + .map(TestUtils::toUrl) + .toArray(URL[]::new); + } + + private static URL toUrl(String path) { + try { + return new File(path).toURI().toURL(); + } catch (MalformedURLException e) { + throw new IllegalArgumentException(e); + } + } + private static void addPathElements(Set elements, String path) { if (path == null || path.isEmpty()) { return; diff --git a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java index 568e3a1902..97fbf74acb 100644 --- a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java @@ -21,15 +21,21 @@ import java.beans.IntrospectionException; import java.beans.Introspector; +import java.lang.ref.WeakReference; import java.lang.reflect.Field; +import java.net.URLClassLoader; import java.util.Arrays; import java.util.List; import java.util.SortedMap; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; -import org.apache.fory.codegen.CodeGenerator; +import org.apache.fory.TestUtils; +import org.apache.fory.platform.internal._JDKAccess; import org.apache.fory.reflect.TypeRef; import org.apache.fory.test.bean.BeanA; import org.testng.Assert; +import org.testng.SkipException; import org.testng.annotations.Test; public class DescriptorTest { @@ -88,13 +94,82 @@ public void getDescriptorsTest() throws IntrospectionException { public void testWarmField() throws Exception { Assert.assertEquals(int.class.getName(), "int"); Assert.assertEquals(Integer.class.getName(), "java.lang.Integer"); - Descriptor.warmField( - BeanA.class, BeanA.class.getDeclaredField("beanB"), CodeGenerator.getCompilationService()); + Descriptor.warmField(BeanA.class, BeanA.class.getDeclaredField("beanB")); Descriptor.getAllDescriptorsMap(BeanA.class); Descriptor.clearDescriptorCache(); Descriptor.getAllDescriptorsMap(BeanA.class); } + public static class WarmOuter { + WarmInner inner; + } + + public static class WarmInner { + WarmLeaf leaf; + } + + public static class WarmLeaf {} + + public static class WarmProbe implements Runnable { + @Override + public void run() { + Descriptor.getAllDescriptorsMap(WarmOuter.class); + } + } + + /** + * Builds descriptors with Fory loaded in its own classloader. Only the background warm-up of + * {@code WarmInner} loads {@code WarmLeaf}, so the recorded loader is the TCCL of a warm-up + * thread. + */ + @Test + public void testWarmContextClassLoader() throws Exception { + warmInIsolatedLoader(); + } + + /** Descriptor warm-up must not keep an isolated Fory classloader reachable after close. */ + @Test + public void testWarmReleasesLoader() throws Exception { + if (_JDKAccess.IS_OPEN_J9) { + throw new SkipException("OpenJ9 unsupported"); + } + WeakReference isolate = warmInIsolatedLoader(); + // Idle compiler pool threads keep the loader reachable until their 5 second keep-alive ends. + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(30); + while (isolate.get() != null && System.nanoTime() < deadline) { + System.gc(); + Thread.sleep(100); + } + Assert.assertNull(isolate.get(), "isolated Fory classloader is still reachable"); + } + + private static WeakReference warmInIsolatedLoader() throws Exception { + try (WarmLeafRecorder isolate = new WarmLeafRecorder()) { + Assert.assertNotSame(Thread.currentThread().getContextClassLoader(), isolate); + Class probe = isolate.loadClass(WarmProbe.class.getName()); + ((Runnable) probe.getDeclaredConstructor().newInstance()).run(); + Assert.assertSame(isolate.leafContextClassLoader.get(30, TimeUnit.SECONDS), isolate); + return new WeakReference<>(isolate); + } + } + + private static final class WarmLeafRecorder extends URLClassLoader { + private final CompletableFuture leafContextClassLoader = new CompletableFuture<>(); + + private WarmLeafRecorder() { + super(TestUtils.forkClassPathUrls(), ClassLoader.getSystemClassLoader().getParent()); + } + + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + Class cls = super.loadClass(name, resolve); + if (name.equals(WarmLeaf.class.getName())) { + leafContextClassLoader.complete(Thread.currentThread().getContextClassLoader()); + } + return cls; + } + } + @Test public void testDescriptorBuilder() { Descriptor descriptor = diff --git a/java/fory-core/src/test/java/org/apache/fory/util/concurrency/ForyJitCompilerThreadFactoryTest.java b/java/fory-core/src/test/java/org/apache/fory/util/concurrency/ForyJitCompilerThreadFactoryTest.java new file mode 100644 index 0000000000..72f141bbe7 --- /dev/null +++ b/java/fory-core/src/test/java/org/apache/fory/util/concurrency/ForyJitCompilerThreadFactoryTest.java @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.fory.util.concurrency; + +import static org.testng.Assert.assertSame; + +import java.net.URL; +import java.net.URLClassLoader; +import org.testng.annotations.Test; + +public class ForyJitCompilerThreadFactoryTest { + + @Test + public void testContextClassLoader() throws Exception { + Thread current = Thread.currentThread(); + ClassLoader previous = current.getContextClassLoader(); + try (URLClassLoader callerLoader = new URLClassLoader(new URL[0], null)) { + current.setContextClassLoader(callerLoader); + Thread thread = new ForyJitCompilerThreadFactory().newThread(() -> {}); + assertSame( + thread.getContextClassLoader(), ForyJitCompilerThreadFactory.class.getClassLoader()); + } finally { + current.setContextClassLoader(previous); + } + } +} From 908e3b700dbcfc092dbe7a7f8768d6459dd08336 Mon Sep 17 00:00:00 2001 From: Steven Schlansker Date: Fri, 23 Oct 2026 12:01:55 -0700 Subject: [PATCH 2/9] fix(java): resize an existing compiler pool and get the warm-up executor once per class createAllDescriptorsMap now gets the warm-up executor at most one time for each class, not one time for each field. Each lookup takes the CodeGenerator class monitor, which getSharedCodeGenerator also takes. The lookup happens only when a field needs warm-up, so a class without such fields still does not create the compiler pool. warmField becomes warmUpTask, which returns the task or null, and the caller submits it. The compiler pool now starts at the first descriptor warm-up, not at the first async compilation. Thus a seMaxCompilationThreadPoolSize call after Fory registration found the pool already created, and the size was ignored. The setter now resizes the existing pool in place. It changes the maximum and core sizes in the order that keeps core <= max at each step, which ThreadPoolExecutor requires. Extra threads exit when they finish their current task. --- .../apache/fory/codegen/CodeGenerator.java | 20 +++++---- .../java/org/apache/fory/type/Descriptor.java | 42 +++++++++++-------- .../fory/codegen/CodeGeneratorTest.java | 32 ++++++++++++++ .../org/apache/fory/type/DescriptorTest.java | 2 +- 4 files changed, 69 insertions(+), 27 deletions(-) diff --git a/java/fory-core/src/main/java/org/apache/fory/codegen/CodeGenerator.java b/java/fory-core/src/main/java/org/apache/fory/codegen/CodeGenerator.java index d3d393cfb1..59e66fdbc6 100644 --- a/java/fory-core/src/main/java/org/apache/fory/codegen/CodeGenerator.java +++ b/java/fory-core/src/main/java/org/apache/fory/codegen/CodeGenerator.java @@ -260,9 +260,8 @@ private static void checkRuntimeCodegenSupported() { /** * Sets the thread count of the compiler pool that runs async compilation and descriptor warm-up. - * The default is half the available processors, at least 1. Call this method before the pool is - * created, which happens at the first async compilation or the first descriptor warm-up. A later - * call logs a warning and has no effect. + * The default is half the available processors, at least 1. A call after the pool exists resizes + * it: extra threads exit when they finish their current task. * * @param maxCompilationThreadPoolSize thread count, at least 1 */ @@ -270,11 +269,16 @@ public static synchronized void seMaxCompilationThreadPoolSize(int maxCompilatio Preconditions.checkArgument( maxCompilationThreadPoolSize >= 1, "Compilation thread pool size must be at least 1: " + maxCompilationThreadPoolSize); - if (compilationExecutorService != null) { - LOG.warn( - "Ignoring compilation thread pool size {}: the pool already exists.", - maxCompilationThreadPoolSize); - return; + if (compilationExecutorService instanceof ThreadPoolExecutor) { + ThreadPoolExecutor executor = (ThreadPoolExecutor) compilationExecutorService; + // ThreadPoolExecutor requires core <= max after each setter call. + if (maxCompilationThreadPoolSize > executor.getMaximumPoolSize()) { + executor.setMaximumPoolSize(maxCompilationThreadPoolSize); + executor.setCorePoolSize(maxCompilationThreadPoolSize); + } else { + executor.setCorePoolSize(maxCompilationThreadPoolSize); + executor.setMaximumPoolSize(maxCompilationThreadPoolSize); + } } maxPoolSize = maxCompilationThreadPoolSize; } diff --git a/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java b/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java index aebd413def..7a0e57a25e 100644 --- a/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java +++ b/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java @@ -879,11 +879,18 @@ public static SortedMap getAllDescriptorsMap( currentDescriptorMap = new TreeMap<>(descriptorMap); return Tuple2.of(descriptorMap, currentDescriptorMap); } + ExecutorService executor = null; do { Field[] fields = clazz.getDeclaredFields(); boolean haveExpose = false, haveIgnore = false; for (Field field : fields) { - warmField(clz, field); + Runnable warmUp = warmUpTask(clz, field); + if (warmUp != null) { + if (executor == null) { + executor = warmUpExecutor(); + } + executor.submit(warmUp); + } if (field.isAnnotationPresent(Expose.class)) { haveExpose = true; } @@ -932,43 +939,42 @@ private static ExecutorService warmUpExecutor() { /** * Speedup generics resolve by multi-thread since {@link Field#getGenericType()} is slow and - * nested Descriptor is slow in single thread. + * nested Descriptor is slow in single thread. Returns the warm-up task for {@code field}, or null + * when the field needs no warm-up. */ - static void warmField(Class context, Field field) { + static Runnable warmUpTask(Class context, Field field) { Class fieldRawType = field.getType(); if (fieldRawType.isPrimitive() || fieldRawType == String.class || fieldRawType == Object.class) { - return; + return null; } if (TypeUtils.isBoxed(fieldRawType)) { - return; + return null; } if (fieldRawType == context) { // avoid duplicate build. - return; + return null; } if (!fieldRawType.getName().startsWith("java")) { - warmUpExecutor() - .submit( - () -> { - // use a flag to avoid blocking thread. - AtomicBoolean flag = - flags.computeIfAbsent(fieldRawType, k -> new AtomicBoolean(false)); - if (flag.compareAndSet(false, true)) { - getAllDescriptorsMap(fieldRawType); - } - }); + return () -> { + // use a flag to avoid blocking thread. + AtomicBoolean flag = flags.computeIfAbsent(fieldRawType, k -> new AtomicBoolean(false)); + if (flag.compareAndSet(false, true)) { + getAllDescriptorsMap(fieldRawType); + } + }; } else if (TypeUtils.isCollection(fieldRawType) || TypeUtils.isMap(fieldRawType)) { // warm up generic type, sun.reflect.generics.repository.FieldRepository // is expensive. - warmUpExecutor().submit(() -> warmGenericTask(TypeUtils.getFieldTypeRef(field))); + return () -> warmGenericTask(TypeUtils.getFieldTypeRef(field)); } else if (fieldRawType.isArray()) { Class componentType = fieldRawType.getComponentType(); if (!componentType.isPrimitive()) { - warmUpExecutor().submit(() -> warmGenericTask(TypeUtils.getFieldTypeRef(field))); + return () -> warmGenericTask(TypeUtils.getFieldTypeRef(field)); } } + return null; } // this method should b executed in background thread pool. diff --git a/java/fory-core/src/test/java/org/apache/fory/codegen/CodeGeneratorTest.java b/java/fory-core/src/test/java/org/apache/fory/codegen/CodeGeneratorTest.java index d1ad8014a0..704b53c008 100644 --- a/java/fory-core/src/test/java/org/apache/fory/codegen/CodeGeneratorTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/codegen/CodeGeneratorTest.java @@ -19,20 +19,27 @@ package org.apache.fory.codegen; +import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; +import static org.testng.Assert.expectThrows; import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URLClassLoader; import java.util.HashMap; import java.util.Map; import java.util.WeakHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import org.apache.fory.Fory; +import org.apache.fory.TestUtils; import org.apache.fory.builder.ObjectCodecBuilder; import org.apache.fory.collection.MultiKeyWeakMap; import org.apache.fory.test.bean.Foo; @@ -78,6 +85,31 @@ public void indentation() { } } + /** Loads Fory in its own classloader, so the test does not change the shared compiler pool. */ + @Test + public void testCompilationThreadPoolSize() throws Exception { + try (URLClassLoader isolate = + new URLClassLoader( + TestUtils.forkClassPathUrls(), ClassLoader.getSystemClassLoader().getParent())) { + Class codeGenerator = isolate.loadClass(CodeGenerator.class.getName()); + Method setSize = codeGenerator.getMethod("seMaxCompilationThreadPoolSize", int.class); + InvocationTargetException e = + expectThrows(InvocationTargetException.class, () -> setSize.invoke(null, 0)); + assertTrue(e.getCause() instanceof IllegalArgumentException); + ThreadPoolExecutor pool = + (ThreadPoolExecutor) codeGenerator.getMethod("getCompilationService").invoke(null); + try { + for (int size : new int[] {pool.getMaximumPoolSize() + 2, 1}) { + setSize.invoke(null, size); + assertEquals(pool.getCorePoolSize(), size); + assertEquals(pool.getMaximumPoolSize(), size); + } + } finally { + pool.shutdownNow(); + } + } + } + @Test public void testGetSharedCodeGenerator() { CodeGenerator.getSharedCodeGenerator(getClass().getClassLoader()); diff --git a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java index 97fbf74acb..48492a2382 100644 --- a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java @@ -94,7 +94,7 @@ public void getDescriptorsTest() throws IntrospectionException { public void testWarmField() throws Exception { Assert.assertEquals(int.class.getName(), "int"); Assert.assertEquals(Integer.class.getName(), "java.lang.Integer"); - Descriptor.warmField(BeanA.class, BeanA.class.getDeclaredField("beanB")); + Descriptor.warmUpTask(BeanA.class, BeanA.class.getDeclaredField("beanB")).run(); Descriptor.getAllDescriptorsMap(BeanA.class); Descriptor.clearDescriptorCache(); Descriptor.getAllDescriptorsMap(BeanA.class); From a4abe95d5b309eb56719eef1b30d627c98c98972 Mon Sep 17 00:00:00 2001 From: Steven Schlansker Date: Fri, 23 Oct 2026 12:01:55 -0700 Subject: [PATCH 3/9] fix(java): skip descriptor warm-up on a shut-down compiler pool getCompilationService() is public, so a caller can shut down the compiler pool. The pool's rejection handler logs a WARN and does not throw, so createAllDescriptorsMap logged one WARN for each field that needs warm-up. It now checks isShutdown() and skips warm-up, which is only an optimization. The ForyJitCompilerThreadFactory comment now says only that the thread never keeps a caller's context classloader. A pool thread still inherits other state from its creator, such as InheritableThreadLocal values. --- .../java/org/apache/fory/type/Descriptor.java | 5 +++- .../ForyJitCompilerThreadFactory.java | 7 +++--- .../org/apache/fory/type/DescriptorTest.java | 24 +++++++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java b/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java index 7a0e57a25e..7f86206c10 100644 --- a/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java +++ b/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java @@ -889,7 +889,10 @@ public static SortedMap getAllDescriptorsMap( if (executor == null) { executor = warmUpExecutor(); } - executor.submit(warmUp); + // A caller can shut down the public compiler pool. Its rejection handler logs each task. + if (!executor.isShutdown()) { + executor.submit(warmUp); + } } if (field.isAnnotationPresent(Expose.class)) { haveExpose = true; diff --git a/java/fory-core/src/main/java/org/apache/fory/util/concurrency/ForyJitCompilerThreadFactory.java b/java/fory-core/src/main/java/org/apache/fory/util/concurrency/ForyJitCompilerThreadFactory.java index 812160e275..734c330b8a 100644 --- a/java/fory-core/src/main/java/org/apache/fory/util/concurrency/ForyJitCompilerThreadFactory.java +++ b/java/fory-core/src/main/java/org/apache/fory/util/concurrency/ForyJitCompilerThreadFactory.java @@ -32,9 +32,10 @@ public Thread newThread(Runnable task) { Thread thread = backingThreadFactory.newThread(task); thread.setName("fory-jit-compiler-" + threadNumber.incrementAndGet()); thread.setDaemon(true); - // Pool threads start lazily on whichever thread submits a task and would inherit its context - // classloader. Pin the loader that defines Fory so every task sees the same context - // classloader on every JDK and a pool thread never keeps a caller's context classloader. + // Pool threads start lazily on whichever thread submits a task. Pin the context classloader to + // the loader that defines Fory, so every task sees the same loader and a pool thread never + // keeps a caller's context classloader. The thread still inherits other state from its + // creator, such as InheritableThreadLocal values. thread.setContextClassLoader(ForyJitCompilerThreadFactory.class.getClassLoader()); return thread; } diff --git a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java index 48492a2382..95d148241e 100644 --- a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java @@ -28,9 +28,12 @@ import java.util.List; import java.util.SortedMap; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import org.apache.fory.TestUtils; +import org.apache.fory.codegen.CodeGenerator; import org.apache.fory.platform.internal._JDKAccess; import org.apache.fory.reflect.TypeRef; import org.apache.fory.test.bean.BeanA; @@ -143,6 +146,27 @@ public void testWarmReleasesLoader() throws Exception { Assert.assertNull(isolate.get(), "isolated Fory classloader is still reachable"); } + /** A shut-down compiler pool must not reject one warm-up task for each field. */ + @Test + public void testWarmSkipsShutDownPool() throws Exception { + try (URLClassLoader isolate = + new URLClassLoader( + TestUtils.forkClassPathUrls(), ClassLoader.getSystemClassLoader().getParent())) { + ThreadPoolExecutor pool = + (ThreadPoolExecutor) + isolate + .loadClass(CodeGenerator.class.getName()) + .getMethod("getCompilationService") + .invoke(null); + AtomicInteger rejected = new AtomicInteger(); + pool.setRejectedExecutionHandler((task, executor) -> rejected.incrementAndGet()); + pool.shutdown(); + Class probe = isolate.loadClass(WarmProbe.class.getName()); + ((Runnable) probe.getDeclaredConstructor().newInstance()).run(); + Assert.assertEquals(rejected.get(), 0); + } + } + private static WeakReference warmInIsolatedLoader() throws Exception { try (WarmLeafRecorder isolate = new WarmLeafRecorder()) { Assert.assertNotSame(Thread.currentThread().getContextClassLoader(), isolate); From 2c809c099c1468564e735fbf87aa3a560bfa9367 Mon Sep 17 00:00:00 2001 From: Steven Schlansker Date: Fri, 23 Oct 2026 12:01:55 -0700 Subject: [PATCH 4/9] test(java): cover a compiler pool size set before the pool starts testCompilationThreadPoolSize now sets the size before the first getCompilationService call and checks that the new pool has that core and maximum size. Before, the test set the size only after the pool existed, so a getCompilationService that ignored the preset size passed. The preset size is the default size plus 1, so it differs from the default on every CPU count. --- .../test/java/org/apache/fory/codegen/CodeGeneratorTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/java/fory-core/src/test/java/org/apache/fory/codegen/CodeGeneratorTest.java b/java/fory-core/src/test/java/org/apache/fory/codegen/CodeGeneratorTest.java index 704b53c008..4cf0c95a06 100644 --- a/java/fory-core/src/test/java/org/apache/fory/codegen/CodeGeneratorTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/codegen/CodeGeneratorTest.java @@ -96,9 +96,13 @@ public void testCompilationThreadPoolSize() throws Exception { InvocationTargetException e = expectThrows(InvocationTargetException.class, () -> setSize.invoke(null, 0)); assertTrue(e.getCause() instanceof IllegalArgumentException); + int presetSize = Math.max(1, Runtime.getRuntime().availableProcessors() / 2) + 1; + setSize.invoke(null, presetSize); ThreadPoolExecutor pool = (ThreadPoolExecutor) codeGenerator.getMethod("getCompilationService").invoke(null); try { + assertEquals(pool.getCorePoolSize(), presetSize); + assertEquals(pool.getMaximumPoolSize(), presetSize); for (int size : new int[] {pool.getMaximumPoolSize() + 2, 1}) { setSize.invoke(null, size); assertEquals(pool.getCorePoolSize(), size); From 3472c9de01730782697f5346f20cf8e90591c8db Mon Sep 17 00:00:00 2001 From: Steven Schlansker Date: Fri, 23 Oct 2026 12:01:55 -0700 Subject: [PATCH 5/9] test(java): load the descriptor warm-up leaf only from warm-up threads On JDK 8, Field.getAnnotatedType() on the WarmInner field resolves the enclosing class of WarmInner and loads other member classes of DescriptorTest, so the nested WarmLeaf loaded on the test thread and testWarmReleasesLoader recorded the wrong context classloader. The leaf is now the top-level DescriptorWarmLeaf, which only the warm-up of WarmInner loads. --- .../org/apache/fory/type/DescriptorTest.java | 10 +++---- .../apache/fory/type/DescriptorWarmLeaf.java | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 java/fory-core/src/test/java/org/apache/fory/type/DescriptorWarmLeaf.java diff --git a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java index 95d148241e..3a05cea9da 100644 --- a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java @@ -108,11 +108,9 @@ public static class WarmOuter { } public static class WarmInner { - WarmLeaf leaf; + DescriptorWarmLeaf leaf; } - public static class WarmLeaf {} - public static class WarmProbe implements Runnable { @Override public void run() { @@ -122,8 +120,8 @@ public void run() { /** * Builds descriptors with Fory loaded in its own classloader. Only the background warm-up of - * {@code WarmInner} loads {@code WarmLeaf}, so the recorded loader is the TCCL of a warm-up - * thread. + * {@code WarmInner} loads {@code DescriptorWarmLeaf}, so the recorded loader is the TCCL of a + * warm-up thread. */ @Test public void testWarmContextClassLoader() throws Exception { @@ -187,7 +185,7 @@ private WarmLeafRecorder() { @Override protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { Class cls = super.loadClass(name, resolve); - if (name.equals(WarmLeaf.class.getName())) { + if (name.equals(DescriptorWarmLeaf.class.getName())) { leafContextClassLoader.complete(Thread.currentThread().getContextClassLoader()); } return cls; diff --git a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorWarmLeaf.java b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorWarmLeaf.java new file mode 100644 index 0000000000..c13de97dc3 --- /dev/null +++ b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorWarmLeaf.java @@ -0,0 +1,28 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.fory.type; + +/** + * Field type that only the descriptor warm-up of {@code DescriptorTest.WarmInner} loads. It is a + * top-level class because on JDK 8, {@link java.lang.reflect.Field#getAnnotatedType()} on a field + * of type {@code WarmInner} looks up its enclosing class and loads other member classes of {@code + * DescriptorTest}. A member class leaf would then load on the test thread, not on a warm-up thread. + */ +public class DescriptorWarmLeaf {} From 1bc205089278f2b7e66ea89f1e30de7a777dfbd7 Mon Sep 17 00:00:00 2001 From: "Claude (on behalf of Steven Schlansker)" Date: Fri, 6 Nov 2026 08:51:59 -0800 Subject: [PATCH 6/9] test(java): require a warm-up task in testWarmSkipsShutDownPool The test asserted only that the shut-down pool rejected no task, so it also passed when WarmOuter needed no warm-up. It now asserts first that the inner field produces a warm-up task. --- .../src/test/java/org/apache/fory/type/DescriptorTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java index 3a05cea9da..6700ee7e96 100644 --- a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java @@ -147,6 +147,8 @@ public void testWarmReleasesLoader() throws Exception { /** A shut-down compiler pool must not reject one warm-up task for each field. */ @Test public void testWarmSkipsShutDownPool() throws Exception { + Assert.assertNotNull( + Descriptor.warmUpTask(WarmOuter.class, WarmOuter.class.getDeclaredField("inner"))); try (URLClassLoader isolate = new URLClassLoader( TestUtils.forkClassPathUrls(), ClassLoader.getSystemClassLoader().getParent())) { From 8118a544f0a372c1d04a10cdb383f51162271189 Mon Sep 17 00:00:00 2001 From: "Claude (on behalf of Steven Schlansker)" Date: Fri, 6 Nov 2026 08:59:06 -0800 Subject: [PATCH 7/9] test(java): wait for the isolated compiler pool to exit in warm-up tests warmInIsolatedLoader closed the isolated loader as soon as the warm-up leaf loaded, while later warm-up tasks could still load classes from it. It now sets the pool keep-alive to 100 ms and waits until no pool thread is left before it closes the loader. The last pool thread exits only when the queue is empty, so no warm-up task runs after the close. The wait needs allowCoreThreadTimeOut, so the test still covers it, and testWarmReleasesLoader no longer waits for the 5 second keep-alive. --- .../org/apache/fory/type/DescriptorTest.java | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java index 6700ee7e96..75e52c1ba8 100644 --- a/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/type/DescriptorTest.java @@ -135,7 +135,6 @@ public void testWarmReleasesLoader() throws Exception { throw new SkipException("OpenJ9 unsupported"); } WeakReference isolate = warmInIsolatedLoader(); - // Idle compiler pool threads keep the loader reachable until their 5 second keep-alive ends. long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(30); while (isolate.get() != null && System.nanoTime() < deadline) { System.gc(); @@ -152,12 +151,7 @@ public void testWarmSkipsShutDownPool() throws Exception { try (URLClassLoader isolate = new URLClassLoader( TestUtils.forkClassPathUrls(), ClassLoader.getSystemClassLoader().getParent())) { - ThreadPoolExecutor pool = - (ThreadPoolExecutor) - isolate - .loadClass(CodeGenerator.class.getName()) - .getMethod("getCompilationService") - .invoke(null); + ThreadPoolExecutor pool = compilationService(isolate); AtomicInteger rejected = new AtomicInteger(); pool.setRejectedExecutionHandler((task, executor) -> rejected.incrementAndGet()); pool.shutdown(); @@ -170,13 +164,31 @@ public void testWarmSkipsShutDownPool() throws Exception { private static WeakReference warmInIsolatedLoader() throws Exception { try (WarmLeafRecorder isolate = new WarmLeafRecorder()) { Assert.assertNotSame(Thread.currentThread().getContextClassLoader(), isolate); + ThreadPoolExecutor pool = compilationService(isolate); + // Keep core-thread timeout under test, but do not wait for the 5 second default keep-alive. + pool.setKeepAliveTime(100, TimeUnit.MILLISECONDS); Class probe = isolate.loadClass(WarmProbe.class.getName()); ((Runnable) probe.getDeclaredConstructor().newInstance()).run(); Assert.assertSame(isolate.leafContextClassLoader.get(30, TimeUnit.SECONDS), isolate); + // The last pool thread exits only when the queue is empty, so no warm-up task can still load + // classes when the loader closes. + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(30); + while (pool.getPoolSize() > 0 && System.nanoTime() < deadline) { + Thread.sleep(10); + } + Assert.assertEquals(pool.getPoolSize(), 0, "compiler pool threads did not time out"); return new WeakReference<>(isolate); } } + private static ThreadPoolExecutor compilationService(ClassLoader isolate) throws Exception { + return (ThreadPoolExecutor) + isolate + .loadClass(CodeGenerator.class.getName()) + .getMethod("getCompilationService") + .invoke(null); + } + private static final class WarmLeafRecorder extends URLClassLoader { private final CompletableFuture leafContextClassLoader = new CompletableFuture<>(); From 128a7e6b7f74d9c2b5d2039ec41052748acdbb50 Mon Sep 17 00:00:00 2001 From: chaokunyang Date: Sat, 7 Nov 2026 16:19:00 +0800 Subject: [PATCH 8/9] fix(java): qualify core type names in generated serializers --- .agents/languages/java.md | 2 + .../apache/fory/codegen/CodegenContext.java | 40 +++---------- .../apache/fory/builder/JITContextTest.java | 59 +++++++++++++++++++ .../fory/codegen/CodegenContextTest.java | 10 ++-- 4 files changed, 73 insertions(+), 38 deletions(-) diff --git a/.agents/languages/java.md b/.agents/languages/java.md index 8e19d7f254..54cd137171 100644 --- a/.agents/languages/java.md +++ b/.agents/languages/java.md @@ -60,6 +60,8 @@ Load this file when changing anything under `java/` or when Java drives a cross- Fory's defining loader. Native-image build time is the only exception: `CodeGenerator` is initialized at build time, so using its compiler service there would store a direct executor in the image, and descriptor warm-up keeps the common pool. +- Generated source must qualify `java.lang` type names. A compiler thread's context classloader + may not see same-package application classes, so do not use it to detect type-name conflicts. - Java root deserialization graph memory budgeting belongs to `ReadContext` and is initialized by `Fory` root APIs. Public config is `maxGraphMemoryBytes` with fixed `128 MiB` default. Positive explicit values override the default; diff --git a/java/fory-core/src/main/java/org/apache/fory/codegen/CodegenContext.java b/java/fory-core/src/main/java/org/apache/fory/codegen/CodegenContext.java index 4b0b6094de..a50d3e4a5a 100644 --- a/java/fory-core/src/main/java/org/apache/fory/codegen/CodegenContext.java +++ b/java/fory-core/src/main/java/org/apache/fory/codegen/CodegenContext.java @@ -35,9 +35,7 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; -import org.apache.fory.Fory; import org.apache.fory.codegen.Expression.BaseInvoke; import org.apache.fory.codegen.Expression.Reference; import org.apache.fory.collection.Collections; @@ -121,8 +119,6 @@ public class CodegenContext { JAVA_RESERVED_WORDS = unmodifiableSet(JAVA_RESERVED_WORDS); } - private static Map> nameConflicts = new ConcurrentHashMap<>(); - Map newValNameIds = new HashMap<>(); Set valNames = new HashSet<>(JAVA_RESERVED_WORDS); @@ -283,8 +279,8 @@ public String namePrefix(Class clz) { * Get type string. * * @param clz type - * @return simple name for class if type's canonical name starts with java.lang or is imported, - * return canonical name otherwise. + * @return the qualified source name for {@code java.lang} types and unimported classes, or the + * simple name for other imported classes */ public String type(Class clz) { if (!sourcePkgLevelAccessible(clz)) { @@ -294,33 +290,11 @@ public String type(Class clz) { return getArrayType(clz); } String type = ReflectionUtils.getLiteralName(clz); - if (type.startsWith("java.lang")) { - if (!type.substring("java.lang.".length()).contains(".")) { - String simpleName = clz.getSimpleName(); - boolean hasPackage = StringUtils.isNotBlank(pkg); - Map packageMap = - nameConflicts.computeIfAbsent(hasPackage ? pkg : "", p -> new ConcurrentHashMap<>()); - Class c = clz; - Boolean conflictRes = - packageMap.computeIfAbsent( - simpleName, - sn -> { - try { - ClassLoader beanClassClassLoader = - c.getClassLoader() == null - ? Thread.currentThread().getContextClassLoader() - : c.getClassLoader(); - if (beanClassClassLoader == null) { - beanClassClassLoader = Fory.class.getClassLoader(); - } - beanClassClassLoader.loadClass(hasPackage ? pkg + "." + sn : sn); - return Boolean.TRUE; - } catch (ClassNotFoundException e) { - return Boolean.FALSE; - } - }); - return conflictRes ? clz.getName() : simpleName; - } + if (type.startsWith("java.lang.")) { + // A compiler thread's context loader may not see same-package classes in the bean's loader. + // Qualify these names so source resolution does not depend on a context-loader conflict + // probe. + return type; } if (imports.contains(type)) { return clz.getSimpleName(); diff --git a/java/fory-core/src/test/java/org/apache/fory/builder/JITContextTest.java b/java/fory-core/src/test/java/org/apache/fory/builder/JITContextTest.java index efe5781a74..3ffb075b38 100644 --- a/java/fory-core/src/test/java/org/apache/fory/builder/JITContextTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/builder/JITContextTest.java @@ -32,6 +32,8 @@ import org.apache.fory.Fory; import org.apache.fory.ForyTestBase; import org.apache.fory.ThreadSafeFory; +import org.apache.fory.codegen.CompileUnit; +import org.apache.fory.codegen.JaninoUtils; import org.apache.fory.context.MetaReadContext; import org.apache.fory.context.MetaWriteContext; import org.apache.fory.logging.Logger; @@ -40,6 +42,7 @@ import org.apache.fory.serializer.Serializer; import org.apache.fory.test.bean.BeanA; import org.apache.fory.test.bean.BeanB; +import org.codehaus.commons.compiler.util.reflect.ByteArrayClassLoader; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -98,6 +101,62 @@ public void testAsyncCompilation(boolean referenceTracking, boolean compatible) assertEquals(fory.deserialize(bytes2), beanA); } + @Test(dataProvider = "config1", timeOut = 60_000) + public void testAsyncTypeNames(boolean referenceTracking, boolean compatible) throws Exception { + String pkg = "org.apache.fory.codegen.loaderconflict"; + ClassLoader parent = Fory.class.getClassLoader(); + ByteArrayClassLoader loader = + JaninoUtils.compile( + parent, + new CompileUnit(pkg, "Object", "package " + pkg + "; public class Object {}"), + new CompileUnit(pkg, "String", "package " + pkg + "; public class String {}"), + new CompileUnit( + pkg, + "Bean", + "package " + + pkg + + "; public class Bean {" + + " public java.lang.Object value;" + + " public java.lang.String text;" + + "}")); + Assert.assertSame(loader.loadClass(Fory.class.getName()), Fory.class); + Assert.expectThrows(ClassNotFoundException.class, () -> parent.loadClass(pkg + ".Object")); + Assert.expectThrows(ClassNotFoundException.class, () -> parent.loadClass(pkg + ".String")); + Class beanClass = loader.loadClass(pkg + ".Bean"); + Object bean = beanClass.getDeclaredConstructor().newInstance(); + beanClass.getField("value").set(bean, 42); + beanClass.getField("text").set(bean, "child loader"); + + Thread current = Thread.currentThread(); + ClassLoader previous = current.getContextClassLoader(); + try { + current.setContextClassLoader(loader); + Fory fory = + Fory.builder() + .withXlang(false) + .withRefTracking(referenceTracking) + .withCompatible(compatible) + .withClassLoader(loader) + .withAsyncCompilation(true) + .build(); + fory.register(beanClass); + byte[] beforeJit = fory.serialize(bean); + while (fory.getJITContext().hasJITResult(beanClass)) { + Thread.sleep(10); + } + // A round trip alone can pass with the interpreter after an async compilation failure. + Assert.assertTrue(getSerializer(fory, beanClass) instanceof Generated); + for (byte[] bytes : new byte[][] {beforeJit, fory.serialize(bean)}) { + Object copy = fory.deserialize(bytes); + Assert.assertSame(copy.getClass(), beanClass); + Assert.assertEquals(beanClass.getField("value").get(copy), 42); + Assert.assertEquals(beanClass.getField("text").get(copy), "child loader"); + } + } finally { + current.setContextClassLoader(previous); + } + } + private Serializer getSerializer(Fory fory, Class cls) { try { fory.getJITContext().lock(); diff --git a/java/fory-core/src/test/java/org/apache/fory/codegen/CodegenContextTest.java b/java/fory-core/src/test/java/org/apache/fory/codegen/CodegenContextTest.java index 3396d4b01b..eeea653daa 100644 --- a/java/fory-core/src/test/java/org/apache/fory/codegen/CodegenContextTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/codegen/CodegenContextTest.java @@ -110,14 +110,14 @@ public void testJavaLangNameConflict() { CodegenContext ctx = new CodegenContext(); ctx.setPackage(CodegenContextTest.class.getPackage().getName()); - // we expect the simple name here. - Assert.assertEquals(ctx.type(Object.class), Object.class.getSimpleName()); + Assert.assertEquals(ctx.type(Object.class), Object.class.getName()); + Assert.assertEquals(ctx.type(String.class), String.class.getName()); + ctx.addImport(Integer.class); + Assert.assertEquals(ctx.type(Integer.class), Integer.class.getName()); ctx.setPackage( org.apache.fory.codegen.javalangnameconflict.Object.class.getPackage().getName()); - // we expect the full name here. Assert.assertEquals(ctx.type(Object.class), Object.class.getName()); - // we expect the simple name here. - Assert.assertEquals(ctx.type(Integer.class), Integer.class.getSimpleName()); + Assert.assertEquals(ctx.type(Integer.class), Integer.class.getName()); } } From 3419e80930fdbcdc6f23e4afb7e7803ca48778bb Mon Sep 17 00:00:00 2001 From: chaokunyang Date: Sat, 7 Nov 2026 16:28:15 +0800 Subject: [PATCH 9/9] test(java): match qualified collection reader type --- .../org/apache/fory/serializer/UnbackedContainerItemsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/fory-core/src/test/java/org/apache/fory/serializer/UnbackedContainerItemsTest.java b/java/fory-core/src/test/java/org/apache/fory/serializer/UnbackedContainerItemsTest.java index 361c6691a5..0f64ac231d 100644 --- a/java/fory-core/src/test/java/org/apache/fory/serializer/UnbackedContainerItemsTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/serializer/UnbackedContainerItemsTest.java @@ -168,7 +168,7 @@ public void testStructProgressProof() { String code = new ObjectCodecBuilder(PositiveStructHolder.class, fory).genCode(); int remoteRead = code.indexOf("private void remoteSameTypeElemsRead("); int dynamicProgressCheck = code.indexOf(".readDataAlwaysAdvances()", remoteRead); - int readCollection = code.indexOf("private Object readCollection("); + int readCollection = code.indexOf("private java.lang.Object readCollection("); int declaredRead = code.indexOf("& 4) == 4", readCollection); int remoteCall = code.indexOf("this.remoteSameTypeElemsRead(", declaredRead); assertTrue(remoteRead >= 0 && dynamicProgressCheck > remoteRead, code);