Author: markt Date: Wed Feb 9 19:46:49 2011 New Revision: 1069056 URL: http://svn.apache.org/viewvc?rev=1069056&view=rev Log: Add further checks that LifecycleBase sub-classes are correctly changing state.
Modified: tomcat/trunk/java/org/apache/catalina/connector/MapperListener.java tomcat/trunk/java/org/apache/catalina/util/LifecycleBase.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/catalina/connector/MapperListener.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/MapperListener.java?rev=1069056&r1=1069055&r2=1069056&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/MapperListener.java (original) +++ tomcat/trunk/java/org/apache/catalina/connector/MapperListener.java Wed Feb 9 19:46:49 2011 @@ -24,6 +24,7 @@ import org.apache.catalina.Engine; import org.apache.catalina.Host; import org.apache.catalina.Lifecycle; import org.apache.catalina.LifecycleEvent; +import org.apache.catalina.LifecycleException; import org.apache.catalina.LifecycleListener; import org.apache.catalina.LifecycleState; import org.apache.catalina.Wrapper; @@ -92,7 +93,7 @@ public class MapperListener extends Life // ------------------------------------------------------- Lifecycle Methods @Override - public void startInternal() { + public void startInternal() throws LifecycleException { setState(LifecycleState.STARTING); @@ -116,7 +117,7 @@ public class MapperListener extends Life @Override - public void stopInternal() { + public void stopInternal() throws LifecycleException { setState(LifecycleState.STOPPING); } Modified: tomcat/trunk/java/org/apache/catalina/util/LifecycleBase.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/LifecycleBase.java?rev=1069056&r1=1069055&r2=1069056&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/util/LifecycleBase.java (original) +++ tomcat/trunk/java/org/apache/catalina/util/LifecycleBase.java Wed Feb 9 19:46:49 2011 @@ -95,16 +95,16 @@ public abstract class LifecycleBase impl if (!state.equals(LifecycleState.NEW)) { invalidTransition(Lifecycle.BEFORE_INIT_EVENT); } - setState(LifecycleState.INITIALIZING); + setStateInternal(LifecycleState.INITIALIZING, null, false); try { initInternal(); } catch (LifecycleException e) { - setState(LifecycleState.FAILED); + setStateInternal(LifecycleState.FAILED, null, false); throw e; } - setState(LifecycleState.INITIALIZED); + setStateInternal(LifecycleState.INITIALIZED, null, false); } @@ -139,12 +139,12 @@ public abstract class LifecycleBase impl invalidTransition(Lifecycle.BEFORE_START_EVENT); } - setState(LifecycleState.STARTING_PREP); + setStateInternal(LifecycleState.STARTING_PREP, null, false); try { startInternal(); } catch (LifecycleException e) { - setState(LifecycleState.FAILED); + setStateInternal(LifecycleState.FAILED, null, false); throw e; } @@ -158,7 +158,7 @@ public abstract class LifecycleBase impl invalidTransition(Lifecycle.AFTER_START_EVENT); } - setState(LifecycleState.STARTED); + setStateInternal(LifecycleState.STARTED, null, false); } } @@ -212,18 +212,18 @@ public abstract class LifecycleBase impl invalidTransition(Lifecycle.BEFORE_STOP_EVENT); } - setState(LifecycleState.STOPPING_PREP); + setStateInternal(LifecycleState.STOPPING_PREP, null, false); try { stopInternal(); } catch (LifecycleException e) { - setState(LifecycleState.FAILED); + setStateInternal(LifecycleState.FAILED, null, false); throw e; } if (state.equals(LifecycleState.MUST_DESTROY)) { // Complete stop process first - setState(LifecycleState.STOPPED); + setStateInternal(LifecycleState.STOPPED, null, false); destroy(); } else { @@ -233,7 +233,7 @@ public abstract class LifecycleBase impl invalidTransition(Lifecycle.AFTER_STOP_EVENT); } - setState(LifecycleState.STOPPED); + setStateInternal(LifecycleState.STOPPED, null, false); } } @@ -271,16 +271,16 @@ public abstract class LifecycleBase impl invalidTransition(Lifecycle.BEFORE_DESTROY_EVENT); } - setState(LifecycleState.DESTROYING); + setStateInternal(LifecycleState.DESTROYING, null, false); try { destroyInternal(); } catch (LifecycleException e) { - setState(LifecycleState.FAILED); + setStateInternal(LifecycleState.FAILED, null, false); throw e; } - setState(LifecycleState.DESTROYED); + setStateInternal(LifecycleState.DESTROYED, null, false); } @@ -307,28 +307,62 @@ public abstract class LifecycleBase impl /** * Provides a mechanism for sub-classes to update the component state. * Calling this method will automatically fire any associated - * {@link Lifecycle} event. + * {@link Lifecycle} event. It will also check that any attempted state + * transition is valid for a sub-class. * * @param state The new state for this component */ - protected synchronized void setState(LifecycleState state) { - setState(state, null); + protected synchronized void setState(LifecycleState state) + throws LifecycleException { + setStateInternal(state, null, true); } /** * Provides a mechanism for sub-classes to update the component state. * Calling this method will automatically fire any associated - * {@link Lifecycle} event. + * {@link Lifecycle} event. It will also check that any attempted state + * transition is valid for a sub-class. * * @param state The new state for this component * @param data The data to pass to the associated {@link Lifecycle} event */ - protected synchronized void setState(LifecycleState state, Object data) { + protected synchronized void setState(LifecycleState state, Object data) + throws LifecycleException { + setStateInternal(state, data, true); + } + + private synchronized void setStateInternal(LifecycleState state, + Object data, boolean check) throws LifecycleException { if (log.isDebugEnabled()) { log.debug(sm.getString("lifecycleBase.setState", this, state)); } + + if (check) { + // Must have been triggered by one of the abstract methods (assume + // code in this class is correct) + // null is never a valid state + if (state == null) { + invalidTransition("null"); + // Unreachable code - here to stop eclipse complaining about + // a possible NPE further down the method + return; + } + + // Any method can transition to failed + // startInternal() permits STARTING_PREP to STARTING + // stopInternal() permits STOPPING_PREP to STOPPING + if (!(state == LifecycleState.FAILED || + (this.state == LifecycleState.STARTING_PREP && + state == LifecycleState.STARTING) || + (this.state == LifecycleState.STOPPING_PREP && + state == LifecycleState.STOPPING))) { + // No other transition permitted + invalidTransition(state.name()); + } + } + this.state = state; String lifecycleEvent = state.getLifecycleEvent(); if (lifecycleEvent != null) { @@ -336,7 +370,6 @@ public abstract class LifecycleBase impl } } - private void invalidTransition(String type) throws LifecycleException { String msg = sm.getString("lifecycleBase.invalidTransition", type, toString(), state); Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1069056&r1=1069055&r2=1069056&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Wed Feb 9 19:46:49 2011 @@ -39,7 +39,8 @@ <body> <!-- - General, Catalina, Coyote, Jasper, Cluster, Web applications, Extras, Other + General, Catalina, Coyote, Jasper, Cluster, Web applications, Extras, Tribes, + Other --> <section name="Tomcat 7.0.9 (markt)"> <subsection name="Catalina"> @@ -57,6 +58,11 @@ Remove ServerLifecycleListener. This was already removed from server.xml and with the Lifecycle re-factoring is no longer required. (markt) </update> + <add> + Add additional checks to ensure that sub-classes of + <code>org.apache.catalina.LifecycleBase</code> correctly implement the + expected state transitions. (markt) + </add> </changelog> </subsection> <subsection name="Tribes"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org