From 697b0cccf2fa26144fa2f2c2d70842ad35d8b9c3 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Mon, 28 Sep 2015 09:46:55 +1000 Subject: [PATCH] 477895 Prevent leak of handles to deleted files after redeploy --- .../annotations/AnnotationConfiguration.java | 38 +++++++++++++------ .../jetty/annotations/AnnotationParser.java | 31 +++++++++++---- .../jetty/util/resource/JarResource.java | 8 ++-- .../jetty/webapp/MetaInfConfiguration.java | 20 ++++++++-- .../jetty/webapp/WebAppClassLoader.java | 7 ++++ .../eclipse/jetty/webapp/WebAppContext.java | 7 ++++ .../jetty/webapp/WebInfConfiguration.java | 6 ++- 7 files changed, 89 insertions(+), 28 deletions(-) diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java index 8713d2bbc3d..3afe51df2c9 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java @@ -88,7 +88,7 @@ public class AnnotationConfiguration extends AbstractConfiguration protected CounterStatistic _webInfLibStats; protected CounterStatistic _webInfClassesStats; protected Pattern _sciExcludePattern; - + protected ServiceLoader _loadedInitializers = null; /** * TimeStatistic * @@ -413,6 +413,9 @@ public class AnnotationConfiguration extends AbstractConfiguration context.removeBean(starter); context.removeAttribute(CONTAINER_INITIALIZER_STARTER); } + + if (_loadedInitializers != null) + _loadedInitializers.reload(); } /** @@ -735,7 +738,7 @@ public class AnnotationConfiguration extends AbstractConfiguration loadingJarName = loadingJarName.substring(0,i+4); loadingJarName = (loadingJarName.startsWith("jar:")?loadingJarName.substring(4):loadingJarName); - return Resource.newResource(loadingJarName); + return Resource.newResource(loadingJarName, false); } /** @@ -823,22 +826,33 @@ public class AnnotationConfiguration extends AbstractConfiguration return sci.getClass().getClassLoader()==context.getClassLoader().getParent(); } + /** + * Get SCIs that are not excluded from consideration + * @param context + * @return + * @throws Exception + */ + /** + * @param context + * @return + * @throws Exception + */ public List getNonExcludedInitializers (WebAppContext context) throws Exception { ArrayList nonExcludedInitializers = new ArrayList(); - + //We use the ServiceLoader mechanism to find the ServletContainerInitializer classes to inspect long start = 0; ClassLoader old = Thread.currentThread().getContextClassLoader(); - ServiceLoader loadedInitializers = null; + try { if (LOG.isDebugEnabled()) start = System.nanoTime(); Thread.currentThread().setContextClassLoader(context.getClassLoader()); - loadedInitializers = ServiceLoader.load(ServletContainerInitializer.class); + _loadedInitializers = ServiceLoader.load(ServletContainerInitializer.class); } finally { @@ -847,21 +861,22 @@ public class AnnotationConfiguration extends AbstractConfiguration if (LOG.isDebugEnabled()) LOG.debug("Service loaders found in {}ms", (TimeUnit.MILLISECONDS.convert((System.nanoTime()-start), TimeUnit.NANOSECONDS))); - + Map sciResourceMap = new HashMap(); ServletContainerInitializerOrdering initializerOrdering = getInitializerOrdering(context); //Get initial set of SCIs that aren't from excluded jars or excluded by the containerExclusionPattern, or excluded //because containerInitializerOrdering omits it - for (ServletContainerInitializer sci:loadedInitializers) - { + for (ServletContainerInitializer sci:_loadedInitializers) + { + if (matchesExclusionPattern(sci)) { if (LOG.isDebugEnabled()) LOG.debug("{} excluded by pattern", sci); continue; } - + Resource sciResource = getJarFor(sci); if (isFromExcludedJar(context, sci, sciResource)) { @@ -921,7 +936,7 @@ public class AnnotationConfiguration extends AbstractConfiguration { for (Map.Entry entry:sciResourceMap.entrySet()) { - if (webInfJar.equals(entry.getValue())) + if (webInfJar.equals(entry.getValue())) nonExcludedInitializers.add(entry.getKey()); } } @@ -933,7 +948,8 @@ public class AnnotationConfiguration extends AbstractConfiguration int i=0; for (ServletContainerInitializer sci:nonExcludedInitializers) LOG.debug("ServletContainerInitializer: {} {} from {}",(++i), sci.getClass().getName(), sciResourceMap.get(sci)); - } + } + return nonExcludedInitializers; } diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java index 587743cc341..1f261e07371 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java @@ -562,7 +562,10 @@ public class AnnotationParser if (resource!= null) { Resource r = Resource.newResource(resource); - scanClass(handlers, null, r.getInputStream()); + try (InputStream is = r.getInputStream()) + { + scanClass(handlers, null, is); + } } } } @@ -594,7 +597,10 @@ public class AnnotationParser if (resource!= null) { Resource r = Resource.newResource(resource); - scanClass(handlers, null, r.getInputStream()); + try (InputStream is = r.getInputStream()) + { + scanClass(handlers, null, is); + } } } } @@ -650,7 +656,10 @@ public class AnnotationParser if (resource!= null) { Resource r = Resource.newResource(resource); - scanClass(handlers, null, r.getInputStream()); + try (InputStream is = r.getInputStream()) + { + scanClass(handlers, null, is); + } } } } @@ -845,8 +854,11 @@ public class AnnotationParser if (fullname.endsWith(".class")) { - scanClass(handlers, null, r.getInputStream()); - return; + try (InputStream is=r.getInputStream()) + { + scanClass(handlers, null, is); + return; + } } if (LOG.isDebugEnabled()) LOG.warn("Resource not scannable for classes: {}", r); @@ -963,11 +975,14 @@ public class AnnotationParser if ((resolver == null) || - (!resolver.isExcluded(shortName) && (!isParsed(shortName) || resolver.shouldOverride(shortName)))) + (!resolver.isExcluded(shortName) && (!isParsed(shortName) || resolver.shouldOverride(shortName)))) { - Resource clazz = Resource.newResource("jar:"+jar.getURI()+"!/"+name); + Resource clazz = Resource.newResource("jar:"+jar.getURI()+"!/"+name,false); if (LOG.isDebugEnabled()) {LOG.debug("Scanning class from jar {}", clazz);}; - scanClass(handlers, jar, clazz.getInputStream()); + try (InputStream is = clazz.getInputStream()) + { + scanClass(handlers, jar, is); + } } } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarResource.java index 301ade8e8a9..4316fa13a86 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarResource.java @@ -26,6 +26,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.net.JarURLConnection; import java.net.URL; +import java.net.URLConnection; import java.util.jar.JarEntry; import java.util.jar.JarInputStream; import java.util.jar.Manifest; @@ -156,9 +157,10 @@ public class JarResource extends URLResource if (LOG.isDebugEnabled()) LOG.debug("Extracting entry = "+subEntryName+" from jar "+jarFileURL); - - try (InputStream is = jarFileURL.openConnection().getInputStream(); - JarInputStream jin = new JarInputStream(is)) + URLConnection c = jarFileURL.openConnection(); + c.setUseCaches(false); + try (InputStream is = c.getInputStream(); + JarInputStream jin = new JarInputStream(is)) { JarEntry entry; boolean shouldExtract; diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java index c17093b3547..497d7b78485 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java @@ -181,10 +181,14 @@ public class MetaInfConfiguration extends AbstractConfiguration { //Resource represents a packed jar URI uri = target.getURI(); - resourcesDir = Resource.newResource("jar:"+uri+"!/META-INF/resources"); + resourcesDir = Resource.newResource("jar:"+uri+"!/META-INF/resources", false); } + if (!resourcesDir.exists() || !resourcesDir.isDirectory()) + { + resourcesDir.close(); resourcesDir = EmptyResource.INSTANCE; + } if (cache != null) { @@ -196,7 +200,9 @@ public class MetaInfConfiguration extends AbstractConfiguration } if (resourcesDir == EmptyResource.INSTANCE) + { return; + } } //add it to the meta inf resources for this context @@ -207,6 +213,7 @@ public class MetaInfConfiguration extends AbstractConfiguration context.setAttribute(METAINF_RESOURCES, dirs); } if (LOG.isDebugEnabled()) LOG.debug(resourcesDir+" added to context"); + dirs.add(resourcesDir); } @@ -245,10 +252,13 @@ public class MetaInfConfiguration extends AbstractConfiguration else { URI uri = jar.getURI(); - webFrag = Resource.newResource("jar:"+uri+"!/META-INF/web-fragment.xml"); + webFrag = Resource.newResource("jar:"+uri+"!/META-INF/web-fragment.xml", false); } if (!webFrag.exists() || webFrag.isDirectory()) + { + webFrag.close(); webFrag = EmptyResource.INSTANCE; + } if (cache != null) { @@ -342,8 +352,10 @@ public class MetaInfConfiguration extends AbstractConfiguration @Override public void postConfigure(WebAppContext context) throws Exception { - context.setAttribute(METAINF_FRAGMENTS, null); context.setAttribute(METAINF_RESOURCES, null); + + context.setAttribute(METAINF_FRAGMENTS, null); + context.setAttribute(METAINF_TLDS, null); } @@ -392,7 +404,7 @@ public class MetaInfConfiguration extends AbstractConfiguration URL url = new URL("jar:"+uri+"!/"); JarURLConnection jarConn = (JarURLConnection) url.openConnection(); - jarConn.setUseCaches(Resource.getDefaultUseCaches()); + jarConn.setUseCaches(false); JarFile jarFile = jarConn.getJarFile(); Enumeration entries = jarFile.entries(); while (entries.hasMoreElements()) diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java index 32bdccd9466..b4c684a912c 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java @@ -599,6 +599,13 @@ public class WebAppClassLoader extends URLClassLoader return clazz; } + + @Override + public void close() throws IOException + { + super.close(); + } + /* ------------------------------------------------------------ */ @Override public String toString() diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index cfccb0566ef..054ae558979 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java @@ -22,6 +22,7 @@ import java.io.File; import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; +import java.net.URLClassLoader; import java.security.PermissionCollection; import java.util.ArrayList; import java.util.Arrays; @@ -59,6 +60,7 @@ import org.eclipse.jetty.servlet.ErrorPageErrorHandler; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHandler; import org.eclipse.jetty.util.AttributesMap; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.Loader; import org.eclipse.jetty.util.MultiException; import org.eclipse.jetty.util.URIUtil; @@ -1354,7 +1356,12 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL finally { if (_ownClassLoader) + { + ClassLoader loader = getClassLoader(); + if (loader != null && loader instanceof URLClassLoader) + ((URLClassLoader)loader).close(); setClassLoader(null); + } setAvailable(true); _unavailableException=null; diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java index 3dccf821d08..095d109e3d8 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java @@ -192,15 +192,17 @@ public class WebInfConfiguration extends AbstractConfiguration //if we're not persisting the temp dir contents delete it if (!context.isPersistTempDirectory()) { - IO.delete(context.getTempDirectory()); + IO.delete(context.getTempDirectory()); } //if it wasn't explicitly configured by the user, then unset it - Boolean tmpdirConfigured = (Boolean)context.getAttribute(TEMPDIR_CONFIGURED); + Boolean tmpdirConfigured = (Boolean)context.getAttribute(TEMPDIR_CONFIGURED); if (tmpdirConfigured != null && !tmpdirConfigured) context.setTempDirectory(null); //reset the base resource back to what it was before we did any unpacking of resources + if (context.getBaseResource() != null) + context.getBaseResource().close(); context.setBaseResource(_preUnpackBaseResource); }