You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2016/02/18 15:49:17 UTC

svn commit: r1731079 - in /tomcat/trunk: java/org/apache/catalina/webresources/ webapps/docs/

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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org