Author: markt Date: Wed Jun 15 20:32:14 2011 New Revision: 1136179 URL: http://svn.apache.org/viewvc?rev=1136179&view=rev Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51376 Complete fix. Ensure Servlet.destroy() is only called if Servlet.init() was called.
Modified: tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java tomcat/trunk/test/org/apache/catalina/core/TestStandardContext.java Modified: tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java?rev=1136179&r1=1136178&r2=1136179&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java (original) +++ tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java Wed Jun 15 20:32:14 2011 @@ -1375,53 +1375,55 @@ public class StandardWrapper extends Con } } - PrintStream out = System.out; - if (swallowOutput) { - SystemLogHandler.startCapture(); - } - - // Call the servlet destroy() method - try { - instanceSupport.fireInstanceEvent - (InstanceEvent.BEFORE_DESTROY_EVENT, instance); - - if( Globals.IS_SECURITY_ENABLED) { - SecurityUtil.doAsPrivilege("destroy", - instance); - SecurityUtil.remove(instance); - } else { - instance.destroy(); - } - - instanceSupport.fireInstanceEvent - (InstanceEvent.AFTER_DESTROY_EVENT, instance); - - // Annotation processing - if (!((Context) getParent()).getIgnoreAnnotations()) { - ((StandardContext)getParent()).getInstanceManager().destroyInstance(instance); - } - - } catch (Throwable t) { - ExceptionUtils.handleThrowable(t); - instanceSupport.fireInstanceEvent - (InstanceEvent.AFTER_DESTROY_EVENT, instance, t); - instance = null; - instancePool = null; - nInstances = 0; - fireContainerEvent("unload", this); - unloading = false; - throw new ServletException - (sm.getString("standardWrapper.destroyException", getName()), - t); - } finally { - // Write captured output + if (instanceInitialized) { + PrintStream out = System.out; if (swallowOutput) { - String log = SystemLogHandler.stopCapture(); - if (log != null && log.length() > 0) { - if (getServletContext() != null) { - getServletContext().log(log); - } else { - out.println(log); + SystemLogHandler.startCapture(); + } + + // Call the servlet destroy() method + try { + instanceSupport.fireInstanceEvent + (InstanceEvent.BEFORE_DESTROY_EVENT, instance); + + if( Globals.IS_SECURITY_ENABLED) { + SecurityUtil.doAsPrivilege("destroy", + instance); + SecurityUtil.remove(instance); + } else { + instance.destroy(); + } + + instanceSupport.fireInstanceEvent + (InstanceEvent.AFTER_DESTROY_EVENT, instance); + + // Annotation processing + if (!((Context) getParent()).getIgnoreAnnotations()) { + ((StandardContext)getParent()).getInstanceManager().destroyInstance(instance); + } + + } catch (Throwable t) { + ExceptionUtils.handleThrowable(t); + instanceSupport.fireInstanceEvent + (InstanceEvent.AFTER_DESTROY_EVENT, instance, t); + instance = null; + instancePool = null; + nInstances = 0; + fireContainerEvent("unload", this); + unloading = false; + throw new ServletException + (sm.getString("standardWrapper.destroyException", getName()), + t); + } finally { + // Write captured output + if (swallowOutput) { + String log = SystemLogHandler.stopCapture(); + if (log != null && log.length() > 0) { + if (getServletContext() != null) { + getServletContext().log(log); + } else { + out.println(log); + } } } } Modified: tomcat/trunk/test/org/apache/catalina/core/TestStandardContext.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestStandardContext.java?rev=1136179&r1=1136178&r2=1136179&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/core/TestStandardContext.java (original) +++ tomcat/trunk/test/org/apache/catalina/core/TestStandardContext.java Wed Jun 15 20:32:14 2011 @@ -321,7 +321,16 @@ public class TestStandardContext extends } - public void testBug51376() throws Exception { + public void testBug51376a() throws Exception { + doTestBug51376(false); + } + + public void testBug51376b() throws Exception { + doTestBug51376(true); + } + + private void doTestBug51376(boolean loadOnStartUp) throws Exception { + // Set up a container Tomcat tomcat = getTomcatInstance(); @@ -330,7 +339,7 @@ public class TestStandardContext extends Context ctx = tomcat.addContext("", docBase.getAbsolutePath()); // Add ServletContainerInitializer - Bug51376SCI sci = new Bug51376SCI(); + Bug51376SCI sci = new Bug51376SCI(loadOnStartUp); ctx.addServletContainerInitializer(sci, null); // Start the context @@ -347,6 +356,11 @@ public class TestStandardContext extends implements ServletContainerInitializer { private Bug51376Servlet s = null; + private boolean loadOnStartUp; + + public Bug51376SCI(boolean loadOnStartUp) { + this.loadOnStartUp = loadOnStartUp; + } private Bug51376Servlet getServlet() { return s; @@ -359,7 +373,9 @@ public class TestStandardContext extends s = new Bug51376Servlet(); ServletRegistration.Dynamic sr = ctx.addServlet("bug51376", s); sr.addMapping("/bug51376"); - sr.setLoadOnStartup(1); + if (loadOnStartUp) { + sr.setLoadOnStartup(1); + } } } @@ -399,6 +415,8 @@ public class TestStandardContext extends if (initOk != null && initOk.booleanValue() && destoryOk != null && destoryOk.booleanValue()) { return true; + } else if (initOk == null && destoryOk == null) { + return true; } else { return false; } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org