Clean up exception throwing/handling during service loading, and fix

dependency cycle detection.

diff --git a/dinit.cc b/dinit.cc
index 6fb730aca1fef2591493e5e17dbbe1d2026c79fd..d54e8d69765c0e1c44001739b562c8f6ac1d4f54 100644 (file)
--- 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 324a66b7bcb312e0e4ec16bc0dab4b67d403101c..936040382b328e785cd7c950afe8abafe9c2641c 100644 (file)
--- 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 1b09dcfc85321c313bbab57f1b1999551dd455cb..599b4ee7afa0f3c73e25229cd228c1ee39c958df 100644 (file)
--- 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<ServiceRecord *> * 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;
+    }
 };