From 25c367a2eebe7c79f6957ff6eae5f255621e2f92 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Thu, 19 Nov 2015 22:03:51 +0000 Subject: [PATCH] Various exception handling fixes and 'noexcept' declarations --- control.cc | 16 +++-- control.h | 4 +- dinit.cc | 7 +- load_service.cc | 185 +++++++++++++++++++++++++----------------------- service.cc | 34 ++++----- service.h | 78 ++++++++++---------- 6 files changed, 174 insertions(+), 150 deletions(-) diff --git a/control.cc b/control.cc index d32a602..eefca49 100644 --- a/control.cc +++ b/control.cc @@ -34,8 +34,16 @@ void ControlConn::processPacket() string serviceName(iobuf + 3, (size_t) svcSize); if (pktType == DINIT_CP_STARTSERVICE) { // TODO do not allow services to be started during system shutdown - service_set->startService(serviceName.c_str()); - // TODO catch exceptions, error response + try { + service_set->startService(serviceName.c_str()); + // TODO ack response + } + catch (ServiceLoadExc &slexc) { + // TODO error response + } + catch (std::bad_alloc &baexc) { + // TODO error response + } } else { // TODO verify the named service exists? @@ -64,7 +72,7 @@ void ControlConn::processPacket() } } -void ControlConn::rollbackComplete() +void ControlConn::rollbackComplete() noexcept { char ackBuf[1] = { DINIT_RP_COMPLETED }; if (write(iob.fd, ackBuf, 1) == -1) { @@ -111,7 +119,7 @@ void ControlConn::dataReady() } } -ControlConn::~ControlConn() +ControlConn::~ControlConn() noexcept { close(iob.fd); ev_io_stop(loop, &iob); diff --git a/control.h b/control.h index 2454318..90c5f0d 100644 --- a/control.h +++ b/control.h @@ -53,10 +53,10 @@ class ControlConn } void processPacket(); - void rollbackComplete(); + void rollbackComplete() noexcept; void dataReady(); - ~ControlConn(); + ~ControlConn() noexcept; }; diff --git a/dinit.cc b/dinit.cc index e8697ea..7a9c0cd 100644 --- a/dinit.cc +++ b/dinit.cc @@ -72,7 +72,7 @@ static void sigint_reboot_cb(struct ev_loop *loop, ev_signal *w, int revents); static void sigquit_cb(struct ev_loop *loop, ev_signal *w, int revents); static void sigterm_cb(struct ev_loop *loop, ev_signal *w, int revents); -void open_control_socket(struct ev_loop *loop); +void open_control_socket(struct ev_loop *loop) noexcept; struct ev_io control_socket_io; @@ -206,6 +206,9 @@ int main(int argc, char **argv) catch (ServiceLoadExc &sle) { log(LogLevel::ERROR, "Problem loading service description: ", sle.serviceName); } + catch (std::bad_alloc &badalloce) { + log(LogLevel::ERROR, "Out of memory when trying to start service: ", *i); + } } event_loop: @@ -291,7 +294,7 @@ static void control_socket_cb(struct ev_loop *loop, ev_io *w, int revents) } } -void open_control_socket(struct ev_loop *loop) +void open_control_socket(struct ev_loop *loop) noexcept { if (! control_socket_open) { // TODO make this use a per-user address if PID != 1, and make the address diff --git a/load_service.cc b/load_service.cc index 46887cd..87116ec 100644 --- a/load_service.cc +++ b/load_service.cc @@ -1,4 +1,5 @@ #include "service.h" +#include #include #include #include @@ -228,110 +229,118 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name) 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); - - while (! (service_file.rdstate() & ios::eofbit)) { - getline(service_file, line); - string::iterator i = line.begin(); - string::iterator end = line.end(); - - i = skipws(i, end); - if (i != end) { - if (*i == '#') { - continue; // comment line - } - string setting = read_setting_name(i, end); + try { + // getline can set failbit if it reaches end-of-file, we don't want an exception in that case: + service_file.exceptions(ios::badbit); + + while (! (service_file.rdstate() & ios::eofbit)) { + getline(service_file, line); + string::iterator i = line.begin(); + string::iterator end = line.end(); + i = skipws(i, end); - if (i == end || (*i != '=' && *i != ':')) { - throw ServiceDescriptionExc(name, "Badly formed line."); - } - i = skipws(++i, end); - - if (setting == "command") { - command = read_setting_value(i, end, &command_offsets); - } - else if (setting == "depends-on") { - string dependency_name = read_setting_value(i, end); - depends_on.push_back(loadServiceRecord(dependency_name.c_str())); - } - else if (setting == "waits-for") { - string dependency_name = read_setting_value(i, end); - depends_soft.push_back(loadServiceRecord(dependency_name.c_str())); - } - else if (setting == "logfile") { - logfile = read_setting_value(i, end); - } - else if (setting == "restart") { - string restart = read_setting_value(i, end); - auto_restart = (restart == "yes" || restart == "true"); - } - else if (setting == "type") { - string type_str = read_setting_value(i, end); - if (type_str == "scripted") { - service_type = ServiceType::SCRIPTED; + if (i != end) { + if (*i == '#') { + continue; // comment line } - else if (type_str == "process") { - service_type = ServiceType::PROCESS; + string setting = read_setting_name(i, end); + i = skipws(i, end); + if (i == end || (*i != '=' && *i != ':')) { + throw ServiceDescriptionExc(name, "Badly formed line."); } - else if (type_str == "internal") { - service_type = ServiceType::INTERNAL; + i = skipws(++i, end); + + if (setting == "command") { + command = read_setting_value(i, end, &command_offsets); } - else { - throw ServiceDescriptionExc(name, "Service type must be \"scripted\"" - " or \"process\" or \"internal\""); + else if (setting == "depends-on") { + string dependency_name = read_setting_value(i, end); + depends_on.push_back(loadServiceRecord(dependency_name.c_str())); } - } - else if (setting == "onstart") { - std::list> indices; - string onstart_cmds = read_setting_value(i, end, &indices); - for (auto indexpair : indices) { - string onstart_cmd = onstart_cmds.substr(indexpair.first, indexpair.second - indexpair.first); - if (onstart_cmd == "release_console") { - onstart_flags.release_console = true; + else if (setting == "waits-for") { + string dependency_name = read_setting_value(i, end); + depends_soft.push_back(loadServiceRecord(dependency_name.c_str())); + } + else if (setting == "logfile") { + logfile = read_setting_value(i, end); + } + else if (setting == "restart") { + string restart = read_setting_value(i, end); + auto_restart = (restart == "yes" || restart == "true"); + } + else if (setting == "type") { + string type_str = read_setting_value(i, end); + if (type_str == "scripted") { + service_type = ServiceType::SCRIPTED; + } + else if (type_str == "process") { + service_type = ServiceType::PROCESS; } - else if (onstart_cmd == "rw_ready") { - onstart_flags.rw_ready = true; + else if (type_str == "internal") { + service_type = ServiceType::INTERNAL; } else { - throw new ServiceDescriptionExc(name, "Unknown onstart command: " + onstart_cmd); + throw ServiceDescriptionExc(name, "Service type must be \"scripted\"" + " or \"process\" or \"internal\""); } } - } - else if (setting == "termsignal") { - string signame = read_setting_value(i, end, nullptr); - int signo = signalNameToNumber(signame); - if (signo == -1) { - throw new ServiceDescriptionExc(name, "Unknown/unsupported termination signal: " + signame); + else if (setting == "onstart") { + std::list> indices; + string onstart_cmds = read_setting_value(i, end, &indices); + for (auto indexpair : indices) { + string onstart_cmd = onstart_cmds.substr(indexpair.first, indexpair.second - indexpair.first); + if (onstart_cmd == "release_console") { + onstart_flags.release_console = true; + } + else if (onstart_cmd == "rw_ready") { + onstart_flags.rw_ready = true; + } + else { + throw new ServiceDescriptionExc(name, "Unknown onstart command: " + onstart_cmd); + } + } + } + else if (setting == "termsignal") { + string signame = read_setting_value(i, end, nullptr); + int signo = signalNameToNumber(signame); + if (signo == -1) { + throw new ServiceDescriptionExc(name, "Unknown/unsupported termination signal: " + signame); + } + else { + term_signal = signo; + } } else { - term_signal = signo; + throw ServiceDescriptionExc(name, "Unknown setting: " + setting); } } - else { - throw ServiceDescriptionExc(name, "Unknown setting: " + setting); + } + + service_file.close(); + // TODO check we actually have all the settings - type, command + + // 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, std::move(command), command_offsets, + & depends_on, & depends_soft); + rval->setLogfile(logfile); + rval->setAutoRestart(auto_restart); + rval->setOnstartFlags(onstart_flags); + rval->setExtraTerminationSignal(term_signal); + *iter = rval; + break; } } + + return rval; } - - service_file.close(); - // TODO check we actually have all the settings - type, command - - // 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, std::move(command), command_offsets, - & depends_on, & depends_soft); - rval->setLogfile(logfile); - rval->setAutoRestart(auto_restart); - rval->setOnstartFlags(onstart_flags); - rval->setExtraTerminationSignal(term_signal); - *iter = rval; - break; - } + catch (...) { + // Must remove the dummy service record. + std::remove(records.begin(), records.end(), rval); + delete rval; + throw; } - - return rval; } diff --git a/service.cc b/service.cc index c245a30..71ec4fc 100644 --- a/service.cc +++ b/service.cc @@ -11,12 +11,12 @@ #include "dinit-log.h" // from dinit.cc: -void open_control_socket(struct ev_loop *loop); +void open_control_socket(struct ev_loop *loop) noexcept; // Find the requested service by name static ServiceRecord * findService(const std::list & records, - const char *name) + const char *name) noexcept { using std::list; list::const_iterator i = records.begin(); @@ -28,7 +28,7 @@ static ServiceRecord * findService(const std::list & records, return (ServiceRecord *)0; } -ServiceRecord * ServiceSet::findService(std::string name) +ServiceRecord * ServiceSet::findService(std::string name) noexcept { return ::findService(records, name.c_str()); } @@ -41,7 +41,7 @@ void ServiceSet::startService(const char *name) record->start(); } -void ServiceSet::stopService(const std::string & name) +void ServiceSet::stopService(const std::string & name) noexcept { ServiceRecord *record = findService(name); if (record != nullptr) { @@ -50,7 +50,7 @@ void ServiceSet::stopService(const std::string & name) } // Called when a service has actually stopped. -void ServiceRecord::stopped() +void ServiceRecord::stopped() noexcept { logServiceStopped(service_name); service_state = ServiceState::STOPPED; @@ -70,7 +70,7 @@ void ServiceRecord::stopped() } } -void ServiceRecord::process_child_callback(struct ev_loop *loop, ev_child *w, int revents) +void ServiceRecord::process_child_callback(struct ev_loop *loop, ev_child *w, int revents) noexcept { ServiceRecord *sr = (ServiceRecord *) w->data; @@ -120,7 +120,7 @@ void ServiceRecord::process_child_callback(struct ev_loop *loop, ev_child *w, in } } -void ServiceRecord::start() +void ServiceRecord::start() noexcept { if ((service_state == ServiceState::STARTING || service_state == ServiceState::STARTED) && desired_state == ServiceState::STOPPED) { @@ -152,7 +152,7 @@ void ServiceRecord::start() allDepsStarted(); } -void ServiceRecord::dependencyStarted() +void ServiceRecord::dependencyStarted() noexcept { if (service_state != ServiceState::STARTING) { return; @@ -163,7 +163,7 @@ void ServiceRecord::dependencyStarted() } } -bool ServiceRecord::startCheckDependencies(bool start_deps) +bool ServiceRecord::startCheckDependencies(bool start_deps) noexcept { bool all_deps_started = true; @@ -206,7 +206,7 @@ bool ServiceRecord::startCheckDependencies(bool start_deps) return all_deps_started; } -void ServiceRecord::allDepsStarted() +void ServiceRecord::allDepsStarted() noexcept { if (service_type == ServiceType::PROCESS) { bool start_success = start_ps_process(); @@ -402,7 +402,7 @@ bool ServiceRecord::start_ps_process(const std::vector &pargs) noex } // Mark this and all dependent services as force-stopped. -void ServiceRecord::forceStop() +void ServiceRecord::forceStop() noexcept { force_stop = true; stop(); @@ -435,7 +435,7 @@ void ServiceRecord::failed_dependency() } } -void ServiceRecord::dependentStopped() +void ServiceRecord::dependentStopped() noexcept { if (service_state == ServiceState::STOPPING) { // Check the other dependents before we stop. @@ -445,7 +445,7 @@ void ServiceRecord::dependentStopped() } } -void ServiceRecord::stop() +void ServiceRecord::stop() noexcept { if ((service_state == ServiceState::STOPPING || service_state == ServiceState::STOPPED) && desired_state == ServiceState::STARTED) { @@ -479,7 +479,7 @@ void ServiceRecord::stop() } } -bool ServiceRecord::stopCheckDependents() +bool ServiceRecord::stopCheckDependents() noexcept { bool all_deps_stopped = true; for (sr_iter i = dependents.begin(); i != dependents.end(); ++i) { @@ -492,7 +492,7 @@ bool ServiceRecord::stopCheckDependents() return all_deps_stopped; } -bool ServiceRecord::stopDependents() +bool ServiceRecord::stopDependents() noexcept { bool all_deps_stopped = true; for (sr_iter i = dependents.begin(); i != dependents.end(); ++i) { @@ -536,12 +536,12 @@ void ServiceRecord::allDepsStopped() } } -void ServiceSet::service_active(ServiceRecord *sr) +void ServiceSet::service_active(ServiceRecord *sr) noexcept { active_services++; } -void ServiceSet::service_inactive(ServiceRecord *sr) +void ServiceSet::service_inactive(ServiceRecord *sr) noexcept { active_services--; if (active_services == 0 && rollback_handler != nullptr) { diff --git a/service.h b/service.h index 0dd330a..a5b3d13 100644 --- a/service.h +++ b/service.h @@ -50,7 +50,7 @@ struct OnstartFlags { bool release_console : 1; bool rw_ready : 1; - OnstartFlags() : release_console(false), rw_ready(false) + OnstartFlags() noexcept : release_console(false), rw_ready(false) { } }; @@ -61,7 +61,7 @@ class ServiceLoadExc public: std::string serviceName; - ServiceLoadExc(std::string serviceName) + ServiceLoadExc(std::string serviceName) noexcept : serviceName(serviceName) { } @@ -70,7 +70,7 @@ class ServiceLoadExc class ServiceNotFound : public ServiceLoadExc { public: - ServiceNotFound(std::string serviceName) + ServiceNotFound(std::string serviceName) noexcept : ServiceLoadExc(serviceName) { } @@ -79,7 +79,7 @@ class ServiceNotFound : public ServiceLoadExc class ServiceCyclicDependency : public ServiceLoadExc { public: - ServiceCyclicDependency(std::string serviceName) + ServiceCyclicDependency(std::string serviceName) noexcept : ServiceLoadExc(serviceName) { } @@ -90,7 +90,7 @@ class ServiceDescriptionExc : public ServiceLoadExc public: std::string extraInfo; - ServiceDescriptionExc(std::string serviceName, std::string extraInfo) + ServiceDescriptionExc(std::string serviceName, std::string extraInfo) noexcept : ServiceLoadExc(serviceName), extraInfo(extraInfo) { } @@ -109,15 +109,15 @@ class ServiceDep /* Whether the 'from' service is waiting for the 'to' service to start */ bool waiting_on; - ServiceDep(ServiceRecord * from, ServiceRecord * to) : from(from), to(to), waiting_on(false) + ServiceDep(ServiceRecord * from, ServiceRecord * to) noexcept : from(from), to(to), waiting_on(false) { } - ServiceRecord * getFrom() + ServiceRecord * getFrom() noexcept { return from; } - ServiceRecord * getTo() + ServiceRecord * getTo() noexcept { return to; } @@ -205,7 +205,7 @@ class ServiceRecord // Service has actually stopped (includes having all dependents // reaching STOPPED state). - void stopped(); + void stopped() noexcept; // Service has successfully started void started(); @@ -222,26 +222,26 @@ class ServiceRecord // Callback from libev when a child process dies static void process_child_callback(struct ev_loop *loop, struct ev_child *w, - int revents); + int revents) noexcept; // A dependency has reached STARTED state - void dependencyStarted(); + void dependencyStarted() noexcept; - void allDepsStarted(); + void allDepsStarted() noexcept; // Check whether dependencies have started, and optionally ask them to start - bool startCheckDependencies(bool do_start); + bool startCheckDependencies(bool do_start) noexcept; // A dependent has reached STOPPED state - void dependentStopped(); + void dependentStopped() noexcept; // check if all dependents have stopped - bool stopCheckDependents(); + bool stopCheckDependents() noexcept; // issue a stop to all dependents, return true if they are all already stopped - bool stopDependents(); + bool stopDependents() noexcept; - void forceStop(); // force-stop this service and all dependents + void forceStop() noexcept; // force-stop this service and all dependents public: @@ -288,30 +288,30 @@ class ServiceRecord } // Set whether this service should automatically restart when it dies - void setAutoRestart(bool auto_restart) + void setAutoRestart(bool auto_restart) noexcept { this->auto_restart = auto_restart; } // Set "on start" flags (commands) - void setOnstartFlags(OnstartFlags flags) + void setOnstartFlags(OnstartFlags flags) noexcept { this->onstart_flags = flags; } // Set an additional signal (other than SIGTERM) to be used to terminate the process - void setExtraTerminationSignal(int signo) + void setExtraTerminationSignal(int signo) noexcept { this->term_signal = signo; } - const char *getServiceName() const { return service_name.c_str(); } - ServiceState getState() const { return service_state; } + const char *getServiceName() const noexcept { return service_name.c_str(); } + ServiceState getState() const noexcept { return service_state; } - void start(); // start the service - void stop(); // stop the service + void start() noexcept; // start the service + void stop() noexcept; // stop the service - bool isDummy() + bool isDummy() noexcept { return service_type == ServiceType::DUMMY; } @@ -329,10 +329,13 @@ class ServiceSet // Private methods // Locate an existing service record. - ServiceRecord *findService(std::string name); + ServiceRecord *findService(std::string name) noexcept; // Load a service description, and dependencies, if there is no existing // record for the given name. + // Throws: + // ServiceLoadException (or subclass) on problem with service description + // std::bad_alloc on out-of-memory condition ServiceRecord *loadServiceRecord(const char *name); // Public @@ -348,30 +351,31 @@ class ServiceSet // Start the service with the given name. The named service will begin // transition to the 'started' state. // - // Throws an exception if the - // service description cannot be loaded. + // Throws a ServiceLoadException (or subclass) if the service description + // cannot be loaded or is invalid; + // Throws std::bad_alloc if out of memory. void startService(const char *name); // Stop the service with the given name. The named service will begin // transition to the 'stopped' state. - void stopService(const std::string &name); + void stopService(const std::string &name) noexcept; // Notification from service that it is active (state != STOPPED) // Only to be called on the transition from inactive to active. - void service_active(ServiceRecord *); + void service_active(ServiceRecord *) noexcept; // Notification from service that it is inactive (STOPPED) // Only to be called on the transition from active to inactive. - void service_inactive(ServiceRecord *); + void service_inactive(ServiceRecord *) noexcept; // Find out how many services are active (starting, running or stopping, // but not stopped). - int count_active_services() + int count_active_services() noexcept { return active_services; } - void stop_all_services() + void stop_all_services() noexcept { restart_enabled = false; for (std::list::iterator i = records.begin(); i != records.end(); ++i) { @@ -379,12 +383,12 @@ class ServiceSet } } - void set_auto_restart(bool restart) + void set_auto_restart(bool restart) noexcept { restart_enabled = restart; } - bool get_auto_restart() + bool get_auto_restart() noexcept { return restart_enabled; } @@ -392,7 +396,7 @@ class ServiceSet // Set the rollback handler, which will be notified when all services have stopped. // There can be only one rollback handler; attempts to set it when already set will // fail. Returns true if successful. - bool setRollbackHandler(ControlConn *conn) + bool setRollbackHandler(ControlConn *conn) noexcept { if (rollback_handler == nullptr) { rollback_handler = conn; @@ -403,7 +407,7 @@ class ServiceSet } } - void clearRollbackHandler(ControlConn *conn) + void clearRollbackHandler(ControlConn *conn) noexcept { if (rollback_handler == conn) { rollback_handler = nullptr; -- 2.25.1