From 69bb19730bf2d4fb506e24116cb577553643bd78 Mon Sep 17 00:00:00 2001 From: markt Date: Tue, 8 Mar 2011 22:15:34 +0000 Subject: [PATCH] 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 --- java/org/apache/catalina/core/StandardContext.java | 8 +-- .../apache/catalina/deploy/LocalStrings.properties | 5 ++ .../apache/catalina/deploy/NamingResources.java | 79 ++++++++++++++++++++++ webapps/docs/changelog.xml | 15 ++-- 4 files changed, 98 insertions(+), 9 deletions(-) diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java index d5d80e3d7..f74bb75bd 100644 --- 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 915b6bf4d..5f2b8cd5d 100644 --- 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 dff954190..726a68076 100644 --- 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 3c6d734c0..6967424d5 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -45,19 +45,26 @@
- + + 25060: 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) + + 26701: Provide a mechanism for users to register their own URLStreamHandlerFactory objects. (markt) - + 50855: Fix NPE on HttpServletRequest.logout() when debug logging is enabled. (markt) - + New context attribute "swallowAbortedUploads" allows to make request data swallowing configurable for requests that are too large. (rjung) - + -- 2.11.0