Refactoring: create base_process_service

This serves as a base for the three service types which start processes.

diff --git a/src/load_service.cc b/src/load_service.cc
index 7cf862e77fe011533b0bba7a11c4d0ed22350173..61ed0e77ccbedeb00540a3f653cc21348c09c756 100644 (file)
--- a/src/load_service.cc
+++ b/src/load_service.cc
@@ -513,6 +513,7 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name)
                 else if (service_type == ServiceType::BGPROCESS) {
                     rval = new bgproc_service(this, string(name), std::move(command),
                         command_offsets, &depends_on, &depends_soft);
+                    rval->set_pid_file(std::move(pid_file));
                 }
                 else if (service_type == ServiceType::SCRIPTED) {
                     rval = new scripted_service(this, string(name), std::move(command),
@@ -528,7 +529,6 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name)
                 rval->setSmoothRecovery(smooth_recovery);
                 rval->setOnstartFlags(onstart_flags);
                 rval->setExtraTerminationSignal(term_signal);
-                rval->set_pid_file(std::move(pid_file));
                 rval->set_socket_details(std::move(socket_path), socket_perms, socket_uid, socket_gid);
                 *iter = rval;
                 break;

diff --git a/src/service.cc b/src/service.cc
index c5a60f375c1c9a2c5e349772b39ee671fda557ae..466a523b746cd0e08ccb94a148fef1353c02a7c2 100644 (file)
--- a/src/service.cc
+++ b/src/service.cc
@@ -124,7 +124,7 @@ void ServiceRecord::stopped() noexcept
 
 dasynq::rearm ServiceChildWatcher::child_status(EventLoop_t &loop, pid_t child, int status) noexcept
 {
-    ServiceRecord *sr = service;
+    base_process_service *sr = service;
     
     sr->pid = -1;
     sr->exit_status = status;
@@ -155,7 +155,7 @@ bool ServiceRecord::do_auto_restart() noexcept
     return false;
 }
 
-void ServiceRecord::handle_exit_status(int exit_status) noexcept
+void base_process_service::handle_exit_status(int exit_status) noexcept
 {
     // TODO make abstract
 }
@@ -225,7 +225,7 @@ void bgproc_service::handle_exit_status(int exit_status) noexcept
         }
         else {
             // We need to re-read the PID, since it has now changed.
-            if (service_type == ServiceType::BGPROCESS && pid_file.length() != 0) {
+            if (pid_file.length() != 0) {
                 if (! read_pid_file()) {
                     need_stop = true;
                 }
@@ -246,7 +246,12 @@ void bgproc_service::handle_exit_status(int exit_status) noexcept
         // 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();
+            if (pid_file.length() != 0 && ! read_pid_file()) {
+                failed_to_start();
+            }
+            else {
+                started();
+            }
         }
         else {
             failed_to_start();
@@ -316,7 +321,7 @@ void scripted_service::handle_exit_status(int exit_status) noexcept
 
 rearm ServiceIoWatcher::fd_event(EventLoop_t &loop, int fd, int flags) noexcept
 {
-    ServiceRecord *sr = service;
+    base_process_service *sr = service;
     sr->waiting_for_execstat = false;
     
     int exec_status;
@@ -456,7 +461,7 @@ void ServiceRecord::do_propagation() noexcept
         }
         else if (service_state == ServiceState::STOPPING) {
             if (stopCheckDependents()) {
-                allDepsStopped();
+                all_deps_stopped();
             }
         }
     }
@@ -664,7 +669,7 @@ void ServiceRecord::acquiredConsole() noexcept
     }
 }
 
