Browse Source

Propagate restart cancellation to dependents

If a restarting service is stopped (whilst in the "stopping" phase of
the restart operation) the restart is cancelled; this needs to be
propagated to dependents also.
Davin McCall 1 year ago
parent
commit
e5f4c70964
2 changed files with 96 additions and 12 deletions
  1. 16 12
      src/service.cc
  2. 80 0
      src/tests/tests.cc

+ 16 - 12
src/service.cc

@@ -566,8 +566,9 @@ void service_record::stop(bool bring_down) noexcept
         }
     }
 
-    if (bring_down && service_state != service_state_t::STOPPED
-    		&& service_state != service_state_t::STOPPING) {
+    if (bring_down && service_state != service_state_t::STOPPED) {
+        // Note even if we are already STOPPING we should call do_stop for the case where we need
+        // to interrupt any currently ongoing restart of dependents.
     	stop_reason = stopped_reason_t::NORMAL;
         do_stop();
     }
@@ -704,23 +705,26 @@ bool service_record::stop_dependents(bool for_restart, bool restart_deps) noexce
                 dep_from->forced_stop();
             }
 
-            if (dep_from->get_state() != service_state_t::STOPPED
-                    && dep_from->get_state() != service_state_t::STOPPING) {
-                dep_from->prop_stop = true;
+            if (dep_from->get_state() != service_state_t::STOPPED) {
                 if (desired_state == service_state_t::STOPPED) {
-                    // if we don't want to restart, don't restart dependent
-                    dep_from->desired_state = service_state_t::STOPPED;
-                    if (dep_from->start_explicit) {
-                        dep_from->start_explicit = false;
-                        dep_from->release(true);
+                    if (dep_from->desired_state != service_state_t::STOPPED) {
+                        // if we don't want to restart, don't restart dependent
+                        dep_from->desired_state = service_state_t::STOPPED;
+                        if (dep_from->start_explicit) {
+                            dep_from->start_explicit = false;
+                            dep_from->release(true);
+                        }
+                        dep_from->prop_stop = true;
+                        services->add_prop_queue(dep_from);
                     }
                 }
-                else if (restart_deps) {
+                else if (restart_deps && dep_from->get_state() != service_state_t::STOPPING) {
                     // restart dependent and propagate restart to all their hard dependents.
                     dep_from->stop_reason = stopped_reason_t::DEPRESTART;
                     dep_from->in_user_restart = true;
+                    dep_from->prop_stop = true;
+                    services->add_prop_queue(dep_from);
                 }
-                services->add_prop_queue(dep_from);
             }
         }
         // Note that soft dependencies are retained if restarting, but otherwise

+ 80 - 0
src/tests/tests.cc

@@ -1626,6 +1626,84 @@ void test_order3()
     assert(sset.count_active_services() == 0);
 }
 
+// Stopping a restarting service, which is in the stopping phase, should prevent restart
+void test_restart_stop1()
+{
+    service_set sset;
+
+    test_service *s1 = new test_service(&sset, "test-service-1", service_type_t::INTERNAL, {});
+    s1->auto_stop = false;
+    sset.add_service(s1);
+    assert(sset.find_service("test-service-1") == s1);
+
+    // start s1
+    sset.start_service(s1);
+    assert(s1->bring_up_reqd == true);
+    s1->started();
+    sset.process_queues();
+    assert(s1->get_state() == service_state_t::STARTED);
+
+    // issue restart
+    s1->restart();
+    sset.process_queues();
+    assert(s1->get_state() == service_state_t::STOPPING);
+
+    // now issue stop, while still in stopping phase of restart: the restart should be cancelled
+    s1->stop();
+    sset.process_queues();
+    assert(s1->get_state() == service_state_t::STOPPING);
+
+    // once stopped, the service should not restart
+    s1->stopped();
+    assert(s1->get_state() == service_state_t::STOPPED);
+}
+
+// Stopping a restarting service, which is in the stopping phase, should prevent restart
+// (this time with a dependent)
+void test_restart_stop2()
+{
+    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}});
+    s1->auto_stop = false;
+    s2->auto_stop = false;
+    sset.add_service(s1);
+    sset.add_service(s2);
+    assert(sset.find_service("test-service-1") == s1);
+    assert(sset.find_service("test-service-2") == s2);
+
+    // start s2, which also starts s1
+    sset.start_service(s2);
+    assert(s1->bring_up_reqd == true);
+    assert(s2->bring_up_reqd == false);
+    s1->started();
+    sset.process_queues();
+    assert(s1->get_state() == service_state_t::STARTED);
+    assert(s2->bring_up_reqd == true);
+    s2->started();
+    sset.process_queues();
+    assert(s2->get_state() == service_state_t::STARTED);
+
+    // issue restart
+    s1->restart();
+    sset.process_queues();
+    assert(s1->get_state() == service_state_t::STOPPING);
+    assert(s2->get_state() == service_state_t::STOPPING);
+
+    // now issue stop, while still in stopping phase of restart: the restart should be cancelled
+    s1->stop();
+    sset.process_queues();
+    assert(s1->get_state() == service_state_t::STOPPING);
+    assert(s2->get_state() == service_state_t::STOPPING);
+
+    // once stopped, the service should not restart
+    s2->stopped();
+    assert(s2->get_state() == service_state_t::STOPPED);
+    s1->stopped();
+    assert(s1->get_state() == service_state_t::STOPPED);
+}
+
 #define RUN_TEST(name, spacing) \
     std::cout << #name "..." spacing << std::flush; \
     name(); \
@@ -1673,4 +1751,6 @@ int main(int argc, char **argv)
     RUN_TEST(test_order1, "               ");
     RUN_TEST(test_order2, "               ");
     RUN_TEST(test_order3, "               ");
+    RUN_TEST(test_restart_stop1, "        ");
+    RUN_TEST(test_restart_stop2, "        ");
 }