From: Davin McCall <davmac@davmac.org>
Date: Mon, 13 Apr 2020 02:12:13 +0000 (+1000)
Subject: Fix bug with activation count for auto-restart services
X-Git-Tag: v0.8.2~9
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=ccdf8b687d3c31da98f7b9778ecd83751b8a3e96;p=oweals%2Fdinit.git

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 c761140..5d23c2f 100644
--- 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 18987c6..3ea862b 100644
--- 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 395ec3e..b75eea3 100644
--- 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()
 {