From df480b7de4cd9378b7384e8ebc4aed7767997e84 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 14 Jun 2016 18:05:31 +0100 Subject: [PATCH] Introduce a "stop queue" and "start queue" in ServiceSet, to better manage service interactions and avoid recursion. Both are implemented as a singly-linked list of services. The queues are processed one at a time until empty. --- src/control.cc | 5 +++ src/service.cc | 24 +++++++------ src/service.h | 97 ++++++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 106 insertions(+), 20 deletions(-) diff --git a/src/control.cc b/src/control.cc index 5c44c11..825e8f3 100644 --- a/src/control.cc +++ b/src/control.cc @@ -161,6 +161,7 @@ bool ControlConn::processStartStop(int pktType) // start service, mark as required if (do_pin) service->pinStart(); service->start(); + service_set->processQueues(true); already_there = service->getState() == ServiceState::STARTED; break; case DINIT_CP_STOPSERVICE: @@ -168,18 +169,21 @@ bool ControlConn::processStartStop(int pktType) if (do_pin) service->pinStop(); service->stop(true); service->forceStop(); + service_set->processQueues(false); already_there = service->getState() == ServiceState::STOPPED; break; case DINIT_CP_WAKESERVICE: // re-start a stopped service (do not mark as required) if (do_pin) service->pinStart(); service->start(false); + service_set->processQueues(true); already_there = service->getState() == ServiceState::STARTED; break; case DINIT_CP_RELEASESERVICE: // remove required mark, stop if not required by dependents if (do_pin) service->pinStop(); service->stop(); + service_set->processQueues(false); already_there = service->getState() == ServiceState::STOPPED; break; default: @@ -226,6 +230,7 @@ bool ControlConn::processUnpinService() } else { service->unpin(); + service_set->processQueues(true); char ack_buf[] = { (char) DINIT_RP_ACK }; if (! queuePacket(ack_buf, 1)) return false; } diff --git a/src/service.cc b/src/service.cc index d367b2b..1e717a1 100644 --- a/src/service.cc +++ b/src/service.cc @@ -58,6 +58,7 @@ void ServiceSet::stopService(const std::string & name) noexcept ServiceRecord *record = findService(name); if (record != nullptr) { record->stop(); + processQueues(false); } } @@ -154,6 +155,7 @@ void ServiceRecord::handle_exit_status() noexcept // Failed startup: no auto-restart. desired_state = ServiceState::STOPPED; forceStop(); + service_set->processQueues(false); } return; @@ -186,6 +188,7 @@ void ServiceRecord::handle_exit_status() noexcept if (! do_auto_restart()) desired_state = ServiceState::STOPPED; forceStop(); } + service_set->processQueues(false); } else { // SCRIPTED if (service_state == ServiceState::STOPPING) { @@ -199,6 +202,7 @@ void ServiceRecord::handle_exit_status() noexcept // can be stopped: stopped(); } + service_set->processQueues(false); } else { // STARTING if (exit_status == 0) { @@ -216,7 +220,7 @@ void ServiceRecord::handle_exit_status() noexcept Rearm ServiceIoWatcher::gotEvent(EventLoop_t *loop, int fd, int flags) noexcept { ServiceRecord::process_child_status(loop, this, flags); - return Rearm::NOOP; + return Rearm::REMOVED; } // TODO remove unused revents param @@ -287,7 +291,7 @@ void ServiceRecord::release() noexcept release_dependencies(); } else { - do_stop(); + service_set->addToStopQueue(this); } } } @@ -522,9 +526,6 @@ bool ServiceRecord::read_pid_file() noexcept pidbuf[r] = 0; // store nul terminator pid = std::atoi(pidbuf); if (kill(pid, 0) == 0) { - //ev_child_init(&child_listener, process_child_callback, pid, 0); - //child_listener.data = this; - //ev_child_start(ev_default_loop(EVFLAG_AUTO), &child_listener); child_listener.registerWith(&eventLoop, pid); } else { @@ -567,7 +568,7 @@ void ServiceRecord::started() noexcept if (force_stop || desired_state == ServiceState::STOPPED) { // We must now stop. - do_stop(); + service_set->addToStopQueue(this); return; } @@ -589,7 +590,10 @@ void ServiceRecord::failed_to_start(bool depfailed) noexcept logServiceFailed(service_name); service_state = ServiceState::STOPPED; - stop(); // release dependencies if appropriate + if (start_explicit) { + start_explicit = false; + release(); + } notifyListeners(ServiceEvent::FAILEDSTART); // Cancel start of dependents: @@ -736,7 +740,7 @@ void ServiceRecord::forceStop() noexcept { if (service_state != ServiceState::STOPPED) { force_stop = true; - do_stop(); + service_set->addToStopQueue(this); } } @@ -759,7 +763,7 @@ void ServiceRecord::stop(bool bring_down) noexcept if (bring_down && desired_state != ServiceState::STOPPED) { desired_state = ServiceState::STOPPED; - do_stop(); + service_set->addToStopQueue(this); } } @@ -830,7 +834,7 @@ bool ServiceRecord::stopDependents() noexcept (*i)->forceStop(); } else { - (*i)->do_stop(); + service_set->addToStopQueue(*i); } } diff --git a/src/service.h b/src/service.h index 4775fcf..8b93080 100644 --- a/src/service.h +++ b/src/service.h @@ -284,10 +284,23 @@ class ServiceRecord int exit_status; // Exit status, if the process has exited (pid == -1). int socket_fd = -1; // For socket-activation services, this is the file // descriptor for the socket. - + ServiceChildWatcher child_listener; ServiceIoWatcher child_status_listener; + // Data for use by ServiceSet + public: + + // Next service (after this one) in the queue for the console. Intended to only be used by ServiceSet class. + ServiceRecord *next_for_console; + + // Start/stop queues + ServiceRecord *next_in_start_queue = nullptr; + ServiceRecord *next_in_stop_queue = nullptr; + + + private: + // All dependents have stopped. void allDepsStopped(); @@ -315,12 +328,6 @@ class ServiceRecord void handle_exit_status() noexcept; - // Called on transition of desired state from stopped to started (or unpinned stop) - void do_start() noexcept; - - // Called on transition of desired state from started to stopped (or unpinned start) - void do_stop() noexcept; - // A dependency has reached STARTED state void dependencyStarted() noexcept; @@ -427,8 +434,11 @@ class ServiceRecord // TODO write a destructor - // Next service (after this one) in the queue for the console. Intended to only be used by ServiceSet class. - ServiceRecord *next_for_console; + // Called on transition of desired state from stopped to started (or unpinned stop) + void do_start() noexcept; + + // Called on transition of desired state from started to stopped (or unpinned start) + void do_stop() noexcept; // Console is available. void acquiredConsole() noexcept; @@ -537,7 +547,25 @@ class ServiceRecord } }; - +/* + * A ServiceSet, as the name suggests, manages a set of services. + * + * Other than the ability to find services by name, the service set manages various queues. + * One is the queue for processes wishing to acquire the console. There is also a set of + * processes that want to start, and another set of those that want to stop. These latter + * two "queues" (not really queues since their order is not important) are used to prevent too + * much recursion and to prevent service states from "bouncing" too rapidly. + * + * A service that wishes to stop puts itself on the stop queue; a service that wishes to start + * puts itself on the start queue. Any operation that potentially manipulates the queues must + * be folloed by a "process queues" order (processQueues method, which can be instructed to + * process either the start queue or the stop queue first). + * + * Note that which queue it does process first, processQueues always repeatedly processes both + * queues until they are empty. The process is finite because starting a service can never + * cause services to be added to the stop queue, unless they fail to start, which should cause + * them to stop semi-permanently. + */ class ServiceSet { int active_services; @@ -549,6 +577,10 @@ class ServiceSet ServiceRecord * console_queue_head = nullptr; // first record in console queue ServiceRecord * console_queue_tail = nullptr; // last record in console queue + + // start/stop "queue" - list of services waiting to stop/start + ServiceRecord * first_start_queue = nullptr; + ServiceRecord * first_stop_queue = nullptr; // Private methods @@ -597,6 +629,51 @@ class ServiceSet // transition to the 'stopped' state. void stopService(const std::string &name) noexcept; + // Add a service record to the start queue + void addToStartQueue(ServiceRecord *service) noexcept + { + if (service->next_in_start_queue == nullptr && first_start_queue != service) { + service->next_in_start_queue = first_start_queue; + first_start_queue = service; + } + } + + // Add a service to the stop queue + void addToStopQueue(ServiceRecord *service) noexcept + { + if (service->next_in_stop_queue == nullptr && first_stop_queue != service) { + service->next_in_stop_queue = first_stop_queue; + first_stop_queue = service; + } + } + + void processQueues(bool do_start_first) noexcept + { + if (! do_start_first) { + while (first_stop_queue != nullptr) { + auto next = first_stop_queue; + first_stop_queue = next->next_in_stop_queue; + next->next_in_stop_queue = nullptr; + next->do_stop(); + } + } + + while (first_stop_queue != nullptr || first_start_queue != nullptr) { + while (first_start_queue != nullptr) { + auto next = first_start_queue; + first_start_queue = next->next_in_start_queue; + next->next_in_start_queue = nullptr; + next->do_start(); + } + while (first_stop_queue != nullptr) { + auto next = first_stop_queue; + first_stop_queue = next->next_in_stop_queue; + next->next_in_stop_queue = nullptr; + next->do_stop(); + } + } + } + // Set the console queue tail (returns previous tail) ServiceRecord * consoleQueueTail(ServiceRecord * newTail) noexcept { -- 2.25.1