This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 944ac328b6e3a0d72bdb122e65a21a7d71d71147 Author: opencode <[email protected]> AuthorDate: Mon Oct 12 19:41:46 2026 +0200 Avoid rebuilding the archive bloom filter on every index read The bloom filter construction block in JarWarResourceSet.getArchiveEntries() sat outside the index construction guard, so every call to getArchiveEntries(), which happens on every resource lookup against a JAR nested in a packed WAR (the CLASSES_JAR sets created by StandardRoot.processWebInfLib), re-hashed every entry of the JAR. The filter is now built only when jarContents is null, matching the base class lifecycle of building it on the first open of the archive and clearing it in gc() when it is not retained. AbstractArchiveResourceSet.jarContents also became volatile, as it is written under the archive lock but read without locking in getResource(), where the update could previously be observed without sufficient synchronisation. getResource() now reads the field once into a local, so a concurrent gc() that nulls it cannot cause a null pointer exception between the null check and the use. Add a test to TestJarWarResourceSet asserting that the archive index and the bloom filter are reused across getArchiveEntries() calls. --- .../webresources/AbstractArchiveResourceSet.java | 9 +++++--- .../catalina/webresources/JarWarResourceSet.java | 2 +- .../webresources/TestJarWarResourceSet.java | 26 ++++++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java b/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java index 0da5a125d6..aeaa58dc88 100644 --- a/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java +++ b/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java @@ -67,9 +67,11 @@ public abstract class AbstractArchiveResourceSet extends AbstractResourceSet { */ protected long archiveUseCount = 0; /** - * The JAR contents. + * The JAR contents. Written under {@code archiveLock} but read without + * locking in {@code getResource()}, so the field is volatile to ensure a + * fully constructed instance is published to readers. */ - protected JarContents jarContents; + protected volatile JarContents jarContents; /** * Whether to retain the bloom filter for archives. */ @@ -293,7 +295,8 @@ public abstract class AbstractArchiveResourceSet extends AbstractResourceSet { * pre-check must use the JAR relative path. Hashing the web application path (with the mount point * embedded in it) would result in false negatives. */ - if (jarContents != null && !jarContents.mightContainResource(pathInJar, "")) { + JarContents contents = jarContents; + if (contents != null && !contents.mightContainResource(pathInJar, "")) { return new EmptyResource(root, path); } diff --git a/java/org/apache/catalina/webresources/JarWarResourceSet.java b/java/org/apache/catalina/webresources/JarWarResourceSet.java index e4bea41a30..69506987f7 100644 --- a/java/org/apache/catalina/webresources/JarWarResourceSet.java +++ b/java/org/apache/catalina/webresources/JarWarResourceSet.java @@ -155,7 +155,7 @@ public class JarWarResourceSet extends AbstractArchiveResourceSet { } } WebResourceRoot root = getRoot(); - if (root.getArchiveIndexStrategyEnum().getUsesBloom()) { + if (jarContents == null && root.getArchiveIndexStrategyEnum().getUsesBloom()) { jarContents = new JarContents(archiveEntries.values()); retainBloomFilterForArchives = root.getArchiveIndexStrategyEnum().getRetain(); } diff --git a/test/org/apache/catalina/webresources/TestJarWarResourceSet.java b/test/org/apache/catalina/webresources/TestJarWarResourceSet.java index be5d98f87a..be4ec953c4 100644 --- a/test/org/apache/catalina/webresources/TestJarWarResourceSet.java +++ b/test/org/apache/catalina/webresources/TestJarWarResourceSet.java @@ -17,6 +17,8 @@ package org.apache.catalina.webresources; import java.io.File; +import java.util.Map; +import java.util.jar.JarEntry; import org.junit.Assert; import org.junit.Before; @@ -53,4 +55,28 @@ public class TestJarWarResourceSet extends TomcatBaseTest { Assert.assertEquals(1, results.length); Assert.assertNotNull(results[0].getURL()); } + + @Test + public void testBloomFilterIsNotRebuiltOnEveryIndexRead() { + // Regression test: the bloom filter for the JAR nested in the WAR + // must be built once when the archive index is built and then + // reused. It previously was re-created on every call to + // getArchiveEntries(), adding work linear in the number of entries + // of the JAR to each resource lookup. + StandardRoot resourceRoot = new StandardRoot(); + resourceRoot.setArchiveIndexStrategy("BLOOM"); + File warFile = new File("test/webresources/war-url-connection.war"); + JarWarResourceSet resourceSet = new JarWarResourceSet(resourceRoot, "/WEB-INF/classes", + warFile.getAbsolutePath(), "WEB-INF/lib/test.jar", "/"); + + Map<String,JarEntry> entries = resourceSet.getArchiveEntries(true); + Assert.assertNotNull(entries); + JarContents jarContents = resourceSet.jarContents; + Assert.assertNotNull(jarContents); + + Assert.assertSame(entries, resourceSet.getArchiveEntries(true)); + Assert.assertSame(jarContents, resourceSet.jarContents); + + resourceSet.gc(); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
