Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=25060

When stopping naming resources look for DataSource resources with a zero-arg close() method and call it if one is found
Works with Commons DBCP.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1079575 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java
index d5d80e3..f74bb75 100644 (file)
--- a/java/org/apache/catalina/core/StandardContext.java
+++ b/java/org/apache/catalina/core/StandardContext.java
@@ -5384,15 +5384,13 @@ public class StandardContext extends ContainerBase
         
         setState(LifecycleState.STOPPING);
 
-        // Currently this is effectively a NO-OP but needs to be called to
-        // ensure the NamingResources follows the correct lifecycle
+        // Binding thread
+        ClassLoader oldCCL = bindThread();
+
         if (namingResources != null) {
             namingResources.stop();
         }
         
-        // Binding thread
-        ClassLoader oldCCL = bindThread();
-
         try {
 
             // Stop our child containers, if any

diff --git a/java/org/apache/catalina/deploy/LocalStrings.properties b/java/org/apache/catalina/deploy/LocalStrings.properties
index 915b6bf..5f2b8cd 100644 (file)
--- a/java/org/apache/catalina/deploy/LocalStrings.properties
+++ b/java/org/apache/catalina/deploy/LocalStrings.properties
@@ -43,5 +43,10 @@ webxml.unrecognisedPublicId=The public ID [{0}] did not match any of the known p
 webXml.version.nfe=Unable to parse [{0}] from the version string [{1}]. This component of the version string will be ignored. 
 webXml.wrongFragmentName=Used a wrong fragment name {0} at web.xml absolute-ordering tag!
 
+namingResources.cleanupCloseFailed=Failed to invoke close method for resource [{0}] in container [{1}] so no cleanup was performed for that resource
+namingResources.cleanupCloseSecurity=Unable to retrieve close method for resource [{0}] in container [{1}] so no cleanup was performed for that resource
+namingResources.cleanupNoClose=Resource [{0}] in container [{1}] does not have a close method so no cleanup was performed for that resource
+namingResources.cleanupNoContext=Failed to retrieve JNDI naming context for container [{0}] so no cleanup was performed for that container
+namingResources.cleanupNoResource=Failed to retrieve JNDI resource [{0}] for container [{1}] so no cleanup was performed for that resource
 namingResources.mbeanCreateFail=Failed to create MBean for naming resource [{0}]
 namingResources.mbeanDestroyFail=Failed to destroy MBean for naming resource [{0}]

diff --git a/java/org/apache/catalina/deploy/NamingResources.java b/java/org/apache/catalina/deploy/NamingResources.java
index dff9541..726a680 100644 (file)
--- a/java/org/apache/catalina/deploy/NamingResources.java
+++ b/java/org/apache/catalina/deploy/NamingResources.java
@@ -22,9 +22,14 @@ package org.apache.catalina.deploy;
 import java.beans.PropertyChangeListener;
 import java.beans.PropertyChangeSupport;
 import java.io.Serializable;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.Hashtable;
 
+import javax.naming.NamingException;
+import javax.sql.DataSource;
+
 import org.apache.catalina.Container;
 import org.apache.catalina.Context;
 import org.apache.catalina.Engine;
@@ -35,6 +40,7 @@ import org.apache.catalina.mbeans.MBeanUtils;
 import org.apache.catalina.util.LifecycleMBeanBase;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.naming.ContextBindings;
 import org.apache.tomcat.util.res.StringManager;
 
 
@@ -949,11 +955,84 @@ public class NamingResources extends LifecycleMBeanBase implements Serializable
 
     @Override
     protected void stopInternal() throws LifecycleException {
+        cleanUp();
         setState(LifecycleState.STOPPING);
         fireLifecycleEvent(CONFIGURE_STOP_EVENT, null);
     }
 
+    /**
+     * Close those resources that an explicit close may help clean-up faster.
+     */
+    private void cleanUp() {
+        if (resources.size() == 0) {
+            return;
+        }
+        javax.naming.Context ctxt;
+        try {
+            if (container instanceof Server) {
+                ctxt = ((Server) container).getGlobalNamingContext();
+            } else {
+                ctxt = ContextBindings.getClassLoader();
+                ctxt = (javax.naming.Context) ctxt.lookup("comp/env");
+            }
+        } catch (NamingException e) {
+            log.warn(sm.getString("namingResources.cleanupNoContext",
+                    container), e);
+            return;
+        }
+        for (ContextResource cr: resources.values()) {
+            if (DataSource.class.getName().equals(cr.getType())) {
+                String name = cr.getName();
+                DataSource ds;
+                try {
+                     ds = (DataSource) ctxt.lookup(name);
+                } catch (NamingException e) {
+                    log.warn(sm.getString("namingResources.cleanupNoResource",
+                            cr.getName(), container), e);
+                    continue;
+                }
+                cleanUp(ds, name);
+            }
+        }
+    }
+
     
+    /**
+     * Closing a database connection pool will close it's open connections. This
+     * will happen on GC but that leaves db connections open that may cause
+     * issues.
+     * @param ds    The DataSource to close.
+     */
+    private void cleanUp(DataSource ds, String name) {
+        // Look for a zero-arg close() method
+        Method m = null;
+        try {
+            m = ds.getClass().getMethod("close", (Class<?>[]) null);
+        } catch (SecurityException e) {
+            log.debug(sm.getString("namingResources.cleanupCloseSecurity", name,
+                    container));
+            return;
+        } catch (NoSuchMethodException e) {
+            log.debug(sm.getString("namingResources.cleanupNoClose", name,
+                    container));
+            return;
+        }
+        if (m != null) {
+            try {
+                m.invoke(ds, (Object[]) null);
+            } catch (IllegalArgumentException e) {
+                log.warn(sm.getString("namingResources.cleanupCloseFailed",
+                        name, container), e);
+            } catch (IllegalAccessException e) {
+                log.warn(sm.getString("namingResources.cleanupCloseFailed",
+                        name, container), e);
+            } catch (InvocationTargetException e) {
+                log.warn(sm.getString("namingResources.cleanupCloseFailed",
+                        name, container), e);
+            }
+        }
+    }
+
     @Override
     protected void destroyInternal() throws LifecycleException {
 

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3c6d734..6967424 100644 (file)
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -45,19 +45,26 @@
 <section name="Tomcat 7.0.11 (markt)">
   <subsection name="Catalina">
     <changelog>
-      <fix>
+      <add>
+        <bug>25060</bug>: Close Apache Commons DBCP datasources when the
+        associated JNDI naming context is stopped (e.g. for a non-global
+        DataSource resource on web application reload) to close remaining
+        database connections immediately rather than waiting for garbage 
+        collection. (markt)
+      </add>
+      <add>
         <bug>26701</bug>: Provide a mechanism for users to register their own
         <code>URLStreamHandlerFactory</code> objects. (markt)
-      </fix>
+      </add>
       <fix>
         <bug>50855</bug>: Fix NPE on HttpServletRequest.logout() when debug
         logging is enabled. (markt)
       </fix>
-      <fix>
+      <add>
         New context attribute "swallowAbortedUploads" allows
         to make request data swallowing configurable for requests
         that are too large. (rjung)
-      </fix>
+      </add>
     </changelog>
   </subsection>
   <subsection name="Coyote">