Various improvements to state transitions.

Introduce a new variable to track if a service is waiting for its
dependencies (starting) or dependents (stopping). In these cases
it is possible to transition directly from STARTING to STOPPED or
from STOPPING to STARTED. This also removes the need for special
handling of "internal" services (which will never transition from
STARTING to STARTED or STOPPING to STOPPED excet via transitions
of their dependencies/dependents).

diff --git a/service.cc b/service.cc
index 33dc2ab3fa2d19660b5eb9f088bf1efc1668171e..b2086f815ef25cebc206e443a0be552432d473e1 100644 (file)
--- a/service.cc
+++ b/service.cc
@@ -129,7 +129,7 @@ void ServiceRecord::process_child_callback(struct ev_loop *loop, ev_child *w, in
     }
 }
 
-void ServiceRecord::start(bool unpinned) noexcept
+void ServiceRecord::start() noexcept
 {
     if ((service_state == ServiceState::STARTING || service_state == ServiceState::STARTED)
             && desired_state == ServiceState::STOPPED) {
@@ -138,21 +138,28 @@ void ServiceRecord::start(bool unpinned) noexcept
         notifyListeners(ServiceEvent::STOPCANCELLED);
     }
 
-    if (desired_state == ServiceState::STARTED && !unpinned) return;
+    if (desired_state == ServiceState::STARTED && service_state != ServiceState::STOPPED) return;
 
     desired_state = ServiceState::STARTED;
     
+    if (pinned_stopped) return;
+    
     if (service_state != ServiceState::STOPPED) {
         // We're already starting/started, or we are stopping and need to wait for
         // that the complete.
-        return;
+        if (service_state != ServiceState::STOPPING || ! can_interrupt_stop()) {
+            return;
+        }
+        // We're STOPPING, and that can be interrupted. Our dependencies might be STOPPING,
+        // but if so they are waiting (for us), so they too can be instantly returned to
+        // STARTING state.
     }
     
-    if (pinned_stopped) return;
-    
     service_state = ServiceState::STARTING;
     service_set->service_active(this);
 
+    waiting_for_deps = true;
+
     // Ask dependencies to start, mark them as being waited on.
     if (! startCheckDependencies(true)) {
         return;
@@ -164,7 +171,7 @@ void ServiceRecord::start(bool unpinned) noexcept
 
 void ServiceRecord::dependencyStarted() noexcept
 {
-    if (service_state != ServiceState::STARTING) {
+    if (service_state != ServiceState::STARTING || ! waiting_for_deps) {
         return;
     }
 
@@ -219,9 +226,12 @@ bool ServiceRecord::startCheckDependencies(bool start_deps) noexcept
 void ServiceRecord::allDepsStarted(bool hasConsole) noexcept
 {
     if (onstart_flags.runs_on_console && ! hasConsole) {
+        waiting_for_deps = true;
         queueForConsole();
         return;
     }
+    
+    waiting_for_deps = false;
 
     if (service_type == ServiceType::PROCESS) {
         bool start_success = start_ps_process();
@@ -287,14 +297,10 @@ void ServiceRecord::started()
 
     // 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();
-        }
+        (*i)->dependencyStarted();
     }
     for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) {
-        if ((*i)->getFrom()->desired_state == ServiceState::STARTED) {
-            (*i)->getFrom()->dependencyStarted();
-        }
+        (*i)->getFrom()->dependencyStarted();
     }
 }
 
@@ -319,11 +325,9 @@ void ServiceRecord::failed_to_start()
         }
     }    
     for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) {
-        if ((*i)->getFrom()->desired_state == ServiceState::STARTED) {
-            // We can send 'start', because this is only a soft dependency.
-            // Our startup failure means that they don't have to wait for us.
-            (*i)->getFrom()->dependencyStarted();
-        }
+        // We can send 'start', because this is only a soft dependency.
+        // Our startup failure means that they don't have to wait for us.
+        (*i)->getFrom()->dependencyStarted();
     }
 }
 
@@ -460,6 +464,7 @@ void ServiceRecord::forceStop() noexcept
 }
 
 // A dependency of this service failed to start.
+// Only called when state == STARTING.
 void ServiceRecord::failed_dependency()
 {
     desired_state = ServiceState::STOPPED;
@@ -467,6 +472,7 @@ void ServiceRecord::failed_dependency()
     // Presumably, we were starting. So now we're not.
     service_state = ServiceState::STOPPED;
     service_set->service_inactive(this);
+    logServiceFailed(service_name);
     
     // Notify dependents of this service also
     for (auto i = dependents.begin(); i != dependents.end(); i++) {
@@ -475,11 +481,9 @@ void ServiceRecord::failed_dependency()
         }
     }
     for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) {
-        if ((*i)->getFrom()->service_state == ServiceState::STARTING) {
-            // It's a soft dependency, so send them 'started' rather than
-            // 'failed dep'.
-            (*i)->getFrom()->dependencyStarted();
-        }
+        // It's a soft dependency, so send them 'started' rather than
+        // 'failed dep'.
+        (*i)->getFrom()->dependencyStarted();
     }    
 }
 
