You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2014/11/22 13:43:23 UTC

svn commit: r1641056 - in /tomcat/tc8.0.x/trunk: ./ java/org/apache/jasper/compiler/ java/org/apache/tomcat/util/descriptor/tld/ webapps/docs/

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


Re: svn commit: r1641056 - in /tomcat/tc8.0.x/trunk: ./ java/org/apache/jasper/compiler/ java/org/apache/tomcat/util/descriptor/tld/ webapps/docs/

Posted by Konstantin Kolinko <kk...@apache.org>.
2014-11-22 15:43 GMT+03:00  <kk...@apache.org>:
> 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.

For a record: Tomcat 7 is OK and already has the necessary Jar.close() calls.


Best regards,
Konstantin Kolinko

>
> 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().

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org