From 34f77d13fdde0db35883dbde23e018c191de417a Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Wed, 24 May 2017 00:19:34 +0100 Subject: [PATCH] Refactoring. Move BGPROCESS and PROCESS service records to separate subclasses of ServiceRecord. --- src/load_service.cc | 14 +++- src/service.cc | 190 ++++++++++++++++++++++++++++---------------- src/service.h | 59 +++++++++++--- 3 files changed, 185 insertions(+), 78 deletions(-) diff --git a/src/load_service.cc b/src/load_service.cc index b4322be..cdd5b8a 100644 --- a/src/load_service.cc +++ b/src/load_service.cc @@ -506,8 +506,18 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name) if (*iter == rval) { // We've found the dummy record delete rval; - rval = new ServiceRecord(this, string(name), service_type, std::move(command), command_offsets, - & depends_on, & depends_soft); + if (service_type == ServiceType::PROCESS) { + rval = new process_service(this, string(name), std::move(command), + command_offsets, &depends_on, &depends_soft); + } + else if (service_type == ServiceType::BGPROCESS) { + rval = new bgproc_service(this, string(name), std::move(command), + command_offsets, &depends_on, &depends_soft); + } + else { + rval = new ServiceRecord(this, string(name), service_type, std::move(command), command_offsets, + &depends_on, &depends_soft); + } rval->setStopCommand(stop_command, stop_command_offsets); rval->setLogfile(logfile); rval->setAutoRestart(auto_restart); diff --git a/src/service.cc b/src/service.cc index fe18e51..350ae82 100644 --- a/src/service.cc +++ b/src/service.cc @@ -143,7 +143,7 @@ dasynq::rearm ServiceChildWatcher::child_status(EventLoop_t &loop, pid_t child, // Must deregister now since handle_exit_status might result in re-launch: deregister(loop, child); - sr->handle_exit_status(); + sr->handle_exit_status(status); return rearm::REMOVED; } @@ -155,7 +155,7 @@ bool ServiceRecord::do_auto_restart() noexcept return false; } -void ServiceRecord::handle_exit_status() noexcept +void ServiceRecord::handle_exit_status(int exit_status) noexcept { bool did_exit = WIFEXITED(exit_status); bool was_signalled = WIFSIGNALED(exit_status); @@ -168,6 +168,99 @@ void ServiceRecord::handle_exit_status() noexcept log(LogLevel::ERROR, "Service ", service_name, " terminated due to signal ", WTERMSIG(exit_status)); } } + + // BGPROCESS and PROCESS override this method; we must be a SCRIPTED service. + if (service_state == ServiceState::STOPPING) { + if (did_exit && WEXITSTATUS(exit_status) == 0) { + stopped(); + } + else { + // ??? failed to stop! Let's log it as info: + if (did_exit) { + log(LogLevel::INFO, "Service ", service_name, " stop command failed with exit code ", WEXITSTATUS(exit_status)); + } + else if (was_signalled) { + log(LogLevel::INFO, "Serivice ", service_name, " stop command terminated due to signal ", WTERMSIG(exit_status)); + } + // Just assume that we stopped, so that any dependencies + // can be stopped: + stopped(); + } + service_set->processQueues(false); + } + else { // STARTING + if (exit_status == 0) { + started(); + } + else { + // failed to start + if (did_exit) { + log(LogLevel::ERROR, "Service ", service_name, " command failed with exit code ", WEXITSTATUS(exit_status)); + } + else if (was_signalled) { + log(LogLevel::ERROR, "Service ", service_name, " command terminated due to signal ", WTERMSIG(exit_status)); + } + failed_to_start(); + } + service_set->processQueues(true); + } +} + +void process_service::handle_exit_status(int exit_status) noexcept +{ + bool did_exit = WIFEXITED(exit_status); + bool was_signalled = WIFSIGNALED(exit_status); + + if (exit_status != 0 && service_state != ServiceState::STOPPING) { + if (did_exit) { + log(LogLevel::ERROR, "Service ", service_name, " process terminated with exit code ", WEXITSTATUS(exit_status)); + } + else if (was_signalled) { + log(LogLevel::ERROR, "Service ", service_name, " terminated due to signal ", WTERMSIG(exit_status)); + } + } + + if (service_state == ServiceState::STARTING) { + if (did_exit && WEXITSTATUS(exit_status) == 0) { + started(); + } + else { + failed_to_start(); + } + } + else if (service_state == ServiceState::STOPPING) { + // We won't log a non-zero exit status or termination due to signal here - + // we assume that the process died because we signalled it. + stopped(); + } + else if (smooth_recovery && service_state == ServiceState::STARTED && desired_state == ServiceState::STARTED) { + // TODO ensure a minimum time between restarts + // TODO if we are pinned-started then we should probably check + // that dependencies have started before trying to re-start the + // service process. + start_ps_process(); + return; + } + else { + if (! do_auto_restart()) desired_state = ServiceState::STOPPED; + forceStop(); + } + service_set->processQueues(false); +} + +void bgproc_service::handle_exit_status(int exit_status) noexcept +{ + bool did_exit = WIFEXITED(exit_status); + bool was_signalled = WIFSIGNALED(exit_status); + + if (exit_status != 0 && service_state != ServiceState::STOPPING) { + if (did_exit) { + log(LogLevel::ERROR, "Service ", service_name, " process terminated with exit code ", WEXITSTATUS(exit_status)); + } + else if (was_signalled) { + log(LogLevel::ERROR, "Service ", service_name, " terminated due to signal ", WTERMSIG(exit_status)); + } + } if (doing_recovery) { // (BGPROCESS only) @@ -184,83 +277,46 @@ void ServiceRecord::handle_exit_status() noexcept } } } - + if (need_stop) { // Failed startup: no auto-restart. desired_state = ServiceState::STOPPED; forceStop(); service_set->processQueues(false); } - + return; } - - if (service_type == ServiceType::PROCESS || service_type == ServiceType::BGPROCESS) { - if (service_state == ServiceState::STARTING) { - // (only applies to BGPROCESS) - if (did_exit && WEXITSTATUS(exit_status) == 0) { - started(); - } - else { - failed_to_start(); - } - } - else if (service_state == ServiceState::STOPPING) { - // We won't log a non-zero exit status or termination due to signal here - - // we assume that the process died because we signalled it. - stopped(); - } - else if (smooth_recovery && service_state == ServiceState::STARTED && desired_state == ServiceState::STARTED) { - // TODO ensure a minimum time between restarts - // TODO if we are pinned-started then we should probably check - // that dependencies have started before trying to re-start the - // service process. - doing_recovery = (service_type == ServiceType::BGPROCESS); - start_ps_process(); - return; + + if (service_state == ServiceState::STARTING) { + // POSIX requires that if the process exited clearly with a status code of 0, + // the exit status value will be 0: + if (exit_status == 0) { + started(); } else { - if (! do_auto_restart()) desired_state = ServiceState::STOPPED; - forceStop(); + failed_to_start(); } - service_set->processQueues(false); } - else { // SCRIPTED - if (service_state == ServiceState::STOPPING) { - if (did_exit && WEXITSTATUS(exit_status) == 0) { - stopped(); - } - else { - // ??? failed to stop! Let's log it as info: - if (did_exit) { - log(LogLevel::INFO, "Service ", service_name, " stop command failed with exit code ", WEXITSTATUS(exit_status)); - } - else if (was_signalled) { - log(LogLevel::INFO, "Serivice ", service_name, " stop command terminated due to signal ", WTERMSIG(exit_status)); - } - // Just assume that we stopped, so that any dependencies - // can be stopped: - stopped(); - } - service_set->processQueues(false); - } - else { // STARTING - if (exit_status == 0) { - started(); - } - else { - // failed to start - if (did_exit) { - log(LogLevel::ERROR, "Service ", service_name, " command failed with exit code ", WEXITSTATUS(exit_status)); - } - else if (was_signalled) { - log(LogLevel::ERROR, "Service ", service_name, " command terminated due to signal ", WTERMSIG(exit_status)); - } - failed_to_start(); - } - service_set->processQueues(true); - } + else if (service_state == ServiceState::STOPPING) { + // We won't log a non-zero exit status or termination due to signal here - + // we assume that the process died because we signalled it. + stopped(); + } + else if (smooth_recovery && service_state == ServiceState::STARTED && desired_state == ServiceState::STARTED) { + // TODO ensure a minimum time between restarts + // TODO if we are pinned-started then we should probably check + // that dependencies have started before trying to re-start the + // service process. + doing_recovery = true; + start_ps_process(); + return; + } + else { + if (! do_auto_restart()) desired_state = ServiceState::STOPPED; + forceStop(); } + service_set->processQueues(false); } rearm ServiceIoWatcher::fd_event(EventLoop_t &loop, int fd, int flags) noexcept @@ -299,7 +355,7 @@ rearm ServiceIoWatcher::fd_event(EventLoop_t &loop, int fd, int flags) noexcept if (sr->pid == -1) { // Somehow the process managed to complete before we even saw the status. - sr->handle_exit_status(); + sr->handle_exit_status(sr->exit_status); } } diff --git a/src/service.h b/src/service.h index 262ada8..5e12bb6 100644 --- a/src/service.h +++ b/src/service.h @@ -41,7 +41,7 @@ * in the STARTING state when waiting for dependencies to start or for the exec() call in * the forked child to complete and return a status. * - * Aquisition/release: + * Acquisition/release: * ------------------ * Each service has a dependent-count ("required_by"). This starts at 0, adds 1 if the * service has explicitly been started (i.e. "start_explicit" is true), and adds 1 for @@ -213,6 +213,7 @@ class ServiceRecord friend class ServiceChildWatcher; friend class ServiceIoWatcher; + protected: typedef std::string string; string service_name; @@ -239,8 +240,6 @@ class ServiceRecord bool pinned_started : 1; bool waiting_for_deps : 1; // if STARTING, whether we are waiting for dependencies (inc console) to start bool waiting_for_execstat : 1; // if we are waiting for exec status after fork() - 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 bool prop_require : 1; // require must be propagated @@ -280,7 +279,7 @@ class ServiceRecord string socket_path; // path to the socket for socket-activation service int socket_perms; // socket permissions ("mode") uid_t socket_uid = -1; // socket user id or -1 - gid_t socket_gid = -1; // sockget group id or -1 + gid_t socket_gid = -1; // socket group id or -1 // Implementation details @@ -305,7 +304,7 @@ class ServiceRecord ServiceRecord *next_in_stop_queue = nullptr; - private: + protected: // All dependents have stopped. void allDepsStopped(); @@ -332,7 +331,7 @@ class ServiceRecord static void process_child_callback(EventLoop_t *loop, ServiceChildWatcher *w, int revents) noexcept; - void handle_exit_status() noexcept; + virtual void handle_exit_status(int exit_status) noexcept; // A dependency has reached STARTED state void dependencyStarted() noexcept; @@ -401,8 +400,8 @@ class ServiceRecord ServiceRecord(ServiceSet *set, string name) : service_state(ServiceState::STOPPED), desired_state(ServiceState::STOPPED), auto_restart(false), pinned_stopped(false), pinned_started(false), waiting_for_deps(false), - waiting_for_execstat(false), doing_recovery(false), - start_explicit(false), prop_require(false), prop_release(false), prop_failure(false), + waiting_for_execstat(false), start_explicit(false), + prop_require(false), prop_release(false), prop_failure(false), force_stop(false), child_listener(this), child_status_listener(this) { service_set = set; @@ -435,7 +434,9 @@ class ServiceRecord } } - // TODO write a destructor + virtual ~ServiceRecord() noexcept + { + } // begin transition from stopped to started state or vice versa depending on current and desired state void execute_transition() noexcept; @@ -555,6 +556,46 @@ class ServiceRecord } }; +class process_service : public ServiceRecord +{ + virtual void handle_exit_status(int exit_status) noexcept override; + + public: + process_service(ServiceSet *sset, string name, string &&command, + std::list> &command_offsets, + sr_list * pdepends_on, sr_list * pdepends_soft) + : ServiceRecord(sset, name, ServiceType::PROCESS, std::move(command), command_offsets, + pdepends_on, pdepends_soft) + { + } + + ~process_service() noexcept + { + } +}; + +class bgproc_service : public ServiceRecord +{ + virtual void handle_exit_status(int exit_status) noexcept override; + + bool doing_recovery : 1; // if we are currently recovering a BGPROCESS (restarting process, while + // holding STARTED service state) + + public: + bgproc_service(ServiceSet *sset, string name, string &&command, + std::list> &command_offsets, + sr_list * pdepends_on, sr_list * pdepends_soft) + : ServiceRecord(sset, name, ServiceType::BGPROCESS, std::move(command), command_offsets, + pdepends_on, pdepends_soft) + { + doing_recovery = false; + } + + ~bgproc_service() noexcept + { + } +}; + /* * A ServiceSet, as the name suggests, manages a set of services. * -- 2.25.1