From d9e61e1bf61af5f559676cb36ae8179e7d6525a1 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Sat, 10 Jun 2017 23:22:56 +0100 Subject: [PATCH] Service logic simplification/cleanup. Cleanly separate initial state propagation from transition actions. --- src/service.cc | 166 +++++++++++++++++++++++-------------------------- src/service.h | 35 ++++++++--- 2 files changed, 107 insertions(+), 94 deletions(-) diff --git a/src/service.cc b/src/service.cc index 1b19770..b39fd10 100644 --- a/src/service.cc +++ b/src/service.cc @@ -78,24 +78,19 @@ void ServiceRecord::stopped() noexcept force_stop = false; // If we are a soft dependency of another target, break the acquisition from that target now: - bool will_restart = (desired_state == ServiceState::STARTED) - && service_set->get_auto_restart(); - - if (! will_restart) { - for (auto dependency : soft_dpts) { - if (dependency->holding_acq) { - dependency->holding_acq = false; - release(); - } + for (auto dependent : soft_dpts) { + if (dependent->holding_acq) { + dependent->holding_acq = false; + release(); } } + bool will_restart = (desired_state == ServiceState::STARTED) + && service_set->get_auto_restart(); + for (auto dependency : depends_on) { - // we signal dependencies in case they are waiting for us to stop - but only if we won't - // restart or if they are stopping uninterruptibly. - if (! will_restart || ! dependency->can_interrupt_stop()) { - dependency->dependentStopped(); - } + // we signal dependencies in case they are waiting for us to stop: + dependency->dependentStopped(); } service_state = ServiceState::STOPPED; @@ -103,7 +98,7 @@ void ServiceRecord::stopped() noexcept if (will_restart) { // Desired state is "started". restarting = true; - service_set->addToStartQueue(this); + start(false); } else { if (socket_fd != -1) { @@ -368,17 +363,9 @@ rearm ServiceIoWatcher::fd_event(EventLoop_t &loop, int fd, int flags) noexcept void ServiceRecord::require() noexcept { if (required_by++ == 0) { - - if (! prop_require) { - prop_require = true; - prop_release = false; - service_set->addToPropQueue(this); - } - - if (service_state == ServiceState::STOPPED) { - // (In any other state, the service is already considered active.) - service_set->service_active(this); - } + prop_require = !prop_release; + prop_release = false; + service_set->addToPropQueue(this); } } @@ -386,15 +373,18 @@ void ServiceRecord::release() noexcept { if (--required_by == 0) { desired_state = ServiceState::STOPPED; - // Can stop, and can release dependencies now: - prop_release = true; + + // Can stop, and can release dependencies now. We don't need to issue a release if + // the require was pending though: + prop_release = !prop_require; prop_require = false; service_set->addToPropQueue(this); - if (service_state != ServiceState::STOPPED) { - service_set->addToStopQueue(this); + + if (service_state == ServiceState::STOPPED) { + service_set->service_inactive(this); } else { - service_set->service_inactive(this); + do_stop(); } } } @@ -422,9 +412,31 @@ void ServiceRecord::start(bool activate) noexcept } if (desired_state == ServiceState::STARTED && service_state != ServiceState::STOPPED) return; - + + bool was_active = service_state != ServiceState::STOPPED || desired_state != ServiceState::STOPPED; desired_state = ServiceState::STARTED; - service_set->addToStartQueue(this); + + if (service_state != ServiceState::STOPPED) { + // We're already starting/started, or we are stopping and need to wait for + // that the complete. + if (service_state != ServiceState::STOPPING || ! can_interrupt_stop()) { + return; + } + // We're STOPPING, and that can be interrupted. Our dependencies might be STOPPING, + // but if so they are waiting (for us), so they too can be instantly returned to + // STARTING state. + notifyListeners(ServiceEvent::STOPCANCELLED); + } + else if (! was_active) { + service_set->service_active(this); + } + + service_state = ServiceState::STARTING; + waiting_for_deps = true; + + if (startCheckDependencies(true)) { + service_set->addToStartQueue(this); + } } void ServiceRecord::do_propagation() noexcept @@ -454,35 +466,27 @@ void ServiceRecord::do_propagation() noexcept failed_to_start(true); } - if (waiting_for_deps) { - if (service_state == ServiceState::STARTING) { - if (startCheckDependencies(false)) { - allDepsStarted(); - } - } - else if (service_state == ServiceState::STOPPING) { - if (stopCheckDependents()) { - all_deps_stopped(); - } - } + if (prop_start) { + prop_start = false; + start(false); + } + + if (prop_stop) { + prop_stop = false; + do_stop(); } } void ServiceRecord::execute_transition() noexcept { - bool is_started = (service_state == ServiceState::STARTED) - || (service_state == ServiceState::STARTING && can_interrupt_start()); - bool is_stopped = (service_state == ServiceState::STOPPED) - || (service_state == ServiceState::STOPPING && can_interrupt_stop()); - - if (is_started && (desired_state == ServiceState::STOPPED || force_stop)) { - if (! pinned_started) { - do_stop(); + if (service_state == ServiceState::STARTING) { + if (startCheckDependencies(false)) { + allDepsStarted(false); } } - else if (is_stopped && desired_state == ServiceState::STARTED) { - if (! pinned_stopped) { - do_start(); + else if (service_state == ServiceState::STOPPING) { + if (stopCheckDependents()) { + all_deps_stopped(); } } } @@ -491,16 +495,8 @@ void ServiceRecord::do_start() noexcept { if (pinned_stopped) return; - if (service_state != ServiceState::STOPPED) { - // We're already starting/started, or we are stopping and need to wait for - // that the complete. - if (service_state != ServiceState::STOPPING || ! can_interrupt_stop()) { - return; - } - // We're STOPPING, and that can be interrupted. Our dependencies might be STOPPING, - // but if so they are waiting (for us), so they too can be instantly returned to - // STARTING state. - notifyListeners(ServiceEvent::STOPCANCELLED); + if (service_state != ServiceState::STARTING) { + return; } service_state = ServiceState::STARTING; @@ -508,18 +504,16 @@ void ServiceRecord::do_start() noexcept waiting_for_deps = true; // Ask dependencies to start, mark them as being waited on. - if (! startCheckDependencies(true)) { - return; + if (startCheckDependencies(false)) { + // Once all dependencies are started, we start properly: + allDepsStarted(); } - - // Actually start this service. - allDepsStarted(); } void ServiceRecord::dependencyStarted() noexcept { if (service_state == ServiceState::STARTING && waiting_for_deps) { - service_set->addToPropQueue(this); + service_set->addToStartQueue(this); } } @@ -531,7 +525,8 @@ bool ServiceRecord::startCheckDependencies(bool start_deps) noexcept if ((*i)->service_state != ServiceState::STARTED) { if (start_deps) { all_deps_started = false; - (*i)->start(false); + (*i)->prop_start = true; + service_set->addToPropQueue(*i); } else { return false; @@ -543,7 +538,8 @@ bool ServiceRecord::startCheckDependencies(bool start_deps) noexcept ServiceRecord * to = i->getTo(); if (start_deps) { if (to->service_state != ServiceState::STARTED) { - to->start(false); + to->prop_start = true; + service_set->addToPropQueue(to); i->waiting_on = true; all_deps_started = false; } @@ -712,7 +708,7 @@ void ServiceRecord::started() noexcept if (force_stop || desired_state == ServiceState::STOPPED) { // We must now stop. - service_set->addToStopQueue(this); + do_stop(); return; } @@ -1002,7 +998,7 @@ void ServiceRecord::forceStop() noexcept void ServiceRecord::dependentStopped() noexcept { if (service_state == ServiceState::STOPPING && waiting_for_deps) { - service_set->addToPropQueue(this); + service_set->addToStopQueue(this); } } @@ -1012,10 +1008,9 @@ void ServiceRecord::stop(bool bring_down) noexcept start_explicit = false; release(); } - - if (bring_down && desired_state != ServiceState::STOPPED) { - desired_state = ServiceState::STOPPED; - service_set->addToStopQueue(this); + + if (bring_down) { + do_stop(); } } @@ -1037,10 +1032,8 @@ void ServiceRecord::do_stop() noexcept // We must have had desired_state == STARTED. notifyListeners(ServiceEvent::STARTCANCELLED); - // Reaching this point, we have can_interrupt_start() == true. So, - // we can stop. Dependents might be starting, but they must be - // waiting on us, so they should also be immediately stoppable. - // Fall through to below,. + // Reaching this point, we are starting interruptibly - so we + // stop now (by falling through to below). } else { // If we're starting we need to wait for that to complete. @@ -1051,10 +1044,8 @@ void ServiceRecord::do_stop() noexcept service_state = ServiceState::STOPPING; waiting_for_deps = true; - - // If we get here, we are in STARTED state; stop all dependents. if (stopDependents()) { - all_deps_stopped(); + service_set->addToStopQueue(this); } } @@ -1088,7 +1079,8 @@ bool ServiceRecord::stopDependents() noexcept (*i)->forceStop(); } - (*i)->do_stop(); + (*i)->prop_stop = true; + service_set->addToPropQueue(*i); } return all_deps_stopped; diff --git a/src/service.h b/src/service.h index 07f5248..5385788 100644 --- a/src/service.h +++ b/src/service.h @@ -71,14 +71,33 @@ * * Two-phase transition * -------------------- - * Transition between states occurs in two phases: propagation and execution. In the - * propagation phase, acquisition/release messages are processed, and desired state may be - * altered accordingly. Desired state of dependencies/dependents should not be examined in - * this phase, since it may change during the phase (i.e. its current value at any point - * may not reflect the true final value). + * Transition between states occurs in two phases: propagation and execution. In both phases + * a linked-list queue is used to keep track of which services need processing; this avoids + * recursion (which would be of unknown depth and therefore liable to stack overflow). + * + * In the propagation phase, acquisition/release messages are processed, and desired state may be + * altered accordingly. Start and stop requests are also propagated in this phase. The state may + * be set to STARTING or STOPPING to reflect the desired state, but will never be set to STARTED + * or STOPPED (that happens in the execution phase). + * + * Propagation variables: + * prop_acquire: the service has transitioned to an acquired state and must issue an acquire + * on its dependencies + * prop_release: the service has transitioned to a released state and must issue a release on + * its dependencies. + * + * prop_start: the service should start + * prop_stop: the service should stop + * + * Note that "prop_acquire"/"prop_release" form a pair which cannot both be set at the same time + * which is enforced via explicit checks. For "prop_start"/"prop_stop" this occurs implicitly. * * In the execution phase, actions are taken to achieve the desired state. Actual state may - * transition according to the current and desired states. + * transition according to the current and desired states. Processes can be sent signals, etc + * in order to stop them. A process can restart if it stops, but it does so by raising prop_start + * which needs to be processed in a second transition phase. Seeing as starting never causes + * another process to stop, the transition-execute-transition cycle always ends at the 2nd + * transition stage, at the latest. */ struct OnstartFlags { @@ -244,6 +263,8 @@ class ServiceRecord bool prop_require : 1; // require must be propagated bool prop_release : 1; // release must be propagated bool prop_failure : 1; // failure to start must be propagated + bool prop_start : 1; + bool prop_stop : 1; bool restarting : 1; // re-starting after unexpected termination int required_by = 0; // number of dependents wanting this service to be started @@ -404,7 +425,7 @@ class ServiceRecord pinned_stopped(false), pinned_started(false), waiting_for_deps(false), waiting_for_execstat(false), start_explicit(false), prop_require(false), prop_release(false), prop_failure(false), - restarting(false), force_stop(false) + prop_start(false), prop_stop(false), restarting(false), force_stop(false) { service_set = set; service_name = name; -- 2.25.1