-bool ServiceRecord::read_pid_file() noexcept
+bool base_process_service::read_pid_file() noexcept
 {
     const char *pid_file_c = pid_file.c_str();
     int fd = open(pid_file_c, O_CLOEXEC);
@@ -695,13 +700,6 @@ bool ServiceRecord::read_pid_file() noexcept
 
 void ServiceRecord::started() noexcept
 {
-    if (service_type == ServiceType::BGPROCESS && pid_file.length() != 0) {
-        if (! read_pid_file()) {
-            failed_to_start();
-            return;
-        }
-    }
-
     if (onstart_flags.runs_on_console && (service_type == ServiceType::SCRIPTED || service_type == ServiceType::BGPROCESS)) {
         tcsetpgrp(0, getpgrp());
         releaseConsole();
@@ -768,11 +766,16 @@ void ServiceRecord::failed_to_start(bool depfailed) noexcept
 }
 
 bool ServiceRecord::start_ps_process() noexcept
+{
+    return true;
+}
+
+bool base_process_service::start_ps_process() noexcept
 {
     return start_ps_process(exec_arg_parts, onstart_flags.runs_on_console);
 }
 
-bool ServiceRecord::start_ps_process(const std::vector<const char *> &cmd, bool on_console) noexcept
+bool base_process_service::start_ps_process(const std::vector<const char *> &cmd, bool on_console) noexcept
 {
     // In general, you can't tell whether fork/exec is successful. We use a pipe to communicate
     // success/failure from the child to the parent. The pipe is set CLOEXEC so a successful
@@ -1045,7 +1048,7 @@ void ServiceRecord::do_stop() noexcept
 
     // If we get here, we are in STARTED state; stop all dependents.
     if (stopDependents()) {
-        allDepsStopped();
+        all_deps_stopped();
     }
 }
 
@@ -1081,51 +1084,54 @@ bool ServiceRecord::stopDependents() noexcept
 }
 
 // All dependents have stopped; we can stop now, too. Only called when STOPPING.
-void ServiceRecord::allDepsStopped()
+void ServiceRecord::all_deps_stopped() noexcept
 {
     waiting_for_deps = false;
-    if (service_type == ServiceType::PROCESS || service_type == ServiceType::BGPROCESS) {
-        if (pid != -1) {
-            // The process is still kicking on - must actually kill it.
-            if (! onstart_flags.no_sigterm) {
-                kill(pid, SIGTERM);
-            }
-            if (term_signal != -1) {
-                kill(pid, term_signal);
-            }
-            
-            // In most cases, the rest is done in process_child_callback.
-            // If we are a BGPROCESS and the process is not our immediate child, however, that
-            // won't work - check for this now:
-            if (service_type == ServiceType::BGPROCESS) {
-                int status;
-                pid_t r = waitpid(pid, &status, WNOHANG);
-                if (r == -1 && errno == ECHILD) {
-                    // We can't track this child (or it's terminated already)
-                    stopped();
-                }
-                else if (r == pid) {
-                    // TODO, examine status and log anything unusual.
-                    stopped();
-                }
-            }
-        }
-        else {
-            // The process is already dead.
-            stopped();
+    stopped();
+}
+
+void base_process_service::all_deps_stopped() noexcept
+{
+    waiting_for_deps = false;
+    if (pid != -1) {
+        // The process is still kicking on - must actually kill it.
+        if (! onstart_flags.no_sigterm) {
+            kill(pid, SIGTERM);
         }
-    }
-    else if (service_type == ServiceType::SCRIPTED) {
-        // Scripted service.
-        if (stop_command.length() == 0) {
-            stopped();
+        if (term_signal != -1) {
+            kill(pid, term_signal);
         }
-        else if (! start_ps_process(stop_arg_parts, false)) {
-            // Couldn't execute stop script, but there's not much we can do:
-            stopped();
+
+        // In most cases, the rest is done in process_child_callback.
+        // If we are a BGPROCESS and the process is not our immediate child, however, that
+        // won't work - check for this now:
+        if (service_type == ServiceType::BGPROCESS) {
+            int status;
+            pid_t r = waitpid(pid, &status, WNOHANG);
+            if (r == -1 && errno == ECHILD) {
+                // We can't track this child (or it's terminated already)
+                stopped();
+            }
+            else if (r == pid) {
+                // TODO, examine status and log anything unusual.
+                stopped();
+            }
         }
     }
     else {
+        // The process is already dead.
+        stopped();
+    }
+}
+
+void scripted_service::all_deps_stopped() noexcept
+{
+    waiting_for_deps = false;
+    if (stop_command.length() == 0) {
+        stopped();
+    }
+    else if (! start_ps_process(stop_arg_parts, false)) {
+        // Couldn't execute stop script, but there's not much we can do:
         stopped();
     }
 }

diff --git a/src/service.h b/src/service.h
index 136f408b8d00fa1b6f86837da7a30c7b030b416d..b4bc8150248cfc9e8a0dfa1b0860b39ac22ca4bf 100644 (file)
--- a/src/service.h
+++ b/src/service.h
@@ -137,8 +137,9 @@ class ServiceDescriptionExc : public ServiceLoadExc
     }    
 };
 
-class ServiceRecord; // forward declaration
-class ServiceSet; // forward declaration
+class ServiceRecord;
+class ServiceSet;
+class base_process_service;
 
 /* Service dependency record */
 class ServiceDep
@@ -193,26 +194,23 @@ static std::vector<const char *> separate_args(std::string &s, std::list<std::pa
 class ServiceChildWatcher : public EventLoop_t::child_proc_watcher_impl<ServiceChildWatcher>
 {
     public:
-    ServiceRecord * service;
+    base_process_service * service;
     rearm child_status(EventLoop_t &eloop, pid_t child, int status) noexcept;
     
-    ServiceChildWatcher(ServiceRecord * sr) noexcept : service(sr) { }
+    ServiceChildWatcher(base_process_service * sr) noexcept : service(sr) { }
 };
 
 class ServiceIoWatcher : public EventLoop_t::fd_watcher_impl<ServiceIoWatcher>
 {
     public:
-    ServiceRecord * service;
+    base_process_service * service;
     rearm fd_event(EventLoop_t &eloop, int fd, int flags) noexcept;
     
-    ServiceIoWatcher(ServiceRecord * sr) noexcept : service(sr) { }
+    ServiceIoWatcher(base_process_service * sr) noexcept : service(sr) { }
 };
 
 class ServiceRecord
 {
-    friend class ServiceChildWatcher;
-    friend class ServiceIoWatcher;
-    
     protected:
     typedef std::string string;
     
@@ -290,8 +288,6 @@ class ServiceRecord
     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:
@@ -307,7 +303,7 @@ class ServiceRecord
     protected:
     
     // All dependents have stopped.
-    void allDepsStopped();
+    virtual void all_deps_stopped() noexcept;
     
     // Service has actually stopped (includes having all dependents
     // reaching STOPPED state).
@@ -320,10 +316,6 @@ class ServiceRecord
     //   dep_failed: whether failure is recorded due to a dependency failing
     void failed_to_start(bool dep_failed = false) noexcept;
 
-    // For process services, start the process, return true on success
-    bool start_ps_process() noexcept;
-    bool start_ps_process(const std::vector<const char *> &args, bool on_console) noexcept;
-    
     void run_child_proc(const char * const *args, const char *logfile, bool on_console, int wpipefd,
             int csfd) noexcept;
     
@@ -331,16 +323,16 @@ class ServiceRecord
     static void process_child_callback(EventLoop_t *loop, ServiceChildWatcher *w,
             int revents) noexcept;
     
-    virtual void handle_exit_status(int exit_status) noexcept;
+    //virtual void handle_exit_status(int exit_status) noexcept;
 
     // A dependency has reached STARTED state
     void dependencyStarted() noexcept;
     
     void allDepsStarted(bool haveConsole = false) noexcept;
-    
-    // Read the pid-file, return false on failure
-    bool read_pid_file() noexcept;
-    
+
+    // Do any post-dependency startup; return false on failure
+    virtual bool start_ps_process() noexcept;
+
     // Open the activation socket, return false on failure
     bool open_socket() noexcept;
 
@@ -394,7 +386,10 @@ class ServiceRecord
     void releaseConsole() noexcept;
     
     bool do_auto_restart() noexcept;
-    
+
+    // Started state reached
+    bool process_started() noexcept;
+
     public:
 
     ServiceRecord(ServiceSet *set, string name)
@@ -402,7 +397,7 @@ class ServiceRecord
             pinned_stopped(false), pinned_started(false), waiting_for_deps(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)
+            force_stop(false)
     {
         service_set = set;
         service_name = name;
@@ -556,7 +551,40 @@ class ServiceRecord
     }
 };
 
-class process_service : public ServiceRecord
+class base_process_service : public ServiceRecord
+{
+    friend class ServiceChildWatcher;
+    friend class ServiceIoWatcher;
+
+    protected:
+    ServiceChildWatcher child_listener;
+    ServiceIoWatcher child_status_listener;
+
+    // start the process, return true on success
+    virtual bool start_ps_process() noexcept;
+    bool start_ps_process(const std::vector<const char *> &args, bool on_console) noexcept;
+
+    virtual void all_deps_stopped() noexcept;
+    virtual void handle_exit_status(int exit_status) noexcept;
+
+    // Read the pid-file, return false on failure
+    bool read_pid_file() noexcept;
+
+    public:
+    base_process_service(ServiceSet *sset, string name, ServiceType service_type, string &&command,
+            std::list<std::pair<unsigned,unsigned>> &command_offsets,
+            sr_list * pdepends_on, sr_list * pdepends_soft)
+         : ServiceRecord(sset, name, service_type, std::move(command), command_offsets,
+             pdepends_on, pdepends_soft), child_listener(this), child_status_listener(this)
+    {
+    }
+
+    ~base_process_service() noexcept
+    {
+    }
+};
+
+class process_service : public base_process_service
 {
     virtual void handle_exit_status(int exit_status) noexcept override;
 
@@ -564,7 +592,7 @@ class process_service : public ServiceRecord
     process_service(ServiceSet *sset, string name, string &&command,
             std::list<std::pair<unsigned,unsigned>> &command_offsets,
             sr_list * pdepends_on, sr_list * pdepends_soft)
-         : ServiceRecord(sset, name, ServiceType::PROCESS, std::move(command), command_offsets,
+         : base_process_service(sset, name, ServiceType::PROCESS, std::move(command), command_offsets,
              pdepends_on, pdepends_soft)
     {
     }
@@ -574,7 +602,7 @@ class process_service : public ServiceRecord
     }
 };
 
-class bgproc_service : public ServiceRecord
+class bgproc_service : public base_process_service
 {
     virtual void handle_exit_status(int exit_status) noexcept override;
 
@@ -585,7 +613,7 @@ class bgproc_service : public ServiceRecord
     bgproc_service(ServiceSet *sset, string name, string &&command,
             std::list<std::pair<unsigned,unsigned>> &command_offsets,
             sr_list * pdepends_on, sr_list * pdepends_soft)
-         : ServiceRecord(sset, name, ServiceType::BGPROCESS, std::move(command), command_offsets,
+         : base_process_service(sset, name, ServiceType::BGPROCESS, std::move(command), command_offsets,
              pdepends_on, pdepends_soft)
     {
         doing_recovery = false;
@@ -596,15 +624,16 @@ class bgproc_service : public ServiceRecord
     }
 };
 
-class scripted_service : public ServiceRecord
+class scripted_service : public base_process_service
 {
+    virtual void all_deps_stopped() noexcept override;
     virtual void handle_exit_status(int exit_status) noexcept override;
 
     public:
     scripted_service(ServiceSet *sset, string name, string &&command,
             std::list<std::pair<unsigned,unsigned>> &command_offsets,
             sr_list * pdepends_on, sr_list * pdepends_soft)
-         : ServiceRecord(sset, name, ServiceType::SCRIPTED, std::move(command), command_offsets,
+         : base_process_service(sset, name, ServiceType::SCRIPTED, std::move(command), command_offsets,
              pdepends_on, pdepends_soft)
     {
     }