This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 865dcce34f72eb1149b7f9a6649a9808c7c80f97 Author: opencode <[email protected]> AuthorDate: Thu Oct 8 15:13:06 2026 +0200 Fix data races in parallel annotation scanning When the parallelAnnotationScanning attribute of StandardContext is enabled, ContextConfig.processAnnotations() dispatches one scan task per web fragment to the server utility executor. The tasks share the per-ServletContainerInitializer class sets of initializerClassMap and the shared java class cache, and none of the shared updates were synchronised. The per-SCI sets were plain HashSets mutated concurrently from the scan tasks, so matched classes could be silently lost or the underlying map corrupted. All SCIs are registered before scanning starts, so the sets are now created as concurrent sets and the map itself is only read during scanning; the concurrent computeIfAbsent() that could structurally mutate the LinkedHashMap is gone. Investigation showed the race was not confined to the value sets. populateJavaClassCache() inserts a class before its super class and interfaces, so a second thread that saw the partially populated cache could compute an incomplete set of interested initializers for a class and cache it permanently. populateSCIsForCacheEntry() now loads the super class or interface entry on demand instead of treating a cache miss as proof of absence, so the computed set is always complete. The lazily computed per-entry set is also now volatile for safe publication between the scanner threads. Verified with a deployment harness using eight fragment JARs and an SCI with @HandlesTypes(Object.class): deployments lost classes from onStartup() before the fix and produced the exact expected set in every run after it. --- .../org/apache/catalina/startup/ContextConfig.java | 32 ++++++++++++++++++---- webapps/docs/changelog.xml | 14 ++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/java/org/apache/catalina/startup/ContextConfig.java b/java/org/apache/catalina/startup/ContextConfig.java index a78896cd11..53c638bae0 100644 --- a/java/org/apache/catalina/startup/ContextConfig.java +++ b/java/org/apache/catalina/startup/ContextConfig.java @@ -1848,7 +1848,9 @@ public class ContextConfig implements LifecycleListener { } for (ServletContainerInitializer sci : detectedScis) { - initializerClassMap.put(sci, new HashSet<>()); + // The per-SCI sets are updated concurrently when parallel annotation + // scanning is enabled + initializerClassMap.put(sci, ConcurrentHashMap.newKeySet()); HandlesTypes ht; try { @@ -2444,9 +2446,12 @@ public class ContextConfig implements LifecycleListener { return; } + // All SCIs are registered in processServletContainerInitializers() + // before scanning starts, so the map is only read here. The value + // sets are updated concurrently when parallel annotation scanning + // is enabled. for (ServletContainerInitializer sci : entry.getSciSet()) { - Set<Class<?>> classes = initializerClassMap.computeIfAbsent(sci, k -> new HashSet<>()); - classes.add(clazz); + initializerClassMap.get(sci).add(clazz); } } } @@ -2549,7 +2554,15 @@ public class ContextConfig implements LifecycleListener { return; } - // May be null of the class is not present or could not be loaded. + // May be null if the class is not present or could not be loaded. + if (superClassCacheEntry == null) { + // With parallel annotation scanning the entry may also be absent because + // another thread has not finished adding the hierarchy to the cache. + // Ensure the entry is present, loading the class if necessary, so the + // set computed here cannot miss an SCI that matches a super class. + populateJavaClassCache(superClassName, javaClassCache); + superClassCacheEntry = javaClassCache.get(superClassName); + } if (superClassCacheEntry != null) { if (superClassCacheEntry.getSciSet() == null) { populateSCIsForCacheEntry(superClassCacheEntry, javaClassCache); @@ -2561,6 +2574,13 @@ public class ContextConfig implements LifecycleListener { // Interfaces for (String interfaceName : cacheEntry.getInterfaceNames()) { JavaClassCacheEntry interfaceEntry = javaClassCache.get(interfaceName); + // As for the super class, a null may mean the parallel scanner has + // not finished populating the cache, so attempt the load before + // deciding there is nothing of interest. + if (interfaceEntry == null) { + populateJavaClassCache(interfaceName, javaClassCache); + interfaceEntry = javaClassCache.get(interfaceName); + } // A null could mean that the class not present in application or // that there is nothing of interest. Either way, nothing to do here // so move along @@ -2969,8 +2989,10 @@ public class ContextConfig implements LifecycleListener { /** * The set of ServletContainerInitializers interested in this class, or {@link #EMPTY_SCI_SET} if none. + * Volatile for safe publication: the set is computed once and the reference may be handed between + * threads when annotation scanning runs in parallel. */ - private Set<ServletContainerInitializer> sciSet = null; + private volatile Set<ServletContainerInitializer> sciSet = null; /** * Constructs a new cache entry from a parsed Java class. diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 4ab02199d3..59d9dec912 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -1948,6 +1948,20 @@ they are per-document values that do not need to be stored in the external resolver. (remm) </fix> + <fix> + Fix multiple data races in annotation scanning when the + <code>parallelAnnotationScanning</code> attribute of the + <code>StandardContext</code> is enabled. Classes matched for a + <code>ServletContainerInitializer</code> were collected into + non-thread-safe sets, which could silently lose matches. A scanner + thread could also cache an incomplete set of initializers for a class + when another thread had not finished adding that class' super class or + interfaces to the shared class cache, and the lazily computed set was + not safely published between threads. Matched classes could therefore + be missing from the set passed to + <code>ServletContainerInitializer.onStartup()</code>, misconfiguring + the application at runtime. (remm) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
