From 6e97cee14ac1055a928869832ff728309f3d0b12 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 1 Sep 2015 16:50:44 +0100 Subject: [PATCH] Clean up exception throwing/handling during service loading, and fix dependency cycle detection. --- dinit.cc | 9 ++++--- load_service.cc | 45 +++++++++++++++++++------------ service.h | 72 ++++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 93 insertions(+), 33 deletions(-) diff --git a/dinit.cc b/dinit.cc index 6fb730a..d54e8d6 100644 --- a/dinit.cc +++ b/dinit.cc @@ -180,11 +180,11 @@ int main(int argc, char **argv) } catch (ServiceNotFound &snf) { // TODO log this better - std::cerr << "Could not find service: " << snf.serviceName << endl; + cerr << "Could not find service description: " << snf.serviceName << endl; } - catch (std::string err) { - std::cerr << err << std::endl; - throw err; + catch (ServiceLoadExc &sle) { + // TODO log this better + cerr << "Problem loading service description: " << sle.serviceName << endl; } } @@ -196,6 +196,7 @@ int main(int argc, char **argv) } if (am_system_init) { + // TODO log this output properly cout << "dinit: No more active services."; if (reboot) { cout << " Will reboot."; diff --git a/load_service.cc b/load_service.cc index 324a66b..9360403 100644 --- a/load_service.cc +++ b/load_service.cc @@ -88,8 +88,8 @@ static string read_setting_value(string_iterator * const i, string_iterator end) // Find a service record, or load it from file. If the service has // dependencies, load those also. // -// Might throw an exception if a dependency cycle is found or if another -// problem occurs (I/O error, service description not found). +// Might throw a ServiceLoadExc exception if a dependency cycle is found or if another +// problem occurs (I/O error, service description not found etc). ServiceRecord * ServiceSet::loadServiceRecord(const char * name) { using std::string; @@ -102,6 +102,9 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name) // First try and find an existing record... ServiceRecord * rval = findService(string(name)); if (rval != 0) { + if (rval->isDummy()) { + throw ServiceCyclicDependency(name); + } return rval; } @@ -127,11 +130,13 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name) service_file.open(service_filename.c_str(), ios::in); } catch (std::ios_base::failure &exc) { - ServiceNotFound snf; - snf.serviceName = name; - throw snf; + throw ServiceNotFound(name); } + // Add a dummy service record now to prevent cyclic dependencies + rval = new ServiceRecord(this, string(name)); + records.push_back(rval); + // getline can set failbit if it reaches end-of-file, we don't want an exception in that case: service_file.exceptions(ios::badbit); @@ -148,8 +153,7 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name) string setting = read_setting_name(&i, end); i = skipws(i, end); if (i == end || *i != '=') { - // TODO: throw a documented exception - throw std::string("Badly formed line."); + throw ServiceDescriptionExc(name, "Badly formed line."); } i = skipws(++i, end); @@ -176,25 +180,32 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name) service_type = SVC_PROCESS; } else { - throw string("Service type must be \"scripted\"" + throw ServiceDescriptionExc(name, "Service type must be \"scripted\"" " or \"process\""); - // TODO throw a better exception } } else { - // TODO throw a better exception - throw string("Unknown setting"); + throw ServiceDescriptionExc(name, "Unknown setting: " + setting); } } } + service_file.close(); // TODO check we actually have all the settings - type, command - rval = new ServiceRecord(this, string(name), service_type, command, - &depends_on); - rval->setLogfile(logfile); - rval->setAutoRestart(auto_restart); - - records.push_back(rval); + // Now replace the dummy service record with a real record: + for (auto iter = records.begin(); iter != records.end(); iter++) { + if (*iter == rval) { + // We've found the dummy record + delete rval; + rval = new ServiceRecord(this, string(name), service_type, command, + &depends_on); + rval->setLogfile(logfile); + rval->setAutoRestart(auto_restart); + *iter = rval; + break; + } + } + return rval; } diff --git a/service.h b/service.h index 1b09dcf..599b4ee 100644 --- a/service.h +++ b/service.h @@ -4,24 +4,60 @@ #include "ev.h" /* Possible service states */ -#define SVC_STOPPED 0 /* service is not running */ -#define SVC_STARTING 1 /* service script is running with "start" */ -#define SVC_STARTED 2 /* service is running; start script finished. */ -#define SVC_STOPPING 3 /* service script is running with "stop" */ +// TODO can we use typesafe enum? +constexpr static int SVC_STOPPED = 0; // service is not running +constexpr static int SVC_STARTING = 1; // service script is running with "start" +constexpr static int SVC_STARTED = 2; // service is running; start script finished. +constexpr static int SVC_STOPPING = 3; // service script is running with "stop" + /* Service types */ -#define SVC_PROCESS 0 /* service runs as a process, and can be stopped +#define SVC_DUMMY 0 /* dummy service, used to detect cyclic dependencies */ +#define SVC_PROCESS 1 /* service runs as a process, and can be stopped by sending the process a signal */ -#define SVC_SCRIPTED 1 /* service requires a command to start, and another +#define SVC_SCRIPTED 2 /* service requires a command to start, and another command to stop */ - -// Exception -class ServiceNotFound +// Exception loading service +class ServiceLoadExc { public: std::string serviceName; + + ServiceLoadExc(std::string serviceName) + : serviceName(serviceName) + { + } +}; + +class ServiceNotFound : public ServiceLoadExc +{ + public: + ServiceNotFound(std::string serviceName) + : ServiceLoadExc(serviceName) + { + } +}; + +class ServiceCyclicDependency : public ServiceLoadExc +{ + public: + ServiceCyclicDependency(std::string serviceName) + : ServiceLoadExc(serviceName) + { + } +}; + +class ServiceDescriptionExc : public ServiceLoadExc +{ + public: + std::string extraInfo; + + ServiceDescriptionExc(std::string serviceName, std::string extraInfo) + : ServiceLoadExc(serviceName), extraInfo(extraInfo) + { + } }; @@ -91,12 +127,19 @@ class ServiceRecord void forceStop(); // force-stop this service and all dependents public: + + ServiceRecord(ServiceSet *set, string name) + : service_state(SVC_STOPPED), desired_state(SVC_STOPPED) + { + service_set = set; + service_name = name; + service_type = SVC_DUMMY; + } + ServiceRecord(ServiceSet *set, string name, int service_type, string command, std::list * pdepends_on) + : service_state(SVC_STOPPED), desired_state(SVC_STOPPED) { - service_state = SVC_STOPPED; - desired_state = SVC_STOPPED; - service_set = set; service_name = name; this->service_type = service_type; @@ -129,6 +172,11 @@ class ServiceRecord void start(); // start the service void stop(); // stop the service + + bool isDummy() + { + return service_type == SVC_DUMMY; + } }; -- 2.25.1