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 2019/12/03 13:13:04 UTC

[tomcat] branch master updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63983 fd 'leak'

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
     new b91a449  Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63983 fd 'leak'
b91a449 is described below

commit b91a449c64ae2ba65b067e6f963e1748bff73055
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Dec 3 13:11:53 2019 +0000

    Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63983 fd 'leak'
    
    Correct a regression in the static resource caching changes introduced
    in 9.0.28. A large number of file descriptors were opened that could
    reach the OS limit before being released by GC.
---
 .../catalina/webresources/CachedResource.java      | 38 ++++++++--------------
 webapps/docs/changelog.xml                         |  6 ++++
 2 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/java/org/apache/catalina/webresources/CachedResource.java b/java/org/apache/catalina/webresources/CachedResource.java
index ec21f40..58247a8 100644
--- a/java/org/apache/catalina/webresources/CachedResource.java
+++ b/java/org/apache/catalina/webresources/CachedResource.java
@@ -461,36 +461,31 @@ public class CachedResource implements WebResource {
         private final StandardRoot root;
         private final String webAppPath;
         private final boolean usesClassLoaderResources;
-        private final URLConnection resourceURLConnection;
-        private boolean connected;
+        private final URL resourceURL;
 
         protected CachedResourceURLConnection(URL resourceURL, StandardRoot root, String webAppPath,
-                boolean usesClassLoaderResources) throws IOException {
+                boolean usesClassLoaderResources) {
             super(resourceURL);
             this.root = root;
             this.webAppPath = webAppPath;
             this.usesClassLoaderResources = usesClassLoaderResources;
-            this.resourceURLConnection = url.openConnection();
+            this.resourceURL = resourceURL;
         }
 
         @Override
         public void connect() throws IOException {
-            if (!connected) {
-                resourceURLConnection.connect();
-                connected = true;
-            }
+            // NO-OP
         }
 
         @Override
         public InputStream getInputStream() throws IOException {
-            connect();
-            InputStream is = getResource().getInputStream();
-            return is;
+            return getResource().getInputStream();
         }
 
         @Override
         public Permission getPermission() throws IOException {
-            return resourceURLConnection.getPermission();
+            // Doesn't trigger a call to connect for file:// URLs
+            return resourceURL.openConnection().getPermission();
         }
 
         @Override
@@ -517,8 +512,7 @@ public class CachedResource implements WebResource {
         private final StandardRoot root;
         private final String webAppPath;
         private final boolean usesClassLoaderResources;
-        private final JarURLConnection resourceURLConnection;
-        private boolean connected;
+        private final URL resourceURL;
 
         protected CachedResourceJarURLConnection(URL resourceURL, StandardRoot root, String webAppPath,
                 boolean usesClassLoaderResources) throws IOException {
@@ -526,27 +520,23 @@ public class CachedResource implements WebResource {
             this.root = root;
             this.webAppPath = webAppPath;
             this.usesClassLoaderResources = usesClassLoaderResources;
-            this.resourceURLConnection = (JarURLConnection) url.openConnection();
+            this.resourceURL = resourceURL;
         }
 
         @Override
         public void connect() throws IOException {
-            if (!connected) {
-                resourceURLConnection.connect();
-                connected = true;
-            }
+            // NO-OP
         }
 
         @Override
         public InputStream getInputStream() throws IOException {
-            connect();
-            InputStream is = getResource().getInputStream();
-            return is;
+            return getResource().getInputStream();
         }
 
         @Override
         public Permission getPermission() throws IOException {
-            return resourceURLConnection.getPermission();
+            // Doesn't trigger a call to connect for jar:// URLs
+            return resourceURL.openConnection().getPermission();
         }
 
         @Override
@@ -565,7 +555,7 @@ public class CachedResource implements WebResource {
 
         @Override
         public JarFile getJarFile() throws IOException {
-            return resourceURLConnection.getJarFile();
+            return ((JarURLConnection) resourceURL.openConnection()).getJarFile();
         }
     }
 }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 7919779..cedc2bd 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -81,6 +81,12 @@
         subsequent calls triggering the logging of a warning. Based on a patch
         by Andy Wilkinson. (markt)
       </fix>
+      <fix>
+        <bug>63983</bug>: Correct a regression in the static resource caching
+        changes introduced in 9.0.28. A large number of file descriptors were
+        opened that could reach the OS limit before being released by GC.
+        (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