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 abfc4e422e6c143d693e9c5916313b9f7f61525b 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 41d2b0a38f..cdb3d66aac 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 82fd0f2e81..b5b91ef28c 100644 --- a/java/org/apache/catalina/webresources/JarWarResourceSet.java +++ b/java/org/apache/catalina/webresources/JarWarResourceSet.java @@ -156,7 +156,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]
