Improve the state model.

A service now transitions into the STARTING state from the STOPPED state
as soon as start() is called, rather than after all dependencies have
started. Similarly a service enteres the STOPPING state (from STARTED)
immediately when stop() is called.

Forced re-starts should now work properly in all cases.

diff --git a/load_service.cc b/load_service.cc
index 7bb52afec34a4b5d94951c4c1ff3a7dde6f2d20c..fcfcab85217619c07c4277125d62cad9a66236be 100644 (file)
--- a/load_service.cc
+++ b/load_service.cc
@@ -200,7 +200,7 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name)
     std::list<ServiceRecord *> depends_on;
     std::list<ServiceRecord *> depends_soft;
     string logfile;
-    OnstartFlags onstart_flags = {false, false};
+    OnstartFlags onstart_flags;
     
     string line;
     bool auto_restart = false;

diff --git a/service.cc b/service.cc
index 3114f7ef608ed79e875009793b96147323eb600d..fa52aa7187ed20eaf14dbf72c11f375c25030e4e 100644 (file)
--- a/service.cc
+++ b/service.cc
@@ -130,65 +130,84 @@ void ServiceRecord::start()
         //      its cancellation
     }
 
-    auto old_desired_state = desired_state;
+    if (desired_state == ServiceState::STARTED) return;
+
     desired_state = ServiceState::STARTED;
     
