Browse Source

Inhibit restart of dependents when service manually stopped

Previously, manually stopping a service inhibited automatic restart of
the service itself, but not of any dependents. As a result the service
could still restart (because a dependent would restart and require the
service). Since this is somewhat inconsistent, change the behaviour so
that manually stopping a service inhibits restarts of dependents also
(and thus completely inhibits restart of the specified service).
Davin McCall 3 years ago
parent
commit
0cc114468e
3 changed files with 52 additions and 6 deletions
  1. 4 6
      doc/manpages/dinitctl.8.m4
  2. 8 0
      src/service.cc
  3. 40 0
      src/tests/tests.cc

+ 4 - 6
doc/manpages/dinitctl.8.m4

@@ -99,11 +99,9 @@ to stop before it is then restarted.
 Stop the specified service, and remove explicit activation. If the service has (non-soft) dependents, an
 error will be displayed unless the \fB\-\-force\fR option is used.
 
-A service with any dependents via soft dependencies will have these dependency links broken when it stops.
-
-The \fBrestart\fR option applied to the stopped service will not by itself cause the service to restart
-when it is stopped via this command. However, a dependent which is configured to restart may
-cause the service itself to restart as a result.
+The \fBrestart\fR option (see \fBdinit-service\fR(5)) applied to the stopped service will not cause the
+service to restart when it is stopped via this command (that is, this command inhibits automatic restart).
+This also applies to dependents that must also be stopped.
 
 Any pending \fBstart\fR orders are cancelled,
 though a service which is starting will continue its startup before then stopping (unless the service is
@@ -210,7 +208,7 @@ automatically, or with a dependent service configured to do so, will restart imm
 unless pinned.
 .\"
 .SH SEE ALSO
-\fBdinit\fR(8).
+\fBdinit\fR(8), \fBdinit-service\fR(5).
 .\"
 .SH AUTHOR
 Dinit, and this manual, were written by Davin McCall.

+ 8 - 0
src/service.cc

@@ -659,6 +659,14 @@ bool service_record::stop_dependents() noexcept
             if (dep_from->get_state() != service_state_t::STOPPED
                     && dep_from->get_state() != service_state_t::STOPPING) {
                 dep_from->prop_stop = true;
+                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);
+                    }
+                }
                 services->add_prop_queue(dep_from);
             }
         }

+ 40 - 0
src/tests/tests.cc

@@ -291,6 +291,45 @@ void basic_test6()
     assert(sset.count_active_services() == 3);
 }
 
+// An explicitly activated service with automatic restart will restart if it
+// stops due to a dependency stopping, therefore also causing the dependency to restart.
+void basic_test7()
+{
+    service_set sset;
+
+    service_record *s1 = new service_record(&sset, "test-service-1", service_type_t::INTERNAL, {});
+    service_record *s2 = new service_record(&sset, "test-service-2", service_type_t::INTERNAL, {{s1, REG}});
+    service_record *s3 = new service_record(&sset, "test-service-3", service_type_t::INTERNAL, {{s2, REG}});
+    s2->set_auto_restart(true);
+    sset.add_service(s1);
+    sset.add_service(s2);
+    sset.add_service(s3);
+
+    assert(sset.find_service("test-service-1") == s1);
+    assert(sset.find_service("test-service-2") == s2);
+    assert(sset.find_service("test-service-3") == s3);
+
+    // Start all three services:
+    sset.start_service(s3);
+
+    // Also explicitly activate s2:
+    sset.start_service(s2);
+
+    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 stop s1, which should also force s2 and s3 to stop.
+    // s2 should not restart, since this was an explicit forced stop which
+    // inhibits restart, and so s1 should also remain stopped.
+    sset.stop_service(s1);
+
+    assert(s3->get_state() == service_state_t::STOPPED);
+    assert(s2->get_state() == service_state_t::STOPPED);
+    assert(s1->get_state() == service_state_t::STOPPED);
+    assert(sset.count_active_services() == 0);
+}
+
 // Test that service pinned in start state is not stopped when its dependency stops.
 void test_pin1()
 {
@@ -1285,6 +1324,7 @@ int main(int argc, char **argv)
     RUN_TEST(basic_test4, "               ");
     RUN_TEST(basic_test5, "               ");
     RUN_TEST(basic_test6, "               ");
+    RUN_TEST(basic_test7, "               ");
     RUN_TEST(test_pin1, "                 ");
     RUN_TEST(test_pin2, "                 ");
     RUN_TEST(test_pin3, "                 ");