Allow restart of services if all dependents restart automatically (#108)

Allow restarting services with hard dependents (with --force)

If the user requested restart involves a pinned started dependency, the
restart will not be allowed. If any hard dependents exist, --force is
required.

If the restart is allowed then the restart is propagated to all hard
dependents and their hard dependents.

src/control.cc

@@ -202,6 +202,59 @@ bool control_conn_t::process_find_load(int pktType)
     return true;
 }
 
+static bool check_restart(service_record *service)
+{
+    if (service->is_start_pinned())
+        return false;
+    for (service_dep *dep : service->get_dependents()) {
+        if (dep->dep_type == dependency_type::REGULAR && dep->holding_acq) {
+            service_record *dep_from = dep->get_from();
+            if (! check_restart(dep_from))
+                return false;
+        }
+    }
+    return true;
+}
+
+bool control_conn_t::check_restart_dependents(service_record *service, bool gentle, bool &had_dependents)
+{
+    std::vector<char> reply_pkt;
+    size_t num_depts = 0;
+
+    for (service_dep *dep : service->get_dependents()) {
+        if (dep->dep_type == dependency_type::REGULAR && dep->holding_acq) {
+            service_record *dep_from = dep->get_from();
+            if (!gentle) {
+                // recursively check that no hard dependency is pinned started.
+                if (check_restart(dep_from))
+                    continue;
+            }
+            num_depts++;
+            // find or allocate a service handle
+            handle_t dept_handle = allocate_service_handle(dep_from);
+            if (reply_pkt.empty()) {
+                // packet type, size
+                reply_pkt.reserve(1 + sizeof(size_t) + sizeof(handle_t));
+                reply_pkt.resize(1 + sizeof(size_t));
+                reply_pkt[0] = DINIT_RP_DEPENDENTS;
+            }
+            auto old_size = reply_pkt.size();
+            reply_pkt.resize(old_size + sizeof(handle_t));
+            memcpy(reply_pkt.data() + old_size, &dept_handle, sizeof(dept_handle));
+        }
+    }
+
+    if (num_depts != 0) {
+        // There are affected dependents
+        had_dependents = true;
+        memcpy(reply_pkt.data() + 1, &num_depts, sizeof(num_depts));
+        return queue_packet(std::move(reply_pkt));
+    }
+
+    had_dependents = false;
+    return true;
+}
+
 bool control_conn_t::check_dependents(service_record *service, bool &had_dependents)
 {
     std::vector<char> reply_pkt;
@@ -288,7 +341,7 @@ bool control_conn_t::process_start_stop(int pktType)
         {
             // force service to stop
             bool do_restart = ((rbuf[1] & 4) == 4);
-            bool gentle = ((rbuf[1] & 2) == 2) || do_restart;  // restart is always "gentle"
+            bool gentle = ((rbuf[1] & 2) == 2);
             if (do_restart && services->is_shutting_down()) {
                 ack_buf[0] = DINIT_RP_SHUTTINGDOWN;
                 break;
@@ -299,7 +352,18 @@ bool control_conn_t::process_start_stop(int pktType)
                 ack_buf[0] = DINIT_RP_PINNEDSTARTED;
                 break;
             }
-            if (gentle) {
+            if (do_restart) {
+                // Check dependents; return appropriate response if any will be affected
+                bool has_dependents;
+                if (! check_restart_dependents(service, gentle, has_dependents)) {
+                    return false;
+                }
+                if (has_dependents) {
+                    // Reply packet has already been sent
+                    goto clear_out;
+                }
+            }
+            else if (gentle) {
                 // Check dependents; return appropriate response if any will be affected
                 bool has_dependents;
                 if (! check_dependents(service, has_dependents)) {

src/dinitctl.cc

@@ -1181,6 +1181,9 @@ static int service_status(int socknum, cpbuffer_t &rbuffer, const char *service_
         case service_state_t::STOPPED:
             cout << "STOPPED";
             switch (stop_reason) {
+            case stopped_reason_t::DEPRESTART:
+                cout << " (dependency restarted)";
+                break;
             case stopped_reason_t::DEPFAILED:
                 cout << " (dependency failed/terminated)";
                 break;

src/includes/control.h

@@ -181,6 +181,12 @@ class control_conn_t : private service_listener
     // Returns false if the connection must be closed, true otherwise.
     bool check_dependents(service_record *service, bool &had_dependents);
 
+    // Check if any dependents will be affected by restarting a service if gentle, otherwise check if
+    // any pinned started dependents will be affected, generate a response packet if so.
+    // had_dependents will be set true if the service should not be restarted, false otherwise.
+    // Returns false if the connection must be closed, true otherwise.
+    bool check_restart_dependents(service_record *service, bool gentle, bool &had_dependents);
+
     // Allocate a new handle for a service; may throw std::bad_alloc
     handle_t allocate_service_handle(service_record *record);
     

src/includes/service-constants.h

@@ -45,6 +45,7 @@ enum class shutdown_type_t {
 enum class stopped_reason_t
 {
     NORMAL,
+    DEPRESTART, // A hard dependency was restarted
 
     // Start failures:
     DEPFAILED, // A dependency failed to start

src/includes/service.h

@@ -273,6 +273,7 @@ class service_record
     bool start_skipped : 1;     // start was skipped by interrupt
     
     bool in_auto_restart : 1;
+    bool in_user_restart : 1;
 
     int required_by = 0;        // number of dependents wanting this service to be started
 
@@ -356,7 +357,10 @@ class service_record
     bool stop_check_dependents() noexcept;
     
     // issue a stop to all dependents, return true if they are all already stopped
-    bool stop_dependents(bool for_restart) noexcept;
+    bool stop_dependents(bool with_restart, bool for_restart) noexcept;
+
+    // issue a restart to all hard dependents
+    bool restart_dependents() noexcept;
     
     void require() noexcept;
     void release(bool issue_stop = true) noexcept;
@@ -437,7 +441,7 @@ class service_record
             waiting_for_console(false), have_console(false), waiting_for_execstat(false),
             start_explicit(false), prop_require(false), prop_release(false), prop_failure(false),
             prop_start(false), prop_stop(false), start_failed(false), start_skipped(false),
-            in_auto_restart(false), force_stop(false)
+            in_auto_restart(false), in_user_restart(false), force_stop(false)
     {
         services = set;
         record_type = service_type_t::DUMMY;

src/service.cc

@@ -245,7 +245,7 @@ void service_record::do_propagation() noexcept
 
     if (prop_stop) {
         prop_stop = false;
-        do_stop();
+        do_stop(in_user_restart);
     }
 }
 
@@ -569,6 +569,7 @@ void service_record::do_stop(bool with_restart) noexcept
     if (pinned_started) return;
 
     in_auto_restart = false;
+    in_user_restart = false;
 
     // Will we restart? desired state of STOPPED inhibits auto-restart
     bool for_restart = with_restart || (auto_restart && desired_state == service_state_t::STARTED);
@@ -581,7 +582,7 @@ void service_record::do_stop(bool with_restart) noexcept
         }
     }
 
-    bool all_deps_stopped = stop_dependents(for_restart);
+    bool all_deps_stopped = stop_dependents(with_restart, for_restart);
 
     if (service_state != service_state_t::STARTED) {
         if (service_state == service_state_t::STARTING) {
@@ -642,7 +643,7 @@ bool service_record::stop_check_dependents() noexcept
     return all_deps_stopped;
 }
 
-bool service_record::stop_dependents(bool for_restart) noexcept
+bool service_record::stop_dependents(bool with_restart, bool for_restart) noexcept
 {
     // We are in either STARTED or STARTING states.
     bool all_deps_stopped = true;
@@ -677,6 +678,12 @@ bool service_record::stop_dependents(bool for_restart) noexcept
                         dep_from->release(true);
                     }
                 }
+                else if (with_restart) {
+                    // if we restart, restart dependent and propagate restart to
+                    // all their hard dependents.
+                    dep_from->stop_reason = stopped_reason_t::DEPRESTART;
+                    dep_from->in_user_restart = true;
+                }
                 services->add_prop_queue(dep_from);
             }
         }

src/tests/tests.cc

@@ -391,6 +391,74 @@ void basic_test8()
     assert(sset.count_active_services() == 0);
 }
 
+// A hard dependent service which restarts due to a requested dependency restart should restart,
+// a soft dependent service should be left untouched.
+void basic_test9()
+{
+    service_set sset;
+
+    test_service *s1 = new test_service(&sset, "test-service-1", service_type_t::INTERNAL, {});
+    test_service *s2 = new test_service(&sset, "test-service-2", service_type_t::INTERNAL, {{s1, REG}});
+    test_service *s3 = new test_service(&sset, "test-service-3", service_type_t::INTERNAL, {{s2, REG}});
+    test_service *s4 = new test_service(&sset, "test-service-4", service_type_t::INTERNAL, {{s1, MS}});
+    sset.add_service(s1);
+    sset.add_service(s2);
+    sset.add_service(s3);
+    sset.add_service(s4);
+
+    assert(sset.find_service("test-service-1") == s1);
+    assert(sset.find_service("test-service-2") == s2);
+    assert(sset.find_service("test-service-3") == s3);
+    assert(sset.find_service("test-service-4") == s4);
+
+    // Start all four services:
+    sset.start_service(s3);
+    sset.start_service(s4);
+
+    s1->started();
+    sset.process_queues();
+    s2->started();
+    sset.process_queues();
+    s3->started();
+    sset.process_queues();
+    s4->started();
+    sset.process_queues();
+
+    assert(s4->get_state() == service_state_t::STARTED);
+    assert(s3->get_state() == service_state_t::STARTED);
+    assert(s2->get_state() == service_state_t::STARTED);
+    assert(s1->get_state() == service_state_t::STARTED);
+
+    // Now restart s1, which should also force s2 and s3 to restart.
+    // s2 (and therefore s1) should restart:
+    s1->restart();
+    sset.process_queues();
+
+    // s4 only has a milestone dependency, and should be left untouched.
+    assert(s4->get_state() == service_state_t::STARTED);
+    assert(s3->get_state() == service_state_t::STARTING);
+    assert(s2->get_state() == service_state_t::STARTING);
+    assert(s1->get_state() == service_state_t::STARTING);
+
+    assert(s4->get_target_state() == service_state_t::STARTED);
+    assert(s3->get_target_state() == service_state_t::STARTED);
+    assert(s2->get_target_state() == service_state_t::STARTED);
+    assert(s1->get_target_state() == service_state_t::STARTED);
+
+    s1->started();
+    sset.process_queues();
+    s2->started();
+    sset.process_queues();
+    s3->started();
+    sset.process_queues();
+
+    assert(s4->get_state() == service_state_t::STARTED);
+    assert(s3->get_state() == service_state_t::STARTED);
+    assert(s2->get_state() == service_state_t::STARTED);
+    assert(s1->get_state() == service_state_t::STARTED);
+    assert(sset.count_active_services() == 4);
+}
+
 // Test that service pinned in start state is not stopped when its dependency stops.
 void test_pin1()
 {
@@ -1573,6 +1641,7 @@ int main(int argc, char **argv)
     RUN_TEST(basic_test6, "               ");
     RUN_TEST(basic_test7, "               ");
     RUN_TEST(basic_test8, "               ");
+    RUN_TEST(basic_test9, "               ");
     RUN_TEST(test_pin1, "                 ");
     RUN_TEST(test_pin2, "                 ");
     RUN_TEST(test_pin3, "                 ");