Largish re-work of how dependencies are handled. The "start" and

"dependentStopped" methods were overloaded in unsound ways.

Add require() and release() methods to make an explicit requirement
on a dependency. A require() is added when the service is explictly
started, or when it is a dependency of such a service (directly),
or when it is a dependency of a service which has been required.

diff --git a/src/service.cc b/src/service.cc
index 0e90b68e811e7268522bdb3954516249c7943b8a..182467dc498eaffde9388c846307994221bb35b5 100644 (file)
--- a/src/service.cc
+++ b/src/service.cc
@@ -56,21 +56,6 @@ void ServiceSet::stopService(const std::string & name) noexcept
     }
 }
 
-void ServiceRecord::notify_dependencies_stopped() noexcept
-{
-    if (desired_state == ServiceState::STOPPED) {
-        // Stop any dependencies whose desired state is STOPPED:
-        for (auto i = depends_on.begin(); i != depends_on.end(); i++) {
-            (*i)->dependentStopped();
-        }
-        for (auto i = soft_deps.begin(); i != soft_deps.end(); i++) {
-            i->getTo()->dependentStopped();
-        }
-        
-        service_set->service_inactive(this);
-    }
-}
-
 // Called when a service has actually stopped.
 void ServiceRecord::stopped() noexcept
 {
@@ -85,6 +70,10 @@ void ServiceRecord::stopped() noexcept
     
     notifyListeners(ServiceEvent::STOPPED);
     
+    for (auto dependency : depends_on) {
+        dependency->dependentStopped();
+    }
+    
     if (desired_state == ServiceState::STARTED) {
         // Desired state is "started".
         do_start();
@@ -95,7 +84,7 @@ void ServiceRecord::stopped() noexcept
             socket_fd = -1;
         }
         
-        notify_dependencies_stopped();
+        release_dependencies();
     }
 }
 
@@ -241,6 +230,49 @@ void ServiceRecord::process_child_status(struct ev_loop *loop, ev_io * stat_io,
     }
 }
 
