Move some data/functions from service_record to base_process_service.
authorDavin McCall <davmac@davmac.org>
Fri, 19 Jan 2018 18:00:35 +0000 (18:00 +0000)
committerDavin McCall <davmac@davmac.org>
Fri, 19 Jan 2018 18:00:35 +0000 (18:00 +0000)
src/baseproc-service.cc
src/includes/proc-service.h
src/includes/service.h
src/load_service.cc
src/run-child-proc.cc
src/service.cc
src/tests/test-run-child-proc.cc

index 87f6088ef69182d873c22783b62407e423d5fbd6..ca16b1bcddfa675e45dfdbe8983d0a6268ca792f 100644 (file)
@@ -1,5 +1,8 @@
 #include <cstring>
 
+#include <sys/un.h>
+#include <sys/socket.h>
+
 #include "dinit.h"
 #include "dinit-log.h"
 #include "dinit-socket.h"
@@ -30,6 +33,10 @@ bool base_process_service::bring_up() noexcept
         return true;
     }
     else {
+        if (! open_socket()) {
+            return false;
+        }
+
         event_loop.get_time(restart_interval_time, clock_type::MONOTONIC);
         restart_interval_count = 0;
         if (start_ps_process(exec_arg_parts, onstart_flags.starts_on_console)) {
@@ -111,7 +118,7 @@ bool base_process_service::start_ps_process(const std::vector<const char *> &cmd
     }
 
     if (forkpid == 0) {
-        run_child_proc(cmd.data(), logfile, on_console, pipefd[1], control_socket[1]);
+        run_child_proc(cmd.data(), logfile, on_console, pipefd[1], control_socket[1], socket_fd);
     }
     else {
         // Parent process
@@ -336,3 +343,101 @@ void base_process_service::timer_expired() noexcept
         do_restart();
     }
 }
+
+void base_process_service::emergency_stop() noexcept
+{
+    if (! do_auto_restart() && start_explicit) {
+        start_explicit = false;
+        release(false);
+    }
+    forced_stop();
+    stop_dependents();
+    stopped();
+}
+
+void base_process_service::becoming_inactive() noexcept
+{
+    if (socket_fd != -1) {
+        close(socket_fd);
+        socket_fd = -1;
+    }
+}
+
+bool base_process_service::open_socket() noexcept
+{
+    if (socket_path.empty() || socket_fd != -1) {
+        // No socket, or already open
+        return true;
+    }
+
+    const char * saddrname = socket_path.c_str();
+
+    // Check the specified socket path
+    struct stat stat_buf;
+    if (stat(saddrname, &stat_buf) == 0) {
+        if ((stat_buf.st_mode & S_IFSOCK) == 0) {
+            // Not a socket
+            log(loglevel_t::ERROR, get_name(), ": Activation socket file exists (and is not a socket)");
+            return false;
+        }
+    }
+    else if (errno != ENOENT) {
+        // Other error
+        log(loglevel_t::ERROR, get_name(), ": Error checking activation socket: ", strerror(errno));
+        return false;
+    }
+
+    // Remove stale socket file (if it exists).
+    // We won't test the return from unlink - if it fails other than due to ENOENT, we should get an
+    // error when we try to create the socket anyway.
+    unlink(saddrname);
+
+    uint sockaddr_size = offsetof(struct sockaddr_un, sun_path) + socket_path.length() + 1;
+    struct sockaddr_un * name = static_cast<sockaddr_un *>(malloc(sockaddr_size));
+    if (name == nullptr) {
+        log(loglevel_t::ERROR, get_name(), ": Opening activation socket: out of memory");
+        return false;
+    }
+
+    name->sun_family = AF_UNIX;
+    strcpy(name->sun_path, saddrname);
+
+    int sockfd = dinit_socket(AF_UNIX, SOCK_STREAM, 0, SOCK_NONBLOCK | SOCK_CLOEXEC);
+    if (sockfd == -1) {
+        log(loglevel_t::ERROR, get_name(), ": Error creating activation socket: ", strerror(errno));
+        free(name);
+        return false;
+    }
+
+    if (bind(sockfd, (struct sockaddr *) name, sockaddr_size) == -1) {
+        log(loglevel_t::ERROR, get_name(), ": Error binding activation socket: ", strerror(errno));
+        close(sockfd);
+        free(name);
+        return false;
+    }
+
+    free(name);
+
+    // POSIX (1003.1, 2013) says that fchown and fchmod don't necessarily work on sockets. We have to
+    // use chown and chmod instead.
+    if (chown(saddrname, socket_uid, socket_gid)) {
+        log(loglevel_t::ERROR, get_name(), ": Error setting activation socket owner/group: ", strerror(errno));
+        close(sockfd);
+        return false;
+    }
+
+    if (chmod(saddrname, socket_perms) == -1) {
+        log(loglevel_t::ERROR, get_name(), ": Error setting activation socket permissions: ", strerror(errno));
+        close(sockfd);
+        return false;
+    }
+
+    if (listen(sockfd, 128) == -1) { // 128 "seems reasonable".
+        log(loglevel_t::ERROR, ": Error listening on activation socket: ", strerror(errno));
+        close(sockfd);
+        return false;
+    }
+
+    socket_fd = sockfd;
+    return true;
+}
index 6a5831c20c979a2bdc3d801d61bc98b3bf6e3ddb..2705811b69b2812f103d74f19123e8c8a9ee1094 100644 (file)
@@ -60,6 +60,13 @@ class base_process_service : public service_record
     // <stop_timeout>). 0 to disable.
     time_val start_timeout = {60, 0}; // default of 1 minute
 
+    pid_t pid = -1;  // PID of the process. If state is STARTING or STOPPING,
+                     //   this is PID of the service script; otherwise it is the
+                     //   PID of the process itself (process service).
+    int exit_status = 0; // Exit status, if the process has exited (pid == -1).
+    int socket_fd = -1;  // For socket-activation services, this is the file
+                         // descriptor for the socket.
+
     bool waiting_restart_timer : 1;
     bool stop_timer_armed : 1;
     bool reserved_child_watch : 1;
@@ -103,12 +110,20 @@ class base_process_service : public service_record
 
     virtual bool interrupt_start() noexcept override;
 
+    void becoming_inactive() noexcept override;
+
     // Kill with SIGKILL
     void kill_with_fire() noexcept;
 
     // Signal the process group of the service process
     void kill_pg(int signo) noexcept;
 
+    // stop immediately
+    void emergency_stop() noexcept;
+
+    // Open the activation socket, return false on failure
+    bool open_socket() noexcept;
+
     public:
     // Constructor for a base_process_service. Note that the various parameters not specified here must in
     // general be set separately (using the appropriate set_xxx function for each).
@@ -153,6 +168,12 @@ class base_process_service : public service_record
         start_is_interruptible = value;
     }
 
