This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 3bce4d9414f0ec7184e5d28e31882b32f7fc855e 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 0a0e2a6358..c43479f4ea 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 3b88af6d46..c519e108a9 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]
