On 29/03/2011 15:52, Filip Hanik - Dev Lists wrote: > namingResources.cleanupNoClose > > couldn't you implement a closeMethod="close" in the<Resource> element, > that way the name of the method is configurable.
Like it. There was a bug reported with the timing of this call so there is some work to do here already. Shouldn't be too hard to add this. Mark > > Filip > > > > On 3/8/2011 3:15 PM, ma...@apache.org wrote: >> Author: markt >> Date: Tue Mar 8 22:15:34 2011 >> New Revision: 1079575 >> >> URL: http://svn.apache.org/viewvc?rev=1079575&view=rev >> Log: >> 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. >> >> Modified: >> tomcat/trunk/java/org/apache/catalina/core/StandardContext.java >> tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties >> tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java >> tomcat/trunk/webapps/docs/changelog.xml >> >> Modified: tomcat/trunk/java/org/apache/catalina/core/StandardContext.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardContext.java?rev=1079575&r1=1079574&r2=1079575&view=diff >> >> ============================================================================== >> >> --- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java >> (original) >> +++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java >> Tue Mar 8 22:15:34 2011 >> @@ -5384,15 +5384,13 @@ public class StandardContext extends Con >> >> 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 >> >> Modified: >> tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties?rev=1079575&r1=1079574&r2=1079575&view=diff >> >> ============================================================================== >> >> --- >> tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties >> (original) >> +++ >> tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties >> Tue Mar 8 22:15:34 2011 >> @@ -43,5 +43,10 @@ webxml.unrecognisedPublicId=The public I >> 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}] >> >> Modified: >> tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java?rev=1079575&r1=1079574&r2=1079575&view=diff >> >> ============================================================================== >> >> --- tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java >> (original) >> +++ tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java >> Tue Mar 8 22:15:34 2011 >> @@ -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.MBeanU >> 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 Lif >> >> @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 { >> >> >> Modified: tomcat/trunk/webapps/docs/changelog.xml >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1079575&r1=1079574&r2=1079575&view=diff >> >> ============================================================================== >> >> --- tomcat/trunk/webapps/docs/changelog.xml (original) >> +++ tomcat/trunk/webapps/docs/changelog.xml Tue Mar 8 22:15:34 2011 >> @@ -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"> >> >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org >> For additional commands, e-mail: dev-h...@tomcat.apache.org >> >> >> >> ----- >> No virus found in this message. >> Checked by AVG - www.avg.com >> Version: 10.0.1204 / Virus Database: 1435/3491 - Release Date: 03/08/11 >> >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
