This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 270ea3484db5ed5c6b9e26af6a04f5fcc653f300 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 | 4 ++-- .../webresources/TestJarWarResourceSet.java | 26 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java b/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java index e670aeac9c..009f1b7a70 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 a8ee1647c1..1c59fb10cb 100644 --- a/java/org/apache/catalina/webresources/JarWarResourceSet.java +++ b/java/org/apache/catalina/webresources/JarWarResourceSet.java @@ -157,8 +157,8 @@ public class JarWarResourceSet extends AbstractArchiveResourceSet { } } WebResourceRoot root = getRoot(); - if (root.getArchiveIndexStrategyEnum().getUsesBloom() || - root.getContext() != null && root.getContext().getUseBloomFilterForArchives()) { + if (jarContents == null && (root.getArchiveIndexStrategyEnum().getUsesBloom() || + root.getContext() != null && root.getContext().getUseBloomFilterForArchives())) { 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]
