Author: kkolinko Date: Sat Nov 22 12:43:22 2014 New Revision: 1641056 URL: http://svn.apache.org/r1641056 Log: Fix closing of Jars during annotation scanning. With this all JarFactory.newInstance() calls are now followed by proper Jar.close() calls.
Merged revisions r1640976, r1640978, r1641026, r1641051, r1641052 from tomcat/trunk: Fix Coverty issue 45316: close Jar resource after use. Fix Coverty issue 45276: close Jar resource after use. Rename method, because it creates Jar object that has to be close()'d and is not a simple getter. (As I commented in Re: r1640978) Fix missing Jar.close() in TagFileProcessor.loadTagFile(). Modified: tomcat/tc8.0.x/trunk/ (props changed) tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TagFileProcessor.java tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TldCache.java tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/descriptor/tld/TldResourcePath.java tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml Propchange: tomcat/tc8.0.x/trunk/ ------------------------------------------------------------------------------ Merged /tomcat/trunk:r1640976,1640978,1641026,1641051-1641052 Modified: tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TagFileProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TagFileProcessor.java?rev=1641056&r1=1641055&r2=1641056&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TagFileProcessor.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TagFileProcessor.java Sat Nov 22 12:43:22 2014 @@ -517,90 +517,96 @@ class TagFileProcessor { TagInfo tagInfo, PageInfo parentPageInfo) throws JasperException { Jar tagJar = null; - if (tagFilePath.startsWith("/META-INF/")) { - try { - tagJar = compiler.getCompilationContext().getTldResourcePath( - tagInfo.getTagLibrary().getURI()).getJar(); - } catch (IOException ioe) { - throw new JasperException(ioe); + try { + if (tagFilePath.startsWith("/META-INF/")) { + try { + tagJar = compiler.getCompilationContext().getTldResourcePath( + tagInfo.getTagLibrary().getURI()).openJar(); + } catch (IOException ioe) { + throw new JasperException(ioe); + } + } + String wrapperUri; + if (tagJar == null) { + wrapperUri = tagFilePath; + } else { + wrapperUri = tagJar.getURL(tagFilePath); } - } - String wrapperUri; - if (tagJar == null) { - wrapperUri = tagFilePath; - } else { - wrapperUri = tagJar.getURL(tagFilePath); - } - JspCompilationContext ctxt = compiler.getCompilationContext(); - JspRuntimeContext rctxt = ctxt.getRuntimeContext(); + JspCompilationContext ctxt = compiler.getCompilationContext(); + JspRuntimeContext rctxt = ctxt.getRuntimeContext(); + + synchronized (rctxt) { + JspServletWrapper wrapper = rctxt.getWrapper(wrapperUri); + if (wrapper == null) { + wrapper = new JspServletWrapper(ctxt.getServletContext(), ctxt + .getOptions(), tagFilePath, tagInfo, ctxt + .getRuntimeContext(), tagJar); + rctxt.addWrapper(wrapperUri, wrapper); - synchronized (rctxt) { - JspServletWrapper wrapper = rctxt.getWrapper(wrapperUri); - if (wrapper == null) { - wrapper = new JspServletWrapper(ctxt.getServletContext(), ctxt - .getOptions(), tagFilePath, tagInfo, ctxt - .getRuntimeContext(), tagJar); - rctxt.addWrapper(wrapperUri, wrapper); - - // Use same classloader and classpath for compiling tag files - wrapper.getJspEngineContext().setClassLoader( - ctxt.getClassLoader()); - wrapper.getJspEngineContext().setClassPath(ctxt.getClassPath()); - } else { - // Make sure that JspCompilationContext gets the latest TagInfo - // for the tag file. TagInfo instance was created the last - // time the tag file was scanned for directives, and the tag - // file may have been modified since then. - wrapper.getJspEngineContext().setTagInfo(tagInfo); - } - - Class<?> tagClazz; - int tripCount = wrapper.incTripCount(); - try { - if (tripCount > 0) { - // When tripCount is greater than zero, a circular - // dependency exists. The circularly dependent tag - // file is compiled in prototype mode, to avoid infinite - // recursion. - - JspServletWrapper tempWrapper = new JspServletWrapper(ctxt - .getServletContext(), ctxt.getOptions(), - tagFilePath, tagInfo, ctxt.getRuntimeContext(), - tagJar); // Use same classloader and classpath for compiling tag files - tempWrapper.getJspEngineContext().setClassLoader( + wrapper.getJspEngineContext().setClassLoader( ctxt.getClassLoader()); - tempWrapper.getJspEngineContext().setClassPath(ctxt.getClassPath()); - tagClazz = tempWrapper.loadTagFilePrototype(); - tempVector.add(tempWrapper.getJspEngineContext() - .getCompiler()); + wrapper.getJspEngineContext().setClassPath(ctxt.getClassPath()); } else { - tagClazz = wrapper.loadTagFile(); + // Make sure that JspCompilationContext gets the latest TagInfo + // for the tag file. TagInfo instance was created the last + // time the tag file was scanned for directives, and the tag + // file may have been modified since then. + wrapper.getJspEngineContext().setTagInfo(tagInfo); } - } finally { - wrapper.decTripCount(); - } - // Add the dependents for this tag file to its parent's - // Dependent list. The only reliable dependency information - // can only be obtained from the tag instance. - try { - Object tagIns = tagClazz.newInstance(); - if (tagIns instanceof JspSourceDependent) { - Iterator<Entry<String,Long>> iter = ((JspSourceDependent) - tagIns).getDependants().entrySet().iterator(); - while (iter.hasNext()) { - Entry<String,Long> entry = iter.next(); - parentPageInfo.addDependant(entry.getKey(), - entry.getValue()); + Class<?> tagClazz; + int tripCount = wrapper.incTripCount(); + try { + if (tripCount > 0) { + // When tripCount is greater than zero, a circular + // dependency exists. The circularly dependent tag + // file is compiled in prototype mode, to avoid infinite + // recursion. + + JspServletWrapper tempWrapper = new JspServletWrapper(ctxt + .getServletContext(), ctxt.getOptions(), + tagFilePath, tagInfo, ctxt.getRuntimeContext(), + tagJar); + // Use same classloader and classpath for compiling tag files + tempWrapper.getJspEngineContext().setClassLoader( + ctxt.getClassLoader()); + tempWrapper.getJspEngineContext().setClassPath(ctxt.getClassPath()); + tagClazz = tempWrapper.loadTagFilePrototype(); + tempVector.add(tempWrapper.getJspEngineContext() + .getCompiler()); + } else { + tagClazz = wrapper.loadTagFile(); } + } finally { + wrapper.decTripCount(); } - } catch (Exception e) { - // ignore errors - } - return tagClazz; + // Add the dependents for this tag file to its parent's + // Dependent list. The only reliable dependency information + // can only be obtained from the tag instance. + try { + Object tagIns = tagClazz.newInstance(); + if (tagIns instanceof JspSourceDependent) { + Iterator<Entry<String,Long>> iter = ((JspSourceDependent) + tagIns).getDependants().entrySet().iterator(); + while (iter.hasNext()) { + Entry<String,Long> entry = iter.next(); + parentPageInfo.addDependant(entry.getKey(), + entry.getValue()); + } + } + } catch (Exception e) { + // ignore errors + } + + return tagClazz; + } + } finally { + if (tagJar != null) { + tagJar.close(); + } } } @@ -630,28 +636,23 @@ class TagFileProcessor { TldResourcePath tldResourcePath = compiler.getCompilationContext().getTldResourcePath( tagFileInfo.getTagInfo().getTagLibrary().getURI()); - Jar jar; - try { - jar = tldResourcePath.getJar(); - } catch (IOException ioe) { - throw new JasperException(ioe); - } - if (jar != null) { - try { + + try (Jar jar = tldResourcePath.openJar()) { + + if (jar != null) { // Add TLD pageInfo.addDependant(jar.getURL(tldResourcePath.getEntryName()), - Long.valueOf(jar.getLastModified(tldResourcePath.getEntryName()))); + Long.valueOf(jar.getLastModified(tldResourcePath.getEntryName()))); // Add Tag pageInfo.addDependant(jar.getURL(tagFilePath.substring(1)), - Long.valueOf(jar.getLastModified(tagFilePath.substring(1)))); - } catch (IOException ioe) { - throw new JasperException(ioe); + Long.valueOf(jar.getLastModified(tagFilePath.substring(1)))); + } else { + pageInfo.addDependant(tagFilePath, + compiler.getCompilationContext().getLastModified( + tagFilePath)); } - } - else { - pageInfo.addDependant(tagFilePath, - compiler.getCompilationContext().getLastModified( - tagFilePath)); + } catch (IOException ioe) { + throw new JasperException(ioe); } } else { pageInfo.addDependant(tagFilePath, Modified: tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java?rev=1641056&r1=1641055&r2=1641056&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java Sat Nov 22 12:43:22 2014 @@ -124,100 +124,98 @@ class TagLibraryInfoImpl extends TagLibr tldResourcePath = generateTldResourcePath(uri, ctxt); } - Jar jar; - try { - jar = tldResourcePath.getJar(); - } catch (IOException ioe) { - throw new JasperException(ioe); - } + try (Jar jar = tldResourcePath.openJar()) { - // Add the dependencies on the TLD to the referencing page - PageInfo pageInfo = ctxt.createCompiler().getPageInfo(); - if (pageInfo != null) { - // If the TLD is in a JAR, that JAR may not be part of the web - // application - String path = tldResourcePath.getWebappPath(); - if (path != null) { - // Add TLD (jar==null) / JAR (jar!=null) file to dependency list - pageInfo.addDependant(path, ctxt.getLastModified(path)); - } - if (jar != null) { - if (path == null) { - // JAR not in the web application so add it directly - URL jarUrl = jar.getJarFileURL(); - long lastMod = -1; - URLConnection urlConn = null; + // Add the dependencies on the TLD to the referencing page + PageInfo pageInfo = ctxt.createCompiler().getPageInfo(); + if (pageInfo != null) { + // If the TLD is in a JAR, that JAR may not be part of the web + // application + String path = tldResourcePath.getWebappPath(); + if (path != null) { + // Add TLD (jar==null) / JAR (jar!=null) file to dependency list + pageInfo.addDependant(path, ctxt.getLastModified(path)); + } + if (jar != null) { + if (path == null) { + // JAR not in the web application so add it directly + URL jarUrl = jar.getJarFileURL(); + long lastMod = -1; + URLConnection urlConn = null; + try { + urlConn = jarUrl.openConnection(); + lastMod = urlConn.getLastModified(); + } catch (IOException ioe) { + throw new JasperException(ioe); + } finally { + if (urlConn != null) { + try { + urlConn.getInputStream().close(); + } catch (IOException e) { + // Ignore + } + } + } + pageInfo.addDependant(jarUrl.toExternalForm(), + Long.valueOf(lastMod)); + } + // Add TLD within the JAR to the dependency list + String entryName = tldResourcePath.getEntryName(); try { - urlConn = jarUrl.openConnection(); - lastMod = urlConn.getLastModified(); + pageInfo.addDependant(jar.getURL(entryName), + Long.valueOf(jar.getLastModified(entryName))); } catch (IOException ioe) { throw new JasperException(ioe); - } finally { - if (urlConn != null) { - try { - urlConn.getInputStream().close(); - } catch (IOException e) { - // Ignore - } - } } - pageInfo.addDependant(jarUrl.toExternalForm(), - Long.valueOf(lastMod)); } - // Add TLD within the JAR to the dependency list - String entryName = tldResourcePath.getEntryName(); - try { - pageInfo.addDependant(jar.getURL(entryName), - Long.valueOf(jar.getLastModified(entryName))); - } catch (IOException ioe) { - throw new JasperException(ioe); + } + + // Get the representation of the TLD + TaglibXml taglibXml = + ctxt.getOptions().getTldCache().getTaglibXml(tldResourcePath); + + // Populate the TagLibraryInfo attributes + this.jspversion = taglibXml.getJspVersion(); + this.tlibversion = taglibXml.getTlibVersion(); + this.shortname = taglibXml.getShortName(); + this.urn = taglibXml.getUri(); + this.info = taglibXml.getInfo(); + + this.tagLibraryValidator = createValidator(taglibXml.getValidator()); + + List<TagInfo> tagInfos = new ArrayList<>(); + for (TagXml tagXml : taglibXml.getTags()) { + tagInfos.add(createTagInfo(tagXml)); + } + + List<TagFileInfo> tagFileInfos = new ArrayList<>(); + for (TagFileXml tagFileXml : taglibXml.getTagFiles()) { + tagFileInfos.add(createTagFileInfo(tagFileXml, jar)); + } + + Set<String> names = new HashSet<>(); + List<FunctionInfo> functionInfos = taglibXml.getFunctions(); + // TODO Move this validation to the parsing stage + for (FunctionInfo functionInfo : functionInfos) { + String name = functionInfo.getName(); + if (!names.add(name)) { + err.jspError("jsp.error.tld.fn.duplicate.name", name, uri); } } - } - // Get the representation of the TLD - TaglibXml taglibXml = - ctxt.getOptions().getTldCache().getTaglibXml(tldResourcePath); - - // Populate the TagLibraryInfo attributes - this.jspversion = taglibXml.getJspVersion(); - this.tlibversion = taglibXml.getTlibVersion(); - this.shortname = taglibXml.getShortName(); - this.urn = taglibXml.getUri(); - this.info = taglibXml.getInfo(); - - this.tagLibraryValidator = createValidator(taglibXml.getValidator()); - - List<TagInfo> tagInfos = new ArrayList<>(); - for (TagXml tagXml : taglibXml.getTags()) { - tagInfos.add(createTagInfo(tagXml)); - } - - List<TagFileInfo> tagFileInfos = new ArrayList<>(); - for (TagFileXml tagFileXml : taglibXml.getTagFiles()) { - tagFileInfos.add(createTagFileInfo(tagFileXml, jar)); - } - - Set<String> names = new HashSet<>(); - List<FunctionInfo> functionInfos = taglibXml.getFunctions(); - // TODO Move this validation to the parsing stage - for (FunctionInfo functionInfo : functionInfos) { - String name = functionInfo.getName(); - if (!names.add(name)) { - err.jspError("jsp.error.tld.fn.duplicate.name", name, uri); + if (tlibversion == null) { + err.jspError("jsp.error.tld.mandatory.element.missing", "tlib-version", uri); + } + if (jspversion == null) { + err.jspError("jsp.error.tld.mandatory.element.missing", "jsp-version", uri); } - } - if (tlibversion == null) { - err.jspError("jsp.error.tld.mandatory.element.missing", "tlib-version", uri); - } - if (jspversion == null) { - err.jspError("jsp.error.tld.mandatory.element.missing", "jsp-version", uri); + this.tags = tagInfos.toArray(new TagInfo[tagInfos.size()]); + this.tagFiles = tagFileInfos.toArray(new TagFileInfo[tagFileInfos.size()]); + this.functions = functionInfos.toArray(new FunctionInfo[functionInfos.size()]); + } catch (IOException ioe) { + throw new JasperException(ioe); } - - this.tags = tagInfos.toArray(new TagInfo[tagInfos.size()]); - this.tagFiles = tagFileInfos.toArray(new TagFileInfo[tagFileInfos.size()]); - this.functions = functionInfos.toArray(new FunctionInfo[functionInfos.size()]); } @Override Modified: tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TldCache.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TldCache.java?rev=1641056&r1=1641055&r2=1641056&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TldCache.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/jasper/compiler/TldCache.java Sat Nov 22 12:43:22 2014 @@ -135,7 +135,7 @@ public class TldCache { conn.getInputStream().close(); } } - try (Jar jar = tldResourcePath.getJar()) { + try (Jar jar = tldResourcePath.openJar()) { if (jar != null) { result[1] = jar.getLastModified(tldResourcePath.getEntryName()); } Modified: tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/descriptor/tld/TldResourcePath.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/descriptor/tld/TldResourcePath.java?rev=1641056&r1=1641055&r2=1641056&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/descriptor/tld/TldResourcePath.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/descriptor/tld/TldResourcePath.java Sat Nov 22 12:43:22 2014 @@ -127,7 +127,15 @@ public class TldResourcePath { } } + /** + * @deprecated Renamed, as it is not just a getter method. Use {@link #openJar()}. + */ + @Deprecated public Jar getJar() throws IOException { + return openJar(); + } + + public Jar openJar() throws IOException { if (entryName == null) { return null; } else { Modified: tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml?rev=1641056&r1=1641055&r2=1641056&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml Sat Nov 22 12:43:22 2014 @@ -80,6 +80,9 @@ <bug>57239</bug>: Correct several message typos. Includes patch by vladk. (kkolinko) </fix> + <fix> + Fix closing of Jars during annotation scanning. (schultz/kkolinko) + </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