+    // Set an additional signal (other than SIGTERM) to be used to terminate the process
+    void set_extra_termination_signal(int signo) noexcept
+    {
+        this->term_signal = signo;
+    }
+
     // The restart/stop timer expired.
     void timer_expired() noexcept;
 };
index be9d94c40c16b62e7300e02fa56bf36ad8e47223..1b2bd4efd7c15df4518cce27a07ce97ca905da8c 100644 (file)
@@ -307,15 +307,6 @@ class service_record
     uid_t socket_uid = -1;  // socket user id or -1
     gid_t socket_gid = -1;  // socket group id or -1
 
-    // Implementation details
-    
-    pid_t pid = -1;  // PID of the process. If state is STARTING or STOPPING,
-                     //   this is PID of the service script; otherwise it is the
-                     //   PID of the process itself (process service).
-    int exit_status; // Exit status, if the process has exited (pid == -1).
-    int socket_fd = -1;  // For socket-activation services, this is the file
-                         // descriptor for the socket.
-    
     // Data for use by service_set
     public:
     
@@ -327,10 +318,7 @@ class service_record
     lls_node<service_record> stop_queue_node;
     
     protected:
-    
-    // stop immediately
-    void emergency_stop() noexcept;
-    
+
     // Service has actually stopped (includes having all dependents
     // reaching STOPPED state).
     void stopped() noexcept;
@@ -343,16 +331,13 @@ class service_record
     void failed_to_start(bool dep_failed = false) noexcept;
 
     void run_child_proc(const char * const *args, const char *logfile, bool on_console, int wpipefd,
-            int csfd) noexcept;
+            int csfd, int socket_fd) noexcept;
     
     // A dependency has reached STARTED state
     void dependency_started() noexcept;
     
     void all_deps_started() noexcept;
 
-    // Open the activation socket, return false on failure
-    bool open_socket() noexcept;
-
     // Start all dependencies, return true if all have started
     bool start_check_dependencies() noexcept;
 