-    if (service_state == ServiceState::STARTED || service_state == ServiceState::STARTING) {
-        // We couldn't be started or starting unless all dependencies have
-        // already started: so there's nothing left to do.
+    if (service_state != ServiceState::STOPPED) {
+        // We're already starting/started, or we are stopping and need to wait for
+        // that the complete.
         return;
     }
     
-    bool all_deps_started = true;
+    service_state = ServiceState::STARTING;
+    service_set->service_active(this);
 
     // Ask dependencies to start, mark them as being waited on.
-    
+    if (! startCheckDependencies(true)) {
+        return;
+    }
+
+    // Actually start this service.
+    allDepsStarted();
+}
+
+void ServiceRecord::dependencyStarted()
+{
+    if (service_state != ServiceState::STARTING) {
+        return;
+    }
+
+    if (startCheckDependencies(false)) {
+        allDepsStarted();
+    }
+}
+
+bool ServiceRecord::startCheckDependencies(bool start_deps)
+{
+    bool all_deps_started = true;
+
     for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) {
-        // Note, we cannot treat a dependency as started if its force_stop
-        // flag is set.
-        if ((*i)->service_state != ServiceState::STARTED || (*i)->force_stop) {
-            all_deps_started = false;
-            (*i)->start();
+        if ((*i)->service_state != ServiceState::STARTED) {
+            if (start_deps) {
+                all_deps_started = false;
+                (*i)->start();
+            }
+            else {
+                return false;
+            }
         }
     }
-    
-    if (old_desired_state != ServiceState::STARTED) {
-        // This is a fresh start, so we mark all soft dependencies as 'waiting on' and ask them
-        // to start:
-        for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) {
-            if (i->getTo()->service_state != ServiceState::STARTED) {
-                all_deps_started = false;
-                i->getTo()->start();
+
+    for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) {
+        ServiceRecord * to = i->getTo();
+        if (start_deps) {
+            if (to->service_state != ServiceState::STARTED) {
+                to->start();
                 i->waiting_on = true;
+                all_deps_started = false;
+            }
+            else {
+                i->waiting_on = false;
             }
         }
-    }
-    else {
-        // This is (or at least may be) a notification that a dependency is ready; let's
-        // just check them:
-        for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) {
-            ServiceRecord * to = i->getTo();
-            if (i->waiting_on) {
-                if ((to->desired_state != ServiceState::STARTED && to->service_state != ServiceState::STARTING) || to->service_state == ServiceState::STARTED) {
-                    // Service has either started or is no longer starting
-                    i->waiting_on = false;
-                }
-                else {
-                    all_deps_started = false;
-                }
+        else if (i->waiting_on) {
+            if (to->service_state != ServiceState::STARTING) {
+                // Service has either started or is no longer starting
+                i->waiting_on = false;
+            }
+            else {
+                // We are still waiting on this service
+                return false;
             }
         }
     }
-
-    if (! all_deps_started) {
-        // The dependencies will notify this service once they've started.
-        return;
-    }
-    
-    // Actually start this service.
-    service_state = ServiceState::STARTING;
-    service_set->service_active(this);
     
+    return all_deps_started;
+}
+
+void ServiceRecord::allDepsStarted()
+{
     if (service_type == ServiceType::PROCESS) {
         bool start_success = start_ps_process();
         if (start_success) {
@@ -211,11 +230,6 @@ void ServiceRecord::start()
     }
 }
 
-void ServiceRecord::dependencyStarted()
-{
-    start();
-}
-
 void ServiceRecord::started()
 {
     logServiceStarted(service_name);
@@ -230,21 +244,22 @@ void ServiceRecord::started()
         open_control_socket(ev_default_loop(EVFLAG_AUTO));
     }
 
-    if (desired_state == ServiceState::STARTED) {
-        // Start any dependents whose desired state is STARTED:
-        for (auto i = dependents.begin(); i != dependents.end(); i++) {
-            if ((*i)->desired_state == ServiceState::STARTED) {
-                (*i)->dependencyStarted();
-            }
-        }
-        for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) {
-            if ((*i)->getFrom()->desired_state == ServiceState::STARTED) {
-                (*i)->getFrom()->dependencyStarted();
-            }
+    if (force_stop || desired_state == ServiceState::STOPPED) {
+        // We must now stop.
+        stop();
+        return;
+    }
+
+    // Notify any dependents whose desired state is STARTED:
+    for (auto i = dependents.begin(); i != dependents.end(); i++) {
+        if ((*i)->desired_state == ServiceState::STARTED) {
+            (*i)->dependencyStarted();
         }
     }
-    else {
-        stop();
+    for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) {
+        if ((*i)->getFrom()->desired_state == ServiceState::STARTED) {
+            (*i)->getFrom()->dependencyStarted();
+        }
     }
 }
 
@@ -422,10 +437,10 @@ void ServiceRecord::failed_dependency()
 
 void ServiceRecord::dependentStopped()
 {
-    if (service_state != ServiceState::STOPPED && (desired_state == ServiceState::STOPPED || force_stop)) {
+    if (service_state == ServiceState::STOPPING) {
         // Check the other dependents before we stop.
         if (stopCheckDependents()) {
-            stopping();
+            allDepsStopped();
         }
     }
 }
@@ -457,9 +472,10 @@ void ServiceRecord::stop()
         return;
     }
     
+    service_state = ServiceState::STOPPING;
     // If we get here, we are in STARTED state; stop all dependents.
-    if (stopCheckDependents()) {
-        stopping();
+    if (stopDependents()) {
+        allDepsStopped();
     }
 }
 
@@ -492,10 +508,8 @@ bool ServiceRecord::stopDependents()
 
 
 // Dependency stopped or is stopping; we must stop too.
-void ServiceRecord::stopping()
+void ServiceRecord::allDepsStopped()
 {
-    service_state = ServiceState::STOPPING;
-
     if (service_type == ServiceType::PROCESS) {
         if (pid != -1) {
           // The process is still kicking on - must actually kill it.
@@ -509,7 +523,10 @@ void ServiceRecord::stopping()
     }
     else if (service_type == ServiceType::SCRIPTED) {
         // Scripted service.
-        start_ps_process(std::vector<string>(1, "stop"));
+        if (! start_ps_process(std::vector<string>(1, "stop"))) {
+            // stop script failed, but there's not much we can do:
+            stopped();
+        }
     }
     else {
         stopped();

diff --git a/service.h b/service.h
index deff596ab564425b1c5758e7de710233ef792049..4eab4b1718ed36fababba38023d474b0e0e87c19 100644 (file)
--- a/service.h
+++ b/service.h
@@ -11,18 +11,18 @@
  *
  * The total state is a combination of the two, current and desired:
  *      STOPPED/STOPPED  : stopped and will remain stopped
- *      STOPPED/STARTED  : stopped and will be started; waiting for dependencies to start.
- *      STARTING/STARTED : starting, but not yet started. All dependencies have started already.
+ *      STOPPED/STARTED  :  - (this state cannot occur)
+ *      STARTING/STARTED : starting, but not yet started. Dependencies may also be starting.
  *      STARTING/STOPPED : as above, but the service will be stopped again as soon as it has
  *                         completed startup.
  *      STARTED/STARTED  : running and will continue running.
- *      STARTED/STOPPED  : running but will stop; waiting for dependents to stop.
- *      STOPPING/STOPPED : stopping and will stop. All dependents have stopped.
+ *      STARTED/STOPPED  :  - (this state cannot occur)
+ *      STOPPING/STOPPED : stopping and will stop. Dependents may be stopping.
  *      STOPPING/STARTED : as above, but the service will be re-started again once it stops.
  *
  * A scripted service is in the STARTING/STOPPING states during the script execution.
- * A process service is in the STOPPING state when it has been signalled to stop (and is never
- *       in the STARTING state; it moves directly from STOPPED to STARTED).
+ * A process service is in the STOPPING state when it has been signalled to stop, and is
+ *       in the STARTING state when waiting for dependencies to start.
  */
 enum class ServiceState {
     STOPPED,    // service is not running.
@@ -47,6 +47,10 @@ enum class ServiceType {
 struct OnstartFlags {
     bool release_console : 1;
     bool rw_ready : 1;
+    
+    OnstartFlags() : release_console(false), rw_ready(false)
+    {
+    }
 };
 
 // Exception while loading a service
@@ -147,8 +151,8 @@ class ServiceRecord
     
     string service_name;
     ServiceType service_type;  /* ServiceType::DUMMY, PROCESS, SCRIPTED, INTERNAL */
-    ServiceState service_state; /* ServiceState::STOPPED, STARTING, STARTED, STOPPING */
-    ServiceState desired_state; /* ServiceState::STOPPED / STARTED */
+    ServiceState service_state = ServiceState::STOPPED; /* ServiceState::STOPPED, STARTING, STARTED, STOPPING */
+    ServiceState desired_state = ServiceState::STOPPED; /* ServiceState::STOPPED / STARTED */
 
     string program_name;          /* storage for program/script and arguments */
     const char **exec_arg_parts;  /* pointer to each argument/part of the program_name */
@@ -192,9 +196,8 @@ class ServiceRecord
 
     ev_child child_listener;
     
-    // Move service to STOPPING state. This can only be called once
-    // all dependents have stopped.
-    void stopping();
+    // All dependents have stopped.
+    void allDepsStopped();
     
     // Service has actually stopped (includes having all dependents
     // reaching STOPPED state).
@@ -219,6 +222,11 @@ class ServiceRecord
 
     // A dependency has reached STARTED state
     void dependencyStarted();
+    
+    void allDepsStarted();
+    
+    // Check whether dependencies have started, and optionally ask them to start
+    bool startCheckDependencies(bool do_start);
 
     // A dependent has reached STOPPED state
     void dependentStopped();