From 6f0f45e5905144ea892ba356fced69bf9f9fbedd Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Fri, 1 Jan 2016 18:11:07 +0000 Subject: [PATCH] Various improvements to state transitions. Introduce a new variable to track if a service is waiting for its dependencies (starting) or dependents (stopping). In these cases it is possible to transition directly from STARTING to STOPPED or from STOPPING to STARTED. This also removes the need for special handling of "internal" services (which will never transition from STARTING to STARTED or STOPPING to STOPPED excet via transitions of their dependencies/dependents). --- service.cc | 88 ++++++++++++++++++++++++++++++------------------------ service.h | 19 ++++++++++-- 2 files changed, 66 insertions(+), 41 deletions(-) diff --git a/service.cc b/service.cc index 33dc2ab..b2086f8 100644 --- a/service.cc +++ b/service.cc @@ -129,7 +129,7 @@ void ServiceRecord::process_child_callback(struct ev_loop *loop, ev_child *w, in } } -void ServiceRecord::start(bool unpinned) noexcept +void ServiceRecord::start() noexcept { if ((service_state == ServiceState::STARTING || service_state == ServiceState::STARTED) && desired_state == ServiceState::STOPPED) { @@ -138,21 +138,28 @@ void ServiceRecord::start(bool unpinned) noexcept notifyListeners(ServiceEvent::STOPCANCELLED); } - if (desired_state == ServiceState::STARTED && !unpinned) return; + if (desired_state == ServiceState::STARTED && service_state != ServiceState::STOPPED) return; desired_state = ServiceState::STARTED; + 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. - return; + 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. } - if (pinned_stopped) return; - service_state = ServiceState::STARTING; service_set->service_active(this); + waiting_for_deps = true; + // Ask dependencies to start, mark them as being waited on. if (! startCheckDependencies(true)) { return; @@ -164,7 +171,7 @@ void ServiceRecord::start(bool unpinned) noexcept void ServiceRecord::dependencyStarted() noexcept { - if (service_state != ServiceState::STARTING) { + if (service_state != ServiceState::STARTING || ! waiting_for_deps) { return; } @@ -219,9 +226,12 @@ bool ServiceRecord::startCheckDependencies(bool start_deps) noexcept void ServiceRecord::allDepsStarted(bool hasConsole) noexcept { if (onstart_flags.runs_on_console && ! hasConsole) { + waiting_for_deps = true; queueForConsole(); return; } + + waiting_for_deps = false; if (service_type == ServiceType::PROCESS) { bool start_success = start_ps_process(); @@ -287,14 +297,10 @@ void ServiceRecord::started() // Notify any dependents whose desired state is STARTED: for (auto i = dependents.begin(); i != dependents.end(); i++) { - if ((*i)->desired_state == ServiceState::STARTED) { - (*i)->dependencyStarted(); - } + (*i)->dependencyStarted(); } for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) { - if ((*i)->getFrom()->desired_state == ServiceState::STARTED) { - (*i)->getFrom()->dependencyStarted(); - } + (*i)->getFrom()->dependencyStarted(); } } @@ -319,11 +325,9 @@ void ServiceRecord::failed_to_start() } } for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) { - if ((*i)->getFrom()->desired_state == ServiceState::STARTED) { - // We can send 'start', because this is only a soft dependency. - // Our startup failure means that they don't have to wait for us. - (*i)->getFrom()->dependencyStarted(); - } + // We can send 'start', because this is only a soft dependency. + // Our startup failure means that they don't have to wait for us. + (*i)->getFrom()->dependencyStarted(); } } @@ -460,6 +464,7 @@ void ServiceRecord::forceStop() noexcept } // A dependency of this service failed to start. +// Only called when state == STARTING. void ServiceRecord::failed_dependency() { desired_state = ServiceState::STOPPED; @@ -467,6 +472,7 @@ void ServiceRecord::failed_dependency() // Presumably, we were starting. So now we're not. service_state = ServiceState::STOPPED; service_set->service_inactive(this); + logServiceFailed(service_name); // Notify dependents of this service also for (auto i = dependents.begin(); i != dependents.end(); i++) { @@ -475,11 +481,9 @@ void ServiceRecord::failed_dependency() } } for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) { - if ((*i)->getFrom()->service_state == ServiceState::STARTING) { - // It's a soft dependency, so send them 'started' rather than - // 'failed dep'. - (*i)->getFrom()->dependencyStarted(); - } + // It's a soft dependency, so send them 'started' rather than + // 'failed dep'. + (*i)->getFrom()->dependencyStarted(); } } @@ -493,7 +497,7 @@ void ServiceRecord::dependentStopped() noexcept } } -void ServiceRecord::stop(bool unpinned) noexcept +void ServiceRecord::stop() noexcept { if ((service_state == ServiceState::STOPPING || service_state == ServiceState::STOPPED) && desired_state == ServiceState::STARTED) { @@ -510,25 +514,30 @@ void ServiceRecord::stop(bool unpinned) noexcept if (service_state != ServiceState::STARTED) { if (service_state == ServiceState::STARTING) { - // Well this is awkward: we're going to have to continue - // starting, but we don't want any dependents to think that - // they are still waiting to start. - // Make sure they remain stopped: - if (stopDependents()) { - if (service_type == ServiceType::INTERNAL) { - // Internal services can go straight from STARTING to STOPPED. - allDepsStopped(); - // (Other types have to finish starting first). - } + if (! can_interrupt_start()) { + // Well this is awkward: we're going to have to continue + // starting, but we don't want any dependents to think that + // they are still waiting to start. + // Make sure they remain stopped: + stopDependents(); + return; } + + // 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. + } + else { + // If we're starting we need to wait for that to complete. + // If we're already stopping/stopped there's nothing to do. + return; } - - // If we're starting we need to wait for that to complete. - // If we're already stopping/stopped there's nothing to do. - return; } service_state = ServiceState::STOPPING; + waiting_for_deps = true; + // If we get here, we are in STARTED state; stop all dependents. if (stopDependents()) { allDepsStopped(); @@ -566,6 +575,7 @@ bool ServiceRecord::stopDependents() noexcept // Dependency stopped or is stopping; we must stop too. void ServiceRecord::allDepsStopped() { + waiting_for_deps = false; if (service_type == ServiceType::PROCESS) { if (pid != -1) { // The process is still kicking on - must actually kill it. @@ -611,13 +621,13 @@ void ServiceRecord::unpin() noexcept if (pinned_started) { pinned_started = false; if (desired_state == ServiceState::STOPPED) { - stop(true); + stop(); } } if (pinned_stopped) { pinned_stopped = false; if (desired_state == ServiceState::STARTED) { - start(true); + start(); } } } diff --git a/service.h b/service.h index ea83149..98e2cdd 100644 --- a/service.h +++ b/service.h @@ -168,6 +168,8 @@ class ServiceRecord bool auto_restart : 1; /* whether to restart this (process) if it dies unexpectedly */ bool pinned_stopped : 1; bool pinned_started : 1; + + bool waiting_for_deps : 1; /* if STARTING, whether we are waiting for dependencies (inc console) to start */ typedef std::list sr_list; typedef sr_list::iterator sr_iter; @@ -240,6 +242,19 @@ class ServiceRecord // Check whether dependencies have started, and optionally ask them to start bool startCheckDependencies(bool do_start) noexcept; + + // Whether a STARTING service can immediately transition to STOPPED (as opposed to + // having to wait for it reach STARTED and then go through STOPPING). + bool can_interrupt_start() noexcept + { + return waiting_for_deps; + } + + // Whether a STOPPING service can immediately transition to STARTED. + bool can_interrupt_stop() noexcept + { + return waiting_for_deps && ! force_stop; + } // A dependent has reached STOPPED state void dependentStopped() noexcept; @@ -352,8 +367,8 @@ class ServiceRecord const char *getServiceName() const noexcept { return service_name.c_str(); } ServiceState getState() const noexcept { return service_state; } - void start(bool unpinned = false) noexcept; // start the service - void stop(bool unpinned = false) noexcept; // stop the service + void start() noexcept; // start the service + void stop() noexcept; // stop the service void pinStart() noexcept; // start the service and pin it void pinStop() noexcept; // stop the service and pin it -- 2.25.1