Author: markt Date: Thu Feb 18 14:49:17 2016 New Revision: 1731079 URL: http://svn.apache.org/viewvc?rev=1731079&view=rev Log: Refactor the JAR and JAR-in-WAR resource handling to reduce the memory footprint of the web application.
Modified: tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java tomcat/trunk/java/org/apache/catalina/webresources/JarResourceSet.java tomcat/trunk/java/org/apache/catalina/webresources/JarWarResourceSet.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java?rev=1731079&r1=1731078&r2=1731079&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java (original) +++ tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java Thu Feb 18 14:49:17 2016 @@ -23,6 +23,7 @@ import java.net.URL; import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; +import java.util.Map; import java.util.Set; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -34,12 +35,12 @@ import org.apache.catalina.util.Resource public abstract class AbstractArchiveResourceSet extends AbstractResourceSet { - private final HashMap<String,JarEntry> jarFileEntries = new HashMap<>(); private URL baseUrl; private String baseUrlString; private JarFile archive = null; - private final Object archiveLock = new Object(); + protected HashMap<String,JarEntry> archiveEntries = null; + protected final Object archiveLock = new Object(); private long archiveUseCount = 0; @@ -61,10 +62,6 @@ public abstract class AbstractArchiveRes return baseUrlString; } - protected final HashMap<String,JarEntry> getJarFileEntries() { - return jarFileEntries; - } - @Override public final String[] list(String path) { @@ -79,7 +76,7 @@ public abstract class AbstractArchiveRes if (pathInJar.length() > 0 && pathInJar.charAt(0) == '/') { pathInJar = pathInJar.substring(1); } - Iterator<String> entries = jarFileEntries.keySet().iterator(); + Iterator<String> entries = getArchiveEntries(false).keySet().iterator(); while (entries.hasNext()) { String name = entries.next(); if (name.length() > pathInJar.length() && @@ -138,7 +135,7 @@ public abstract class AbstractArchiveRes } } - Iterator<String> entries = jarFileEntries.keySet().iterator(); + Iterator<String> entries = getArchiveEntries(false).keySet().iterator(); while (entries.hasNext()) { String name = entries.next(); if (name.length() > pathInJar.length() && @@ -169,6 +166,34 @@ public abstract class AbstractArchiveRes return result; } + + /** + * Obtain the map of entries in the archive. May return null in which case + * {@link #getArchiveEntry(String)} should be used. + * + * @param single Is this request being make to support a single lookup? If + * false, a map will always be returned. If true, + * implementations may use this as a hint in determining the + * optimum way to respond. + * + * @return The archives entries mapped to their names or null if + * {@link #getArchiveEntry(String)} should be used. + */ + protected abstract HashMap<String,JarEntry> getArchiveEntries(boolean single); + + + /** + * Obtain a single entry from the archive. For performance reasons, + * {@link #getArchiveEntries(boolean)} should always be called first and the + * archive entry looked up in the map if one is returned. Only if that call + * returns null should this method be used. + * + * @param pathInArchive The path in the archive of the entry required + * + * @return The specified archive entry or null if it does not exist + */ + protected abstract JarEntry getArchiveEntry(String pathInArchive); + @Override public final boolean mkdir(String path) { checkPath(path); @@ -228,15 +253,24 @@ public abstract class AbstractArchiveRes return new JarResourceRoot(root, new File(getBase()), baseUrlString, path); } else { + Map<String,JarEntry> jarEntries = getArchiveEntries(true); JarEntry jarEntry = null; if (!(pathInJar.charAt(pathInJar.length() - 1) == '/')) { - jarEntry = jarFileEntries.get(pathInJar + '/'); + if (jarEntries == null) { + jarEntry = getArchiveEntry(pathInJar + '/'); + } else { + jarEntry = jarEntries.get(pathInJar + '/'); + } if (jarEntry != null) { path = path + '/'; } } if (jarEntry == null) { - jarEntry = jarFileEntries.get(pathInJar); + if (jarEntries == null) { + jarEntry = getArchiveEntry(pathInJar); + } else { + jarEntry = jarEntries.get(pathInJar); + } } if (jarEntry == null) { return new EmptyResource(root, path); @@ -294,6 +328,7 @@ public abstract class AbstractArchiveRes // Log at least WARN } archive = null; + archiveEntries = null; } } } Modified: tomcat/trunk/java/org/apache/catalina/webresources/JarResourceSet.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/JarResourceSet.java?rev=1731079&r1=1731078&r2=1731079&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/webresources/JarResourceSet.java (original) +++ tomcat/trunk/java/org/apache/catalina/webresources/JarResourceSet.java Thu Feb 18 14:49:17 2016 @@ -20,6 +20,7 @@ import java.io.File; import java.io.IOException; import java.net.MalformedURLException; import java.util.Enumeration; +import java.util.HashMap; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.jar.Manifest; @@ -81,16 +82,57 @@ public class JarResourceSet extends Abst return new JarResource(this, webAppPath, getBaseUrlString(), jarEntry); } + + @Override + protected HashMap<String,JarEntry> getArchiveEntries(boolean single) { + synchronized (archiveLock) { + if (archiveEntries == null && !single) { + JarFile jarFile = null; + archiveEntries = new HashMap<>(); + try { + jarFile = openJarFile(); + Enumeration<JarEntry> entries = jarFile.entries(); + while (entries.hasMoreElements()) { + JarEntry entry = entries.nextElement(); + archiveEntries.put(entry.getName(), entry); + } + } catch (IOException ioe) { + // Should never happen + archiveEntries = null; + throw new IllegalStateException(ioe); + } finally { + if (jarFile != null) { + closeJarFile(); + } + } + } + return archiveEntries; + } + } + + + @Override + protected JarEntry getArchiveEntry(String pathInArchive) { + JarFile jarFile = null; + try { + jarFile = openJarFile(); + return jarFile.getJarEntry(pathInArchive); + } catch (IOException ioe) { + // Should never happen + throw new IllegalStateException(ioe); + } finally { + if (jarFile != null) { + closeJarFile(); + } + } + } + + //-------------------------------------------------------- Lifecycle methods @Override protected void initInternal() throws LifecycleException { try (JarFile jarFile = new JarFile(getBase())) { - Enumeration<JarEntry> entries = jarFile.entries(); - while (entries.hasMoreElements()) { - JarEntry entry = entries.nextElement(); - getJarFileEntries().put(entry.getName(), entry); - } setManifest(jarFile.getManifest()); } catch (IOException ioe) { throw new IllegalArgumentException(ioe); Modified: tomcat/trunk/java/org/apache/catalina/webresources/JarWarResourceSet.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/JarWarResourceSet.java?rev=1731079&r1=1731078&r2=1731079&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/webresources/JarWarResourceSet.java (original) +++ tomcat/trunk/java/org/apache/catalina/webresources/JarWarResourceSet.java Thu Feb 18 14:49:17 2016 @@ -20,6 +20,7 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.net.MalformedURLException; +import java.util.HashMap; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.jar.JarInputStream; @@ -84,6 +85,67 @@ public class JarWarResourceSet extends A return new JarWarResource(this, webAppPath, getBaseUrlString(), jarEntry, archivePath); } + + /** + * {@inheritDoc} + * <p> + * JarWar can't optimise for a single resource so the Map is always + * returned. + */ + @Override + protected HashMap<String,JarEntry> getArchiveEntries(boolean single) { + synchronized (archiveLock) { + if (archiveEntries == null) { + JarFile warFile = null; + InputStream jarFileIs = null; + archiveEntries = new HashMap<>(); + try { + warFile = openJarFile(); + JarEntry jarFileInWar = warFile.getJarEntry(archivePath); + jarFileIs = warFile.getInputStream(jarFileInWar); + + try (JarInputStream jarIs = new JarInputStream(jarFileIs)) { + JarEntry entry = jarIs.getNextJarEntry(); + while (entry != null) { + archiveEntries.put(entry.getName(), entry); + entry = jarIs.getNextJarEntry(); + } + setManifest(jarIs.getManifest()); + } + } catch (IOException ioe) { + // Should never happen + archiveEntries = null; + throw new IllegalStateException(ioe); + } finally { + if (warFile != null) { + closeJarFile(); + } + if (jarFileIs != null) { + try { + jarFileIs.close(); + } catch (IOException e) { + // Ignore + } + } + } + } + return archiveEntries; + } + } + + + /** + * {@inheritDoc} + * <p> + * Should never be called since {@link #getArchiveEntries(boolean)} always + * returns a Map. + */ + @Override + protected JarEntry getArchiveEntry(String pathInArchive) { + throw new IllegalStateException("Coding error"); + } + + //-------------------------------------------------------- Lifecycle methods @Override protected void initInternal() throws LifecycleException { @@ -93,11 +155,6 @@ public class JarWarResourceSet extends A InputStream jarFileIs = warFile.getInputStream(jarFileInWar); try (JarInputStream jarIs = new JarInputStream(jarFileIs)) { - JarEntry entry = jarIs.getNextJarEntry(); - while (entry != null) { - getJarFileEntries().put(entry.getName(), entry); - entry = jarIs.getNextJarEntry(); - } setManifest(jarIs.getManifest()); } } catch (IOException ioe) { Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1731079&r1=1731078&r2=1731079&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Thu Feb 18 14:49:17 2016 @@ -88,6 +88,10 @@ Fix some resource leaks in the error handling for accessing files from JARs and WARs. (markt) </fix> + <fix> + Refactor the JAR and JAR-in-WAR resource handling to reduce the memory + footprint of the web application. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org