+void ServiceRecord::require() noexcept
+{
+    if (required_by++ == 0) {
+        // Need to require all our dependencies
+        for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) {
+            (*i)->require();
+        }
+
+        for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) {
+            ServiceRecord * to = i->getTo();
+            to->require();
+        }
+    }
+}
+
+void ServiceRecord::release() noexcept
+{
+    if (--required_by == 0) {
+        desired_state = ServiceState::STOPPED;
+        // Can stop, and release dependencies once we're stopped.
+        if (service_state == ServiceState::STOPPED) {
+            release_dependencies();
+        }
+        else {
+            do_stop();
+        }
+    }
+}
+
+void ServiceRecord::release_dependencies() noexcept
+{
+    for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) {
+        (*i)->release();
+    }
+
+    for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) {
+        ServiceRecord * to = i->getTo();
+        to->release();
+    }
+    
+    service_set->service_inactive(this);
+}
+
 void ServiceRecord::start(bool transitive) noexcept
 {
     if ((service_state == ServiceState::STARTING || service_state == ServiceState::STARTED)
@@ -250,10 +282,8 @@ void ServiceRecord::start(bool transitive) noexcept
         notifyListeners(ServiceEvent::STOPCANCELLED);
     }
 
-    if (transitive) {
-        started_deps++;
-    }
-    else {
+    if (!transitive) {
+        if (!start_explicit) require();
         start_explicit = true;
     }
     
@@ -531,8 +561,6 @@ void ServiceRecord::failed_to_start(bool depfailed) noexcept
     service_state = ServiceState::STOPPED;
     notifyListeners(ServiceEvent::FAILEDSTART);
     
-    notify_dependencies_stopped();
-
     // Cancel start of dependents:
     for (sr_iter i = dependents.begin(); i != dependents.end(); i++) {
         if ((*i)->service_state == ServiceState::STARTING) {
@@ -670,58 +698,28 @@ void ServiceRecord::forceStop() noexcept
 {
     if (service_state != ServiceState::STOPPED) {
         force_stop = true;
-        for (sr_iter i = dependents.begin(); i != dependents.end(); i++) {
-            (*i)->forceStop();
-        }
-        
-        if (service_state == ServiceState::STARTED) {
-            do_stop();
-        }
-        
-        // We don't want to force stop soft dependencies, however.
+        do_stop();
     }
 }
 
 void ServiceRecord::dependentStopped() noexcept
 {
-    started_deps--;
-    if (service_state == ServiceState::STOPPING) {
+    if (service_state == ServiceState::STOPPING && waiting_for_deps) {
         // Check the other dependents before we stop.
         if (stopCheckDependents()) {
             allDepsStopped();
         }
     }
-    else if (started_deps == 0 && !start_explicit) {
-        desired_state = ServiceState::STOPPED;
-        service_state = ServiceState::STOPPING;
-        allDepsStopped();
-    }
 }
 
 void ServiceRecord::stop() noexcept
 {
     if (desired_state == ServiceState::STOPPED && service_state != ServiceState::STARTED) return;
     
-    start_explicit = false;
-    if (started_deps != 0) {
-        return;
-    }
-
-    if ((service_state == ServiceState::STOPPING || service_state == ServiceState::STOPPED)
-            && desired_state == ServiceState::STARTED) {
-        // The service *was* stopped/stopping, but it was going to restart.
-        // Now, we'll cancel the restart.
-        notifyListeners(ServiceEvent::STARTCANCELLED);
-    }
-    
-    desired_state = ServiceState::STOPPED;
-
-    if (service_state == ServiceState::STOPPED) {
-        notify_dependencies_stopped();
-        return;
+    if (start_explicit) {
+        start_explicit = false;
+        release();
     }
-    
-    do_stop();
 }
 
 void ServiceRecord::do_stop() noexcept
@@ -758,7 +756,7 @@ void ServiceRecord::do_stop() noexcept
     waiting_for_deps = true;
     
     // If we get here, we are in STARTED state; stop all dependents.
-    if (stopCheckDependents()) {
+    if (stopDependents()) {
         allDepsStopped();
     }
 }
@@ -767,7 +765,7 @@ bool ServiceRecord::stopCheckDependents() noexcept
 {
     bool all_deps_stopped = true;
     for (sr_iter i = dependents.begin(); i != dependents.end(); ++i) {
-        if ((*i)->service_state != ServiceState::STOPPED) {
+        if (! (*i)->is_stopped()) {
             all_deps_stopped = false;
             break;
         }
@@ -780,9 +778,14 @@ bool ServiceRecord::stopDependents() noexcept
 {
     bool all_deps_stopped = true;
     for (sr_iter i = dependents.begin(); i != dependents.end(); ++i) {
-        if ((*i)->service_state != ServiceState::STOPPED) {
+        if (! (*i)->is_stopped()) {
             all_deps_stopped = false;
-            (*i)->stop();
+            if (force_stop) {
+                (*i)->forceStop();
+            }
+            else {
+                (*i)->do_stop();
+            }
         }
     }
     

diff --git a/src/service.h b/src/service.h
index ffd28898d70e8440391dd58af4605844e4882c99..7c39395f6d26655197981d0bd451c434dbe26a1c 100644 (file)
--- a/src/service.h
+++ b/src/service.h
@@ -178,7 +178,7 @@ class ServiceRecord
     bool doing_recovery : 1;    // if we are currently recovering a BGPROCESS (restarting process, while
                                 //   holding STARTED service state)
     bool start_explicit : 1;    // whether we are are explictly required to be started
-    int started_deps = 0;       // number of dependents that require this service to be started
+    int required_by = 0;        // number of dependents wanting this service to be started
 
     typedef std::list<ServiceRecord *> sr_list;
     typedef sr_list::iterator sr_iter;
@@ -255,6 +255,9 @@ class ServiceRecord
     
     void handle_exit_status() noexcept;
     
+    void do_start() noexcept;
+    void do_stop() noexcept;
+    
     // A dependency has reached STARTED state
     void dependencyStarted() noexcept;
     
@@ -295,7 +298,16 @@ class ServiceRecord
     // issue a stop to all dependents, return true if they are all already stopped
     bool stopDependents() noexcept;
     
-    void forceStop() noexcept; // force-stop this service and all dependents
+    void require() noexcept;
+    void release() noexcept;
+    void release_dependencies() noexcept;
+    
+    // Check if service is, fundamentally, stopped.
+    bool is_stopped() noexcept
+    {
+        return service_state == ServiceState::STOPPED
+            || (service_state == ServiceState::STARTING && waiting_for_deps);
+    }
     
     void notifyListeners(ServiceEvent event) noexcept
     {
@@ -313,11 +325,6 @@ class ServiceRecord
     // Release console (console must be currently held by this service)
     void releaseConsole() noexcept;
     
-    bool get_start_flag(bool transitive)
-    {
-        return transitive ? started_deps != 0 : start_explicit;
-    }
-    
     public:
 
     ServiceRecord(ServiceSet *set, string name)
@@ -424,8 +431,8 @@ class ServiceRecord
     
     void start(bool transitive = false) noexcept;  // start the service
     void stop() noexcept;   // stop the service
-    void do_start() noexcept;
-    void do_stop() noexcept;
+    
+    void forceStop() noexcept; // force-stop this service and all dependents
     
     void pinStart() noexcept;  // start the service and pin it
     void pinStop() noexcept;   // stop the service and pin it