Adjust dependency auto-break logic.

[oweals/dinit.git] / src / service.cc

diff --git a/src/service.cc b/src/service.cc
index 5195c56599c8248d0db07f695a36cc839c54a2af..2ae19150f8b04e8770ce0703395a30827ca264b2 100644 (file)
--- a/src/service.cc
+++ b/src/service.cc
@@ -26,7 +26,7 @@ static service_record * find_service(const std::list<service_record *> & records
 {
     using std::list;
     list<service_record *>::const_iterator i = records.begin();
-    for ( ; i != records.end(); i++ ) {
+    for ( ; i != records.end(); ++i ) {
         if (strcmp((*i)->get_name().c_str(), name) == 0) {
             return *i;
         }
@@ -52,7 +52,7 @@ void service_set::stop_service(const std::string & name) noexcept
 // is due to an unexpected process termination.
 void service_record::stopped() noexcept
 {
-    if (onstart_flags.runs_on_console) {
+    if (have_console) {
         bp_sys::tcsetpgrp(0, bp_sys::getpgrp());
         discard_console_log_buffer();
         release_console();
@@ -60,10 +60,11 @@ void service_record::stopped() noexcept
 
     force_stop = false;
 
-    // If we are a soft dependency of another target, break the acquisition from that target now:
+    // If we are a soft dependency of another target, break the acquisition from that target now,
+    // so that we don't re-start:
     for (auto & dependent : dependents) {
         if (dependent->dep_type != dependency_type::REGULAR) {
-            if (dependent->holding_acq) {
+            if (dependent->holding_acq  && ! dependent->waiting_on) {
                 dependent->holding_acq = false;
                 release();
             }
@@ -89,15 +90,25 @@ void service_record::stopped() noexcept
         becoming_inactive();
         
         if (start_explicit) {
+            // If we were explicitly started, our required_by count must be at least 1. Use
+            // release() to correctly release, mark inactive and release dependencies.
             start_explicit = false;
             release();
         }
         else if (required_by == 0) {
+            // This can only be the case if we didn't have start_explicit, since required_by would
+            // otherwise by non-zero.
+            prop_release = !prop_require;
+            prop_require = false;
+            services->add_prop_queue(this);
             services->service_inactive(this);
         }
     }
 
-    log_service_stopped(service_name);
+    // Start failure will have been logged already, only log if we are stopped for other reasons:
+    if (! start_failed) {
+        log_service_stopped(service_name);
+    }
     notify_listeners(service_event_t::STOPPED);
 }
 
@@ -136,6 +147,7 @@ void service_record::release(bool issue_stop) noexcept
             services->service_inactive(this);
         }
         else if (issue_stop) {
+               stop_reason = stopped_reason_t::NORMAL;
             do_stop();
         }
     }
@@ -146,8 +158,10 @@ void service_record::release_dependencies() noexcept
     for (auto & dependency : depends_on) {
         service_record * dep_to = dependency.get_to();
         if (dependency.holding_acq) {
-            dep_to->release();
+            // We must clear holding_acq before calling release, otherwise the dependency
+            // may decide to stop, check this link and release itself a second time.
             dependency.holding_acq = false;
+            dep_to->release();
         }
     }
 }
@@ -179,6 +193,8 @@ void service_record::start(bool activate) noexcept
         services->service_active(this);
     }
 
+    start_failed = false;
+    start_skipped = false;
     service_state = service_state_t::STARTING;
     waiting_for_deps = true;
 
@@ -205,6 +221,7 @@ void service_record::do_propagation() noexcept
     
     if (prop_failure) {
         prop_failure = false;
+        stop_reason = stopped_reason_t::DEPFAILED;
         failed_to_start(true);
     }
     
@@ -365,25 +382,18 @@ void service_record::started() noexcept
     }
 }
 
-void service_record::failed_to_start(bool depfailed) noexcept
+void service_record::failed_to_start(bool depfailed, bool immediate_stop) noexcept
 {
-    if (have_console) {
-        bp_sys::tcsetpgrp(0, bp_sys::getpgrp());
-        release_console();
-    }
     if (waiting_for_console) {
         services->unqueue_console(this);
         waiting_for_console = false;
     }
-    
-    log_service_failed(get_name());
-    service_state = service_state_t::STOPPED;
+
     if (start_explicit) {
         start_explicit = false;
         release(false);
     }
-    notify_listeners(service_event_t::FAILEDSTART);
-    
+
     // Cancel start of dependents:
     for (auto & dept : dependents) {
         switch (dept->dep_type) {
@@ -400,12 +410,23 @@ void service_record::failed_to_start(bool depfailed) noexcept
                 dept->waiting_on = false;
                 dept->get_from()->dependency_started();
             }
-            if (dept->holding_acq) {
-                dept->holding_acq = false;
-                release();
-            }
+        }
+
+        // Always release now, so that our desired state will be STOPPED before we call
+        // stopped() below (if we do so). Otherwise it may decide to restart us.
+        if (dept->holding_acq) {
+            dept->holding_acq = false;
+            release(false);
         }
     }
+
+    start_failed = true;
+    log_service_failed(get_name());
+    notify_listeners(service_event_t::FAILEDSTART);
+
+    if (immediate_stop) {
+        stopped();
+    }
 }
 
 bool service_record::bring_up() noexcept
@@ -441,7 +462,9 @@ void service_record::stop(bool bring_down) noexcept
         release();
     }
 
-    if (bring_down) {
+    if (bring_down && service_state != service_state_t::STOPPED
+               && service_state != service_state_t::STOPPING) {
+       stop_reason = stopped_reason_t::NORMAL;
         do_stop();
     }
 }
@@ -463,13 +486,14 @@ void service_record::do_stop() noexcept
             // we need to delegate to can_interrupt_start() (which can be overridden).
             if (! waiting_for_deps && ! waiting_for_console) {
                 if (! can_interrupt_start()) {
-                    // Well this is awkward: we're going to have to continue starting. We can stop once we've
-                    // reached the started state.
+                    // Well this is awkward: we're going to have to continue starting. We can stop once
+                    // we've reached the started state.
                     return;
                 }
 
                 if (! interrupt_start()) {
                     // Now wait for service startup to actually end; we don't need to handle it here.
+                    notify_listeners(service_event_t::STARTCANCELLED);
                     return;
                 }
             }
@@ -536,6 +560,19 @@ bool service_record::stop_dependents() noexcept
             dept->get_from()->prop_stop = true;
             services->add_prop_queue(dept->get_from());
         }
+        else {
+            // waits-for or soft dependency:
+            if (dept->waiting_on) {
+                dept->waiting_on = false;
+                dept->get_from()->dependency_started();
+            }
+            if (dept->holding_acq) {
+                dept->holding_acq = false;
+                // release without issuing stop, since we should be called only when this
+                // service is already stopped/stopping:
+                release(false);
+            }
+        }
     }
 
     return all_deps_stopped;
@@ -580,9 +617,6 @@ void service_record::release_console() noexcept
 
 bool service_record::interrupt_start() noexcept
 {
-    if (onstart_flags.starts_on_console) {
-        services->unqueue_console(this);
-    }
     return true;
 }