From ade30f5d62acf2540b0b2c51d826a140c1093893 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Mon, 11 Jan 2016 19:38:12 +0000 Subject: [PATCH] Largish re-work of how dependencies are handled. The "start" and "dependentStopped" methods were overloaded in unsound ways. Add require() and release() methods to make an explicit requirement on a dependency. A require() is added when the service is explictly started, or when it is a dependency of such a service (directly), or when it is a dependency of a service which has been required. --- src/service.cc | 125 +++++++++++++++++++++++++------------------------ src/service.h | 25 ++++++---- 2 files changed, 80 insertions(+), 70 deletions(-) diff --git a/src/service.cc b/src/service.cc index 0e90b68..182467d 100644 --- a/src/service.cc +++ b/src/service.cc @@ -56,21 +56,6 @@ void ServiceSet::stopService(const std::string & name) noexcept } } -void ServiceRecord::notify_dependencies_stopped() noexcept -{ - if (desired_state == ServiceState::STOPPED) { - // Stop any dependencies whose desired state is STOPPED: - for (auto i = depends_on.begin(); i != depends_on.end(); i++) { - (*i)->dependentStopped(); - } - for (auto i = soft_deps.begin(); i != soft_deps.end(); i++) { - i->getTo()->dependentStopped(); - } - - service_set->service_inactive(this); - } -} - // Called when a service has actually stopped. void ServiceRecord::stopped() noexcept { @@ -85,6 +70,10 @@ void ServiceRecord::stopped() noexcept notifyListeners(ServiceEvent::STOPPED); + for (auto dependency : depends_on) { + dependency->dependentStopped(); + } + if (desired_state == ServiceState::STARTED) { // Desired state is "started". do_start(); @@ -95,7 +84,7 @@ void ServiceRecord::stopped() noexcept socket_fd = -1; } - notify_dependencies_stopped(); + release_dependencies(); } } @@ -241,6 +230,49 @@ void ServiceRecord::process_child_status(struct ev_loop *loop, ev_io * stat_io, } } +void ServiceRecord::require() noexcept +{ + if (required_by++ == 0) { + // Need to require all our dependencies + for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) { + (*i)->require(); + } + + for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) { + ServiceRecord * to = i->getTo(); + to->require(); + } + } +} + +void ServiceRecord::release() noexcept +{ + if (--required_by == 0) { + desired_state = ServiceState::STOPPED; + // Can stop, and release dependencies once we're stopped. + if (service_state == ServiceState::STOPPED) { + release_dependencies(); + } + else { + do_stop(); + } + } +} + +void ServiceRecord::release_dependencies() noexcept +{ + for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) { + (*i)->release(); + } + + for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) { + ServiceRecord * to = i->getTo(); + to->release(); + } + + service_set->service_inactive(this); +} + void ServiceRecord::start(bool transitive) noexcept { if ((service_state == ServiceState::STARTING || service_state == ServiceState::STARTED) @@ -250,10 +282,8 @@ void ServiceRecord::start(bool transitive) noexcept notifyListeners(ServiceEvent::STOPCANCELLED); } - if (transitive) { - started_deps++; - } - else { + if (!transitive) { + if (!start_explicit) require(); start_explicit = true; } @@ -531,8 +561,6 @@ void ServiceRecord::failed_to_start(bool depfailed) noexcept service_state = ServiceState::STOPPED; notifyListeners(ServiceEvent::FAILEDSTART); - notify_dependencies_stopped(); - // Cancel start of dependents: for (sr_iter i = dependents.begin(); i != dependents.end(); i++) { if ((*i)->service_state == ServiceState::STARTING) { @@ -670,58 +698,28 @@ void ServiceRecord::forceStop() noexcept { if (service_state != ServiceState::STOPPED) { force_stop = true; - for (sr_iter i = dependents.begin(); i != dependents.end(); i++) { - (*i)->forceStop(); - } - - if (service_state == ServiceState::STARTED) { - do_stop(); - } - - // We don't want to force stop soft dependencies, however. + do_stop(); } } void ServiceRecord::dependentStopped() noexcept { - started_deps--; - if (service_state == ServiceState::STOPPING) { + if (service_state == ServiceState::STOPPING && waiting_for_deps) { // Check the other dependents before we stop. if (stopCheckDependents()) { allDepsStopped(); } } - else if (started_deps == 0 && !start_explicit) { - desired_state = ServiceState::STOPPED; - service_state = ServiceState::STOPPING; - allDepsStopped(); - } } void ServiceRecord::stop() noexcept { if (desired_state == ServiceState::STOPPED && service_state != ServiceState::STARTED) return; - start_explicit = false; - if (started_deps != 0) { - return; - } - - if ((service_state == ServiceState::STOPPING || service_state == ServiceState::STOPPED) - && desired_state == ServiceState::STARTED) { - // The service *was* stopped/stopping, but it was going to restart. - // Now, we'll cancel the restart. - notifyListeners(ServiceEvent::STARTCANCELLED); - } - - desired_state = ServiceState::STOPPED; - - if (service_state == ServiceState::STOPPED) { - notify_dependencies_stopped(); - return; + if (start_explicit) { + start_explicit = false; + release(); } - - do_stop(); } void ServiceRecord::do_stop() noexcept @@ -758,7 +756,7 @@ void ServiceRecord::do_stop() noexcept waiting_for_deps = true; // If we get here, we are in STARTED state; stop all dependents. - if (stopCheckDependents()) { + if (stopDependents()) { allDepsStopped(); } } @@ -767,7 +765,7 @@ bool ServiceRecord::stopCheckDependents() noexcept { bool all_deps_stopped = true; for (sr_iter i = dependents.begin(); i != dependents.end(); ++i) { - if ((*i)->service_state != ServiceState::STOPPED) { + if (! (*i)->is_stopped()) { all_deps_stopped = false; break; } @@ -780,9 +778,14 @@ bool ServiceRecord::stopDependents() noexcept { bool all_deps_stopped = true; for (sr_iter i = dependents.begin(); i != dependents.end(); ++i) { - if ((*i)->service_state != ServiceState::STOPPED) { + if (! (*i)->is_stopped()) { all_deps_stopped = false; - (*i)->stop(); + if (force_stop) { + (*i)->forceStop(); + } + else { + (*i)->do_stop(); + } } } diff --git a/src/service.h b/src/service.h index ffd2889..7c39395 100644 --- a/src/service.h +++ b/src/service.h @@ -178,7 +178,7 @@ class ServiceRecord bool doing_recovery : 1; // if we are currently recovering a BGPROCESS (restarting process, while // holding STARTED service state) bool start_explicit : 1; // whether we are are explictly required to be started - int started_deps = 0; // number of dependents that require this service to be started + int required_by = 0; // number of dependents wanting this service to be started typedef std::list sr_list; typedef sr_list::iterator sr_iter; @@ -255,6 +255,9 @@ class ServiceRecord void handle_exit_status() noexcept; + void do_start() noexcept; + void do_stop() noexcept; + // A dependency has reached STARTED state void dependencyStarted() noexcept; @@ -295,7 +298,16 @@ class ServiceRecord // issue a stop to all dependents, return true if they are all already stopped bool stopDependents() noexcept; - void forceStop() noexcept; // force-stop this service and all dependents + void require() noexcept; + void release() noexcept; + void release_dependencies() noexcept; + + // Check if service is, fundamentally, stopped. + bool is_stopped() noexcept + { + return service_state == ServiceState::STOPPED + || (service_state == ServiceState::STARTING && waiting_for_deps); + } void notifyListeners(ServiceEvent event) noexcept { @@ -313,11 +325,6 @@ class ServiceRecord // Release console (console must be currently held by this service) void releaseConsole() noexcept; - bool get_start_flag(bool transitive) - { - return transitive ? started_deps != 0 : start_explicit; - } - public: ServiceRecord(ServiceSet *set, string name) @@ -424,8 +431,8 @@ class ServiceRecord void start(bool transitive = false) noexcept; // start the service void stop() noexcept; // stop the service - void do_start() noexcept; - void do_stop() noexcept; + + void forceStop() noexcept; // force-stop this service and all dependents void pinStart() noexcept; // start the service and pin it void pinStop() noexcept; // stop the service and pin it -- 2.25.1