Add further checks that LifecycleBase sub-classes are correctly changing state.

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

diff --git a/java/org/apache/catalina/connector/MapperListener.java b/java/org/apache/catalina/connector/MapperListener.java
index abc6f56..c461967 100644 (file)
--- a/java/org/apache/catalina/connector/MapperListener.java
+++ b/java/org/apache/catalina/connector/MapperListener.java
@@ -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 LifecycleMBeanBase
     // ------------------------------------------------------- Lifecycle Methods
 
     @Override
-    public void startInternal() {
+    public void startInternal() throws LifecycleException {
 
         setState(LifecycleState.STARTING);
 
@@ -116,7 +117,7 @@ public class MapperListener extends LifecycleMBeanBase
         
 
     @Override
-    public void stopInternal() {
+    public void stopInternal() throws LifecycleException {
         setState(LifecycleState.STOPPING);
     }
 

diff --git a/java/org/apache/catalina/util/LifecycleBase.java b/java/org/apache/catalina/util/LifecycleBase.java
index bc5f4aa..efdc593 100644 (file)
--- a/java/org/apache/catalina/util/LifecycleBase.java
+++ b/java/org/apache/catalina/util/LifecycleBase.java
@@ -95,16 +95,16 @@ public abstract class LifecycleBase implements Lifecycle {
         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 implements Lifecycle {
             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 implements Lifecycle {
                 invalidTransition(Lifecycle.AFTER_START_EVENT);
             }
             
-            setState(LifecycleState.STARTED);
+            setStateInternal(LifecycleState.STARTED, null, false);
         }
     }
 
@@ -212,18 +212,18 @@ public abstract class LifecycleBase implements Lifecycle {
             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 implements Lifecycle {
                 invalidTransition(Lifecycle.AFTER_STOP_EVENT);
             }
 
-            setState(LifecycleState.STOPPED);
+            setStateInternal(LifecycleState.STOPPED, null, false);
         }
     }
 
@@ -271,16 +271,16 @@ public abstract class LifecycleBase implements Lifecycle {
             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 implements Lifecycle {
     /**
      * 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 implements Lifecycle {
         }
     }
 
-    
     private void invalidTransition(String type) throws LifecycleException {
         String msg = sm.getString("lifecycleBase.invalidTransition", type,
                 toString(), state);

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index dbe38f5..a4cb3b4 100644 (file)
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -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">