@@ -493,7 +497,7 @@ void ServiceRecord::dependentStopped() noexcept
     }
 }
 
-void ServiceRecord::stop(bool unpinned) noexcept
+void ServiceRecord::stop() noexcept
 {
     if ((service_state == ServiceState::STOPPING || service_state == ServiceState::STOPPED)
             && desired_state == ServiceState::STARTED) {
@@ -510,25 +514,30 @@ void ServiceRecord::stop(bool unpinned) noexcept
 
     if (service_state != ServiceState::STARTED) {
         if (service_state == ServiceState::STARTING) {
-            // Well this is awkward: we're going to have to continue
-            // starting, but we don't want any dependents to think that
-            // they are still waiting to start.
-            // Make sure they remain stopped:
-            if (stopDependents()) {
-                if (service_type == ServiceType::INTERNAL) {
-                    // Internal services can go straight from STARTING to STOPPED.
-                    allDepsStopped();
-                    // (Other types have to finish starting first).
-                }
+            if (! can_interrupt_start()) {
+                // Well this is awkward: we're going to have to continue
+                // starting, but we don't want any dependents to think that
+                // they are still waiting to start.
+                // Make sure they remain stopped:
+                stopDependents();
+                return;
             }
+            
+            // Reaching this point, we have can_interrupt_start() == true. So,
+            // we can stop. Dependents might be starting, but they must be
+            // waiting on us, so they should also be immediately stoppable.
+            // Fall through to below.
+        }
+        else {
+            // If we're starting we need to wait for that to complete.
+            // If we're already stopping/stopped there's nothing to do.
+            return;
         }
-        
-        // If we're starting we need to wait for that to complete.
-        // If we're already stopping/stopped there's nothing to do.
-        return;
     }
     
     service_state = ServiceState::STOPPING;
+    waiting_for_deps = true;
+    
     // If we get here, we are in STARTED state; stop all dependents.
     if (stopDependents()) {
         allDepsStopped();
@@ -566,6 +575,7 @@ bool ServiceRecord::stopDependents() noexcept
 // Dependency stopped or is stopping; we must stop too.
 void ServiceRecord::allDepsStopped()
 {
+    waiting_for_deps = false;
     if (service_type == ServiceType::PROCESS) {
         if (pid != -1) {
             // The process is still kicking on - must actually kill it.
@@ -611,13 +621,13 @@ void ServiceRecord::unpin() noexcept
     if (pinned_started) {
         pinned_started = false;
         if (desired_state == ServiceState::STOPPED) {
-            stop(true);
+            stop();
         }
     }
     if (pinned_stopped) {
         pinned_stopped = false;
         if (desired_state == ServiceState::STARTED) {
-            start(true);
+            start();
         }
     }
 }

diff --git a/service.h b/service.h
index ea8314982fc5669bfc2cd3d544c2dd7ad6dce000..98e2cdd198c51af754ee1b1a3e76dda3a4ea9525 100644 (file)
--- a/service.h
+++ b/service.h
@@ -168,6 +168,8 @@ class ServiceRecord
     bool auto_restart : 1; /* whether to restart this (process) if it dies unexpectedly */
     bool pinned_stopped : 1;
     bool pinned_started : 1;
+    
+    bool waiting_for_deps : 1;  /* if STARTING, whether we are waiting for dependencies (inc console) to start */
 
     typedef std::list<ServiceRecord *> sr_list;
     typedef sr_list::iterator sr_iter;
@@ -240,6 +242,19 @@ class ServiceRecord
     
     // Check whether dependencies have started, and optionally ask them to start
     bool startCheckDependencies(bool do_start) noexcept;
+    
+    // Whether a STARTING service can immediately transition to STOPPED (as opposed to
+    // having to wait for it reach STARTED and then go through STOPPING).
+    bool can_interrupt_start() noexcept
+    {
+        return waiting_for_deps;
+    }
+    
+    // Whether a STOPPING service can immediately transition to STARTED.
+    bool can_interrupt_stop() noexcept
+    {
+        return waiting_for_deps && ! force_stop;
+    }
 
     // A dependent has reached STOPPED state
     void dependentStopped() noexcept;
@@ -352,8 +367,8 @@ class ServiceRecord
     const char *getServiceName() const noexcept { return service_name.c_str(); }
     ServiceState getState() const noexcept { return service_state; }
     
-    void start(bool unpinned = false) noexcept;  // start the service
-    void stop(bool unpinned = false) noexcept;   // stop the service
+    void start() noexcept;  // start the service
+    void stop() noexcept;   // stop the service
     
     void pinStart() noexcept;  // start the service and pin it
     void pinStop() noexcept;   // stop the service and pin it