@@ -442,6 +427,10 @@ class service_record
     // issued but service has not yet responded (state will be set to STOPPING).
     virtual bool interrupt_start() noexcept;
 
+    // The service is becoming inactive - i.e. it has stopped and will not be immediately restarted. Perform
+    // any appropriate cleanup.
+    virtual void becoming_inactive() noexcept { }
+
     public:
 
     service_record(service_set *set, string name)
@@ -456,7 +445,6 @@ class service_record
         service_name = name;
         record_type = service_type_t::DUMMY;
         socket_perms = 0;
-        exit_status = 0;
     }
 
     service_record(service_set *set, string name, service_type_t record_type_p,
@@ -519,13 +507,7 @@ class service_record
     {
         this->onstart_flags = flags;
     }
-    
-    // Set an additional signal (other than SIGTERM) to be used to terminate the process
-    void set_extra_termination_signal(int signo) noexcept
-    {
-        this->term_signal = signo;
-    }
-    
+
     void set_pid_file(string &&pid_file) noexcept
     {
         this->pid_file = std::move(pid_file);
index 7944cb3d07e20fa98591da9185bad80557bc3c86..82dae1a701f5db640649fdf392ad0417bede28a3 100644 (file)
@@ -613,6 +613,7 @@ service_record * dirload_service_set::load_service(const char * name)
                     rvalps->set_stop_timeout(stop_timeout);
                     rvalps->set_start_timeout(start_timeout);
                     rvalps->set_start_interruptible(start_is_interruptible);
+                    rvalps->set_extra_termination_signal(term_signal);
                     rval = rvalps;
                 }
                 else if (service_type == service_type_t::BGPROCESS) {
@@ -624,6 +625,7 @@ service_record * dirload_service_set::load_service(const char * name)
                     rvalps->set_stop_timeout(stop_timeout);
                     rvalps->set_start_timeout(start_timeout);
                     rvalps->set_start_interruptible(start_is_interruptible);
+                    rvalps->set_extra_termination_signal(term_signal);
                     rval = rvalps;
                 }
                 else if (service_type == service_type_t::SCRIPTED) {
@@ -633,6 +635,7 @@ service_record * dirload_service_set::load_service(const char * name)
                     rvalps->set_stop_timeout(stop_timeout);
                     rvalps->set_start_timeout(start_timeout);
                     rvalps->set_start_interruptible(start_is_interruptible);
+                    rvalps->set_extra_termination_signal(term_signal);
                     rval = rvalps;
                 }
                 else {
@@ -642,7 +645,6 @@ service_record * dirload_service_set::load_service(const char * name)
                 rval->set_auto_restart(auto_restart);
                 rval->set_smooth_recovery(smooth_recovery);
                 rval->set_flags(onstart_flags);
-                rval->set_extra_termination_signal(term_signal);
                 rval->set_socket_details(std::move(socket_path), socket_perms, socket_uid, socket_gid);
                 *iter = rval;
                 break;
index 0154a099a442c7bc23277d103388448bcfa227fc..66f51994f45663d0633e5479f9c6e64bc6806deb 100644 (file)
@@ -10,7 +10,7 @@
 #include "service.h"
 
 void service_record::run_child_proc(const char * const *args, const char *logfile, bool on_console,
-        int wpipefd, int csfd) noexcept
+        int wpipefd, int csfd, int socket_fd) noexcept
 {
     // Child process. Must not allocate memory (or otherwise risk throwing any exception)
     // from here until exit().
index e5351edee6a2fe2c7ba3cb904fefca4fd7004a9f..710d9e5672f7ecc864e0c606bf7afc27819be650 100644 (file)
@@ -1,6 +1,5 @@
 #include <cstring>
 #include <cerrno>
-#include <sstream>
 #include <iterator>
 #include <memory>
 #include <cstddef>
@@ -8,8 +7,6 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/ioctl.h>
-#include <sys/un.h>
-#include <sys/socket.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <termios.h>
@@ -91,10 +88,7 @@ void service_record::stopped() noexcept
         start(false);
     }
     else {
-        if (socket_fd != -1) {
-            close(socket_fd);
-            socket_fd = -1;
-        }
+        becoming_inactive();
         
         if (start_explicit) {
             start_explicit = false;
@@ -109,7 +103,6 @@ void service_record::stopped() noexcept
     notify_listeners(service_event_t::STOPPED);
 }
 
-
 bool service_record::do_auto_restart() noexcept
 {
     if (auto_restart) {
@@ -118,18 +111,6 @@ bool service_record::do_auto_restart() noexcept
     return false;
 }
 
-void service_record::emergency_stop() noexcept
-{
-    if (! do_auto_restart() && start_explicit) {
-        start_explicit = false;
-        release(false);
-    }
-    forced_stop();
-    stop_dependents();
-    stopped();
-}
-
-
 void service_record::require() noexcept
 {
     if (required_by++ == 0) {
@@ -311,85 +292,6 @@ bool service_record::check_deps_started() noexcept
     return true;
 }
 
-bool service_record::open_socket() noexcept
-{
-    if (socket_path.empty() || socket_fd != -1) {
-        // No socket, or already open
-        return true;
-    }
-    
-    const char * saddrname = socket_path.c_str();
-    
-    // Check the specified socket path
-    struct stat stat_buf;
-    if (stat(saddrname, &stat_buf) == 0) {
-        if ((stat_buf.st_mode & S_IFSOCK) == 0) {
-            // Not a socket
-            log(loglevel_t::ERROR, service_name, ": Activation socket file exists (and is not a socket)");
-            return false;
-        }
-    }
-    else if (errno != ENOENT) {
-        // Other error
-        log(loglevel_t::ERROR, service_name, ": Error checking activation socket: ", strerror(errno));
-        return false;
-    }
-
-    // Remove stale socket file (if it exists).
-    // We won't test the return from unlink - if it fails other than due to ENOENT, we should get an
-    // error when we try to create the socket anyway.
-    unlink(saddrname);
-
-    uint sockaddr_size = offsetof(struct sockaddr_un, sun_path) + socket_path.length() + 1;
-    struct sockaddr_un * name = static_cast<sockaddr_un *>(malloc(sockaddr_size));
-    if (name == nullptr) {
-        log(loglevel_t::ERROR, service_name, ": Opening activation socket: out of memory");
-        return false;
-    }
-
-    name->sun_family = AF_UNIX;
-    strcpy(name->sun_path, saddrname);
-
-    int sockfd = dinit_socket(AF_UNIX, SOCK_STREAM, 0, SOCK_NONBLOCK | SOCK_CLOEXEC);
-    if (sockfd == -1) {
-        log(loglevel_t::ERROR, service_name, ": Error creating activation socket: ", strerror(errno));
-        free(name);
-        return false;
-    }
-
-    if (bind(sockfd, (struct sockaddr *) name, sockaddr_size) == -1) {
-        log(loglevel_t::ERROR, service_name, ": Error binding activation socket: ", strerror(errno));
-        close(sockfd);
-        free(name);
-        return false;
-    }
-    
-    free(name);
-    
-    // POSIX (1003.1, 2013) says that fchown and fchmod don't necessarily work on sockets. We have to
-    // use chown and chmod instead.
-    if (chown(saddrname, socket_uid, socket_gid)) {
-        log(loglevel_t::ERROR, service_name, ": Error setting activation socket owner/group: ", strerror(errno));
-        close(sockfd);
-        return false;
-    }
-    
-    if (chmod(saddrname, socket_perms) == -1) {
-        log(loglevel_t::ERROR, service_name, ": Error setting activation socket permissions: ", strerror(errno));
-        close(sockfd);
-        return false;
-    }
-
-    if (listen(sockfd, 128) == -1) { // 128 "seems reasonable".
-        log(loglevel_t::ERROR, ": Error listening on activation socket: ", strerror(errno));
-        close(sockfd);
-        return false;
-    }
-    
-    socket_fd = sockfd;
-    return true;
-}
-
 void service_record::all_deps_started() noexcept
 {
     if (onstart_flags.starts_on_console && ! have_console) {
@@ -404,10 +306,6 @@ void service_record::all_deps_started() noexcept
         return;
     }
 
-    if (! open_socket()) {
-        failed_to_start();
-    }
-
     bool start_success = bring_up();
     if (! start_success) {
         failed_to_start();
index 3cd81fcdc818925feb20770d01091d7b408a8aba..db4dc32fd8dc301751e86c406a2591ccf04bd386 100644 (file)
@@ -3,7 +3,7 @@
 // Stub out run_child_proc function, for testing purposes.
 
 void service_record::run_child_proc(const char * const *args, const char *logfile, bool on_console,
-        int wpipefd, int csfd) noexcept
+        int wpipefd, int csfd, int socket_fd) noexcept
 {
 
 }