From f8693a9e47be002588d31a29a47cfecc57ec56ad Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Fri, 19 Jan 2018 18:00:35 +0000 Subject: [PATCH] Move some data/functions from service_record to base_process_service. --- src/baseproc-service.cc | 107 ++++++++++++++++++++++++++++++- src/includes/proc-service.h | 21 ++++++ src/includes/service.h | 32 ++------- src/load_service.cc | 4 +- src/run-child-proc.cc | 2 +- src/service.cc | 104 +----------------------------- src/tests/test-run-child-proc.cc | 2 +- 7 files changed, 140 insertions(+), 132 deletions(-) diff --git a/src/baseproc-service.cc b/src/baseproc-service.cc index 87f6088..ca16b1b 100644 --- a/src/baseproc-service.cc +++ b/src/baseproc-service.cc @@ -1,5 +1,8 @@ #include +#include +#include + #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 &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(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; +} diff --git a/src/includes/proc-service.h b/src/includes/proc-service.h index 6a5831c..2705811 100644 --- a/src/includes/proc-service.h +++ b/src/includes/proc-service.h @@ -60,6 +60,13 @@ class base_process_service : public service_record // ). 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; }; diff --git a/src/includes/service.h b/src/includes/service.h index be9d94c..1b2bd4e 100644 --- a/src/includes/service.h +++ b/src/includes/service.h @@ -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 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); diff --git a/src/load_service.cc b/src/load_service.cc index 7944cb3..82dae1a 100644 --- a/src/load_service.cc +++ b/src/load_service.cc @@ -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; diff --git a/src/run-child-proc.cc b/src/run-child-proc.cc index 0154a09..66f5199 100644 --- a/src/run-child-proc.cc +++ b/src/run-child-proc.cc @@ -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(). diff --git a/src/service.cc b/src/service.cc index e5351ed..710d9e5 100644 --- a/src/service.cc +++ b/src/service.cc @@ -1,6 +1,5 @@ #include #include -#include #include #include #include @@ -8,8 +7,6 @@ #include #include #include -#include -#include #include #include #include @@ -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(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(); diff --git a/src/tests/test-run-child-proc.cc b/src/tests/test-run-child-proc.cc index 3cd81fc..db4dc32 100644 --- a/src/tests/test-run-child-proc.cc +++ b/src/tests/test-run-child-proc.cc @@ -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 { } -- 2.25.1