From 53f706a8a4fcce89c2972da82e77ba38681e3b40 Mon Sep 17 00:00:00 2001 From: markt Date: Sun, 27 Feb 2011 11:00:12 +0000 Subject: [PATCH] Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50836 Better documentation of the meaning of Lifecycle.isAvailable() and correct a couple of cases where this could incorrectly return true. git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1075022 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/catalina/Lifecycle.java | 46 ++++++++++++++++-------- java/org/apache/catalina/LifecycleListener.java | 3 +- java/org/apache/catalina/LifecycleState.java | 12 +++++-- java/org/apache/catalina/util/LifecycleBase.java | 14 ++++++-- webapps/docs/changelog.xml | 5 +++ 5 files changed, 60 insertions(+), 20 deletions(-) diff --git a/java/org/apache/catalina/Lifecycle.java b/java/org/apache/catalina/Lifecycle.java index e141e6d5f..d1e3cde2c 100644 --- a/java/org/apache/catalina/Lifecycle.java +++ b/java/org/apache/catalina/Lifecycle.java @@ -25,7 +25,8 @@ package org.apache.catalina; * the functionality they support) in order to provide a consistent mechanism * to start and stop the component. *
- * The valid state transitions for components that support Lifecycle are: + * The valid state transitions for components that support {@link Lifecycle} + * are: * 
  *    init()
  * NEW ->-- INITIALIZING
@@ -44,9 +45,11 @@ package org.apache.catalina;
  * |  |          |                                                             |
  * |  |         \|/            auto                 auto              start()  |
  * |  |     STOPPING_PREP ------>----- STOPPING ------>----- STOPPED ---->------
- * |  |          ^                                           |  |  ^
+ * |  |                                   ^                  |  |  ^
+ * |  |                  stop()           |                  |  |  |
+ * |  |          --------------------------                  |  |  |
  * |  |          |                                  auto     |  |  |
- * |  |          |stop()            MUST_DESTROY------<-------  |  |
+ * |  |          |                  MUST_DESTROY------<-------  |  |
  * |  |          |                    |                         |  |
  * |  |          |                    |auto                     |  |
  * |  |          |    destroy()      \|/              destroy() |  |
@@ -197,8 +200,8 @@ public interface Lifecycle {
 
 
     /**
-     * Get the lifecycle listeners associated with this lifecycle. If this 
-     * Lifecycle has no listeners registered, a zero-length array is returned.
+     * Get the life cycle listeners associated with this life cycle. If this 
+     * component has no listeners registered, a zero-length array is returned.
      */
     public LifecycleListener[] findLifecycleListeners();
 
@@ -226,10 +229,12 @@ public interface Lifecycle {
     public void init() throws LifecycleException;
 
     /**
-     * Prepare for the beginning of active use of the public methods of this
-     * component.  This method should be called before any of the public
-     * methods of this component are utilized. The following
-     * {@link LifecycleEvent}s will be fired in the following order:
+     * Prepare for the beginning of active use of the public methods other than
+     * property getters/setters and life cycle methods of this component. This
+     * method should be called before any of the public methods other than
+     * property getters/setters and life cycle methods of this component are
+     * utilized. The following {@link LifecycleEvent}s will be fired in the
+     * following order:
      * 
    
      *   
  1. BEFORE_START_EVENT: At the beginning of the method. It is as this
      *                           point the state transitions to
@@ -237,7 +242,9 @@ public interface Lifecycle {
      *   
  2. START_EVENT: During the method once it is safe to call start() for
      *                    any child components. It is at this point that the
      *                    state transitions to {@link LifecycleState#STARTING}
-     *                    and that the public methods may be used.
    3. 
+     *                    and that the public methods other than property
+     *                    getters/setters and life cycle methods may be 
+     *                    used.
      *   
  3. AFTER_START_EVENT: At the end of the method, immediately before it
      *                          returns. It is at this point that the state
      *                          transitions to {@link LifecycleState#STARTED}.
@@ -251,10 +258,11 @@ public interface Lifecycle {
 
 
     /**
-     * Gracefully terminate the active use of the public methods of this
-     * component. Once the STOP_EVENT is fired, the public methods should not
-     * be used. The following {@link LifecycleEvent}s will be fired in the
-     * following order:
+     * Gracefully terminate the active use of the public methods other than
+     * property getters/setters and life cycle methods of this component. Once
+     * the STOP_EVENT is fired, the public methods other than property
+     * getters/setters and life cycle methods should not be used. The following
+     * {@link LifecycleEvent}s will be fired in the following order:
      * 
      
      *   
    1. BEFORE_STOP_EVENT: At the beginning of the method. It is at this
      *                          point that the state transitions to
@@ -262,13 +270,21 @@ public interface Lifecycle {
      *   
    2. STOP_EVENT: During the method once it is safe to call stop() for
      *                   any child components. It is at this point that the
      *                   state transitions to {@link LifecycleState#STOPPING}
-     *                   and that the public methods may no longer be used.
      3. 
+     *                   and that the public methods other than property
+     *                   getters/setters and life cycle methods may no longer be
+     *                   used.
      *   
    3. AFTER_STOP_EVENT: At the end of the method, immediately before it
      *                         returns. It is at this point that the state
      *                         transitions to {@link LifecycleState#STOPPED}.
      *                         
      4. 
      * 
    
      * 
+     * Note that if transitioning from {@link LifecycleState#FAILED} then the
+     * three events above will be fired but the component will transition
+     * directly from {@link LifecycleState#FAILED} to
+     * {@link LifecycleState#STOPPING}, bypassing
+     * {@link LifecycleState#STOPPING_PREP}
+     * 
      * @exception LifecycleException if this component detects a fatal error
      *  that needs to be reported
      */
diff --git a/java/org/apache/catalina/LifecycleListener.java b/java/org/apache/catalina/LifecycleListener.java
index 741bf5b34..4f75f049b 100644
--- a/java/org/apache/catalina/LifecycleListener.java
+++ b/java/org/apache/catalina/LifecycleListener.java
@@ -22,7 +22,8 @@ package org.apache.catalina;
 /**
  * Interface defining a listener for significant events (including "component
  * start" and "component stop" generated by a component that implements the
- * Lifecycle interface.
+ * Lifecycle interface. The listener will be fired after the associated state
+ * change has taken place.
  *
  * @author Craig R. McClanahan
  * @version $Id$
diff --git a/java/org/apache/catalina/LifecycleState.java b/java/org/apache/catalina/LifecycleState.java
index 3ddd48dec..46c80fc86 100644
--- a/java/org/apache/catalina/LifecycleState.java
+++ b/java/org/apache/catalina/LifecycleState.java
@@ -35,7 +35,7 @@ public enum LifecycleState {
     DESTROYED(false, Lifecycle.AFTER_DESTROY_EVENT),
     FAILED(false, null),
     MUST_STOP(true, null),
-    MUST_DESTROY(true, null);
+    MUST_DESTROY(false, null);
     
     private final boolean available;
     private final String lifecycleEvent;
@@ -46,7 +46,15 @@ public enum LifecycleState {
     }
     
     /**
-     * Is a component in this state available for use?
+     * May the public methods other than property getters/setters and lifecycle
+     * methods be called for a component in this state? It returns
+     * true for any component in any of the following states:
+     * 
      
+     * 
    • {@link #STARTING}
      • 
+     * 
    • {@link #STARTED}
      • 
+     * 
    • {@link #STOPPING_PREP}
      • 
+     * 
    • {@link #MUST_STOP}
      • 
+     * 
    
      */
     public boolean isAvailable() {
         return available;
diff --git a/java/org/apache/catalina/util/LifecycleBase.java b/java/org/apache/catalina/util/LifecycleBase.java
index efdc593c8..66b6be268 100644
--- a/java/org/apache/catalina/util/LifecycleBase.java
+++ b/java/org/apache/catalina/util/LifecycleBase.java
@@ -212,7 +212,14 @@ public abstract class LifecycleBase implements Lifecycle {
             invalidTransition(Lifecycle.BEFORE_STOP_EVENT);
         }
         
-        setStateInternal(LifecycleState.STOPPING_PREP, null, false);
+        if (state.equals(LifecycleState.FAILED)) {
+            // Don't transition to STOPPING_PREP as that would briefly mark the
+            // component as available but do ensure the BEFORE_STOP_EVENT is
+            // fired
+            fireLifecycleEvent(BEFORE_STOP_EVENT, null);
+        } else {
+            setStateInternal(LifecycleState.STOPPING_PREP, null, false);
+        }
 
         try {
             stopInternal();
@@ -352,11 +359,14 @@ public abstract class LifecycleBase implements Lifecycle {
             
             // Any method can transition to failed
             // startInternal() permits STARTING_PREP to STARTING
-            // stopInternal() permits STOPPING_PREP to STOPPING
+            // stopInternal() permits STOPPING_PREP to STOPPING and FAILED to
+            // STOPPING
             if (!(state == LifecycleState.FAILED ||
                     (this.state == LifecycleState.STARTING_PREP &&
                             state == LifecycleState.STARTING) ||
                     (this.state == LifecycleState.STOPPING_PREP &&
+                            state == LifecycleState.STOPPING) ||
+                    (this.state == LifecycleState.FAILED &&
                             state == LifecycleState.STOPPING))) {
                 // No other transition permitted
                 invalidTransition(state.name());
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 250ef0f11..b255c3725 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -155,6 +155,11 @@
         Ensure Servlet 2.2 jspFile elements are correctly converted to use a
         leading '/' if missing. (markt)
       
+      
+        50836: Better documentation of the meaning of
+        Lifecycle.isAvailable() and correct a couple of cases where
+        this could incorrectly return true. (markt)
+      
     
   
   
-- 
2.11.0