Fix bug with activation count for auto-restart services

This could result in shutdown failure. Also, when stopping such a
process via dinitctl, the process would always restart if it had any
dependents (including soft); fix that.

diff --git a/src/proc-service.cc b/src/proc-service.cc
index c761140176617ff8cdbf746e659df38f0b5932ea..5d23c2fcc32147723e138e8929463462802449d1 100644 (file)
--- a/src/proc-service.cc
+++ b/src/proc-service.cc
@@ -281,7 +281,6 @@ void bgproc_service::handle_exit_status(bp_sys::exit_status exit_status) noexcep
     // This may be a "smooth recovery" where we are restarting the process while leaving the
     // service in the STARTED state.
     if (restarting && service_state == service_state_t::STARTED) {
-        //restarting = false;
         bool need_stop = false;
         if ((did_exit && exit_status.get_exit_status() != 0) || was_signalled) {
             need_stop = true;
@@ -313,7 +312,6 @@ void bgproc_service::handle_exit_status(bp_sys::exit_status exit_status) noexcep
         return;
     }
 
-    //restarting = false;
     if (service_state == service_state_t::STARTING) {
         // POSIX requires that if the process exited clearly with a status code of 0,
         // the exit status value will be 0:

diff --git a/src/service.cc b/src/service.cc
index 18987c669ba69cc5e811bc55930f91d3cff1a7e0..3ea862bb1f62254f754067825982d4bf79e2a981 100644 (file)
--- a/src/service.cc
+++ b/src/service.cc
@@ -50,8 +50,10 @@ void service_record::stopped() noexcept
 
     force_stop = false;
 
+    // If we are to re-start, restarting should have been set true and desired_state should be STARTED.
+    // (A restart could be cancelled via a separately issued stop, including via a shutdown).
     restarting |= auto_restart;
-    bool will_restart = restarting && required_by > 0;
+    bool will_restart = restarting && desired_state == service_state_t::STARTED;
     if (restarting && ! will_restart) {
         notify_listeners(service_event_t::STARTCANCELLED);
     }
@@ -493,6 +495,8 @@ void service_record::stop(bool bring_down) noexcept
         bring_down = true;
     }
 
+    // Set desired state to STOPPED, this will be set back to STARTED if there any hard dependents
+    // that want to restart.
     desired_state = service_state_t::STOPPED;
 
     if (bring_down && service_state != service_state_t::STOPPED

diff --git a/src/tests/proctests.cc b/src/tests/proctests.cc
index 395ec3e05e0365f04a132d3657a4d29d87f4b43c..b75eea344e5fae6ccba9e2b851acef9ddaf70a87 100644 (file)
--- a/src/tests/proctests.cc
+++ b/src/tests/proctests.cc
@@ -251,6 +251,7 @@ void test_proc_term_restart2()
     assert(p.get_state() == service_state_t::STARTED);
     assert(event_loop.active_timers.size() == 0);
 
+    // simulate process terminating, should then be restarted:
     base_process_service_test::handle_exit(&p, 0);
     sset.process_queues();
 
@@ -268,11 +269,32 @@ void test_proc_term_restart2()
     assert(p.get_state() == service_state_t::STARTED);
     assert(event_loop.active_timers.size() == 0);
 
+    assert(sset.count_active_services() == 2);
+
+    // Request stop, this time it should not restart:
+    p.stop(true);
+    sset.process_queues();
+    base_process_service_test::handle_exit(&p, 0);
+    sset.process_queues();
+
+    assert(p.get_state() == service_state_t::STOPPED);
+    assert(event_loop.active_timers.size() == 0);
+    assert(sset.count_active_services() == 1);
+
+    // simulate terminate dinit
+    sset.stop_all_services();
+
+    //base_process_service_test::handle_exit(&p, 0);
+    sset.process_queues();
+
+    assert(p.get_state() == service_state_t::STOPPED);
+    assert(event_loop.active_timers.size() == 0);
+    assert(sset.count_active_services() == 0);
+
     sset.remove_service(&p);
     sset.remove_service(&b);
 }
 
-
 // Termination via stop request
 void test_term_via_stop()
 {