From f1d6fa077bc3918e030fc3e2da2bc5efefdcda6a Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Fri, 16 Nov 2018 10:31:38 +0000 Subject: [PATCH] Backend support for S6-style readiness notification. No support in service configuration yet. --- src/baseproc-service.cc | 35 +++++++++++++- src/includes/proc-service.h | 49 ++++++++++++++++++- src/includes/service.h | 14 ++++-- src/proc-service.cc | 46 ++++++++++++++++-- src/run-child-proc.cc | 83 ++++++++++++++++++++++++++------ src/tests/test-run-child-proc.cc | 2 +- 6 files changed, 201 insertions(+), 28 deletions(-) diff --git a/src/baseproc-service.cc b/src/baseproc-service.cc index f7232be..c437185 100644 --- a/src/baseproc-service.cc +++ b/src/baseproc-service.cc @@ -86,6 +86,11 @@ bool base_process_service::start_ps_process(const std::vector &cmd control_conn_t *control_conn = nullptr; int control_socket[2] = {-1, -1}; + int notify_pipe[2] = {-1, -1}; + bool have_notify = !notification_var.empty() || force_notification_fd != -1; + ready_notify_watcher * rwatcher = have_notify ? get_ready_watcher() : nullptr; + bool ready_watcher_registered = false; + if (onstart_flags.pass_cs_fd) { if (dinit_socketpair(AF_UNIX, SOCK_STREAM, /* protocol */ 0, control_socket, SOCK_NONBLOCK)) { log(loglevel_t::ERROR, get_name(), ": can't create control socket: ", strerror(errno)); @@ -105,6 +110,27 @@ bool base_process_service::start_ps_process(const std::vector &cmd } } + if (have_notify) { + // Create a notification pipe: + if (bp_sys::pipe2(notify_pipe, 0) != 0) { + log(loglevel_t::ERROR, get_name(), ": can't create notification pipe: ", strerror(errno)); + goto out_cs_h; + } + + // Set the read side as close-on-exec: + int fdflags = bp_sys::fcntl(notify_pipe[0], F_GETFD); + bp_sys::fcntl(notify_pipe[0], F_SETFD, fdflags | FD_CLOEXEC); + + // add, but don't yet enable, readiness watcher: + try { + rwatcher->add_watch(event_loop, notify_pipe[0], dasynq::IN_EVENTS, false); + ready_watcher_registered = true; + } + catch (std::exception &exc) { + log(loglevel_t::ERROR, get_name(), ": can't add notification watch: ", exc.what()); + } + } + // Set up complete, now fork and exec: pid_t forkpid; @@ -129,7 +155,7 @@ bool base_process_service::start_ps_process(const std::vector &cmd const char * working_dir_c = nullptr; if (! working_dir.empty()) working_dir_c = working_dir.c_str(); run_child_proc(cmd.data(), working_dir_c, logfile, on_console, pipefd[1], control_socket[1], - socket_fd, run_as_uid, run_as_gid); + socket_fd, notify_pipe[0], force_notification_fd, nullptr, run_as_uid, run_as_gid); } else { // Parent process @@ -139,6 +165,7 @@ bool base_process_service::start_ps_process(const std::vector &cmd } pid = forkpid; + notification_fd = notify_pipe[0]; waiting_for_execstat = true; return true; } @@ -150,6 +177,12 @@ bool base_process_service::start_ps_process(const std::vector &cmd child_status_listener.deregister(event_loop); } + if (notify_pipe[0] != -1) bp_sys::close(notify_pipe[0]); + if (notify_pipe[1] != -1) bp_sys::close(notify_pipe[1]); + if (ready_watcher_registered) { + rwatcher->deregister(event_loop); + } + if (onstart_flags.pass_cs_fd) { delete control_conn; diff --git a/src/includes/proc-service.h b/src/includes/proc-service.h index 469fc67..be01767 100644 --- a/src/includes/proc-service.h +++ b/src/includes/proc-service.h @@ -40,9 +40,23 @@ class exec_status_pipe_watcher : public eventloop_t::fd_watcher_impl +{ + public: + base_process_service * service; + dasynq::rearm fd_event(eventloop_t &eloop, int fd, int flags) noexcept; + + ready_notify_watcher(base_process_service * sr) noexcept : service(sr) { } + + ready_notify_watcher(const ready_notify_watcher &) = delete; + void operator=(const ready_notify_watcher &) = delete; +}; + + class service_child_watcher : public eventloop_t::child_proc_watcher_impl { public: @@ -61,6 +75,7 @@ class base_process_service : public service_record friend class service_child_watcher; friend class exec_status_pipe_watcher; friend class base_process_service_test; + friend class ready_notify_watcher; private: // Re-launch process @@ -100,6 +115,8 @@ class base_process_service : public service_record uid_t run_as_uid = -1; gid_t run_as_gid = -1; + int force_notification_fd = -1; // if set, notification fd for service process is set to this fd + string notification_var; // if set, name of an environment variable for notification fd 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 @@ -107,6 +124,7 @@ class base_process_service : public service_record bp_sys::exit_status 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. + int notification_fd = -1; // If readiness notification is via fd bool waiting_restart_timer : 1; bool stop_timer_armed : 1; @@ -163,6 +181,12 @@ class base_process_service : public service_record // Open the activation socket, return false on failure bool open_socket() noexcept; + // Get the readiness notification watcher for this service, if it has one; may return nullptr. + virtual ready_notify_watcher *get_ready_watcher() noexcept + { + return nullptr; + } + 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). @@ -226,6 +250,19 @@ class base_process_service : public service_record working_dir = working_dir_p; } + // Set the notification fd number that the service process will use + void set_notification_fd(int fd) + { + force_notification_fd = fd; + } + + // Set the name of the environment variable that will be set to the notification fd number + // when the service process is run + void set_notification_var(string &&varname) + { + notification_var = std::move(varname); + } + // The restart/stop timer expired. void timer_expired() noexcept; @@ -254,12 +291,20 @@ class process_service : public base_process_service virtual void exec_succeeded() noexcept override; virtual void bring_down() noexcept override; + ready_notify_watcher readiness_watcher; + + protected: + ready_notify_watcher *get_ready_watcher() noexcept override + { + return &readiness_watcher; + } + public: process_service(service_set *sset, const string &name, string &&command, std::list> &command_offsets, const std::list &depends_p) : base_process_service(sset, name, service_type_t::PROCESS, std::move(command), command_offsets, - depends_p) + depends_p), readiness_watcher(this) { } diff --git a/src/includes/service.h b/src/includes/service.h index 5d34a00..aea9ca1 100644 --- a/src/includes/service.h +++ b/src/includes/service.h @@ -289,18 +289,26 @@ class service_record // immediate_stop: whether to set state as STOPPED and handle complete stop. void failed_to_start(bool dep_failed = false, bool immediate_stop = true) noexcept; - // Run a child process (call after forking). + // Run a child process (call after forking). Note that some arguments specify file descriptors, + // but in general file descriptors may be moved before the exec call. // - args specifies the program arguments including the executable (argv[0]) // - working_dir specifies the working directory; may be null - // - logfile specifies the logfile + // - logfile specifies the logfile (where stdout/stderr are directed) // - on_console: if true, process is run with access to console // - wpipefd: if the exec is unsuccessful, or another error occurs beforehand, the // error number (errno) is written to this file descriptor // - csfd: the control socket fd; may be -1 to inhibit passing of control socket // - socket_fd: the pre-opened socket file descriptor (may be -1) + // - notify_fd: the readiness notification fd; process should write to this descriptor when + // is is ready + // - force_notify_fd: if not -1, specifies the file descriptor that notify_fd should be moved + // to (via dup2 and close of the original). + // - notify_var: the name of an environment variable which will be set to contain the notification + // fd // - uid/gid: the identity to run the process as (may be both -1, otherwise both must be valid) void run_child_proc(const char * const *args, const char *working_dir, const char *logfile, - bool on_console, int wpipefd, int csfd, int socket_fd, uid_t uid, gid_t gid) noexcept; + bool on_console, int wpipefd, int csfd, int socket_fd, int notify_fd, int force_notify_fd, + const char *notify_var,uid_t uid, gid_t gid) noexcept; // A dependency has reached STARTED state void dependency_started() noexcept; diff --git a/src/proc-service.cc b/src/proc-service.cc index 2aa91ac..d46a50d 100644 --- a/src/proc-service.cc +++ b/src/proc-service.cc @@ -47,7 +47,13 @@ void process_service::exec_succeeded() noexcept // might be stopped (and killed via a signal) during smooth recovery. We don't to // process startup again in either case, so we check for state STARTING: if (get_state() == service_state_t::STARTING) { - started(); + if (force_notification_fd != -1 || !notification_var.empty()) { + // Wait for readiness notification: + readiness_watcher.set_enabled(event_loop, true); + } + else { + started(); + } } else if (get_state() == service_state_t::STOPPING) { // stopping, but smooth recovery was in process. That's now over so we can @@ -102,6 +108,31 @@ rearm exec_status_pipe_watcher::fd_event(eventloop_t &loop, int fd, int flags) n return rearm::REMOVED; } +rearm ready_notify_watcher::fd_event(eventloop_t &, int fd, int flags) noexcept +{ + char buf[128]; + if (service->get_state() == service_state_t::STARTING) { + // can we actually read anything from the notification pipe? + int r = bp_sys::read(fd, buf, sizeof(buf)); + if (r > 0) { + service->started(); + } + else if (r == 0 || errno != EAGAIN) { + service->failed_to_start(false, false); + } + } + else { + int r = bp_sys::read(fd, buf, sizeof(buf)); + if (r == 0) { + // Process closed write end or terminated + close(fd); + service->notification_fd = -1; + return rearm::DISARM; + } + } + return rearm::REARM; +} + dasynq::rearm service_child_watcher::status_change(eventloop_t &loop, pid_t child, int status) noexcept { base_process_service *sr = service; @@ -109,10 +140,9 @@ dasynq::rearm service_child_watcher::status_change(eventloop_t &loop, pid_t chil sr->pid = -1; sr->exit_status = bp_sys::exit_status(status); - // Ok, for a process service, any process death which we didn't rig - // ourselves is a bit... unexpected. Probably, the child died because - // we asked it to (sr->service_state == STOPPING). But even if - // we didn't, there's not much we can do. + // Ok, for a process service, any process death which we didn't rig ourselves is a bit... unexpected. + // Probably, the child died because we asked it to (sr->service_state == STOPPING). But even if we + // didn't, there's not much we can do. if (sr->waiting_for_execstat) { // We still don't have an exec() status from the forked child, wait for that @@ -140,6 +170,12 @@ void process_service::handle_exit_status(bp_sys::exit_status exit_status) noexce restarting = false; auto service_state = get_state(); + if (notification_fd != -1) { + readiness_watcher.deregister(event_loop); + bp_sys::close(notification_fd); + notification_fd = -1; + } + if (exit_status.did_exit_clean() && service_state != service_state_t::STOPPING) { if (did_exit) { log(loglevel_t::ERROR, "Service ", get_name(), " process terminated with exit code ", diff --git a/src/run-child-proc.cc b/src/run-child-proc.cc index 9ee2860..fa1cff1 100644 --- a/src/run-child-proc.cc +++ b/src/run-child-proc.cc @@ -1,4 +1,5 @@ #include +#include #include #include @@ -11,12 +12,27 @@ #include "service.h" +// Move an fd, if necessary, to another fd. The destination fd must be available (not open). +// if fd is specified as -1, returns -1 immediately. Returns 0 on success. +static int move_fd(int fd, int dest) +{ + if (fd == -1) return -1; + if (fd == dest) return 0; + + if (dup2(fd, dest) == -1) { + return -1; + } + + close(fd); + return 0; +} + void service_record::run_child_proc(const char * const *args, const char *working_dir, const char *logfile, bool on_console, int wpipefd, int csfd, int socket_fd, + int notify_fd, int force_notify_fd, const char *notify_var, uid_t uid, gid_t gid) noexcept { - // Child process. Must not allocate memory (or otherwise risk throwing any exception) - // from here until exit(). + // Child process. Must not risk throwing any uncaught exception from here until exit(). // If the console already has a session leader, presumably it is us. On the other hand // if it has no session leader, and we don't create one, then control inputs such as @@ -34,19 +50,31 @@ void service_record::run_child_proc(const char * const *args, const char *workin sigdelset(&sigwait_set, SIGTERM); sigdelset(&sigwait_set, SIGQUIT); - constexpr int bufsz = ((CHAR_BIT * sizeof(pid_t)) / 3 + 2) + 11; + constexpr int bufsz = 11 + ((CHAR_BIT * sizeof(pid_t) + 2) / 3) + 1; // "LISTEN_PID=" - 11 characters; the expression above gives a conservative estimate // on the maxiumum number of bytes required for LISTEN=nnn, including nul terminator, // where nnn is a pid_t in decimal (i.e. one decimal digit is worth just over 3 bits). char nbuf[bufsz]; // "DINIT_CS_FD=" - 12 bytes. (we -1 from sizeof(int) in account of sign bit). - constexpr int csenvbufsz = ((CHAR_BIT * sizeof(int) - 1) / 3 + 2) + 12; + constexpr int csenvbufsz = 12 + ((CHAR_BIT * sizeof(int) - 1 + 2) / 3) + 1; char csenvbuf[csenvbufsz]; int minfd = (socket_fd == -1) ? 3 : 4; - // Move wpipefd/csfd to another fd if necessary + // Move wpipefd/csfd/notifyfd to another fd if necessary + + // first allocate the forced notification fd, if specified: + if (force_notify_fd != -1) { + if (notify_fd != force_notify_fd) { + if (dup2(notify_fd, force_notify_fd) == -1) { + goto failure_out; + } + close(notify_fd); + notify_fd = force_notify_fd; + } + } + if (wpipefd < minfd) { wpipefd = fcntl(wpipefd, F_DUPFD_CLOEXEC, minfd); if (wpipefd == -1) goto failure_out; @@ -57,12 +85,28 @@ void service_record::run_child_proc(const char * const *args, const char *workin if (csfd == -1) goto failure_out; } + if (notify_fd < minfd && notify_fd != force_notify_fd) { + notify_fd = fcntl(notify_fd, F_DUPFD, minfd); + if (notify_fd == -1) goto failure_out; + } + + // Set up notify-fd variable + if (notify_var != nullptr && *notify_var != 0) { + // We need to do an allocation: the variable name length, '=', and space for the value, + // and nul terminator: + int notify_var_len = strlen(notify_var); + int req_sz = notify_var_len + ((CHAR_BIT * sizeof(int) - 1 + 2) / 3) + 1; + char * var_str = (char *) malloc(req_sz); + if (var_str == nullptr) goto failure_out; + snprintf(var_str, req_sz, "%s=%d", notify_var, notify_fd); + if (putenv(var_str)) goto failure_out; + } + + // Set up Systemd-style socket activation if (socket_fd != -1) { - // If we passing a pre-opened socket, it has to be fd number 3. (Thanks, systemd). + // If we passing a pre-opened socket, it has to be fd number 3. (Thanks, Systemd). if (dup2(socket_fd, 3) == -1) goto failure_out; - if (socket_fd != 3) { - close(socket_fd); - } + if (socket_fd != 3) close(socket_fd); if (putenv(const_cast("LISTEN_FDS=1"))) goto failure_out; snprintf(nbuf, bufsz, "LISTEN_PID=%jd", static_cast(getpid())); @@ -82,15 +126,22 @@ void service_record::run_child_proc(const char * const *args, const char *workin if (! on_console) { // Re-set stdin, stdout, stderr - close(0); close(1); close(2); + for (int i = 0; i < 3; i++) { + if (i != force_notify_fd) close(i); + } - if (open("/dev/null", O_RDONLY) == 0) { - // stdin = 0. That's what we should have; proceed with opening - // stdout and stderr. - if (open(logfile, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR) != 1) { - goto failure_out; + if (notify_fd == 0 || move_fd(open("/dev/null", O_RDONLY), 0) == 0) { + // stdin = 0. That's what we should have; proceed with opening stdout and stderr. We have to + // take care not to clobber the notify_fd. + if (notify_fd != 1) { + if (move_fd(open(logfile, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR), 1) != 0) { + goto failure_out; + } + if (notify_fd != 2 && dup2(1, 2) != 2) { + goto failure_out; + } } - if (dup2(1, 2) != 2) { + else if (move_fd(open(logfile, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR), 2) != 0) { goto failure_out; } } diff --git a/src/tests/test-run-child-proc.cc b/src/tests/test-run-child-proc.cc index bcad2d7..b387992 100644 --- a/src/tests/test-run-child-proc.cc +++ b/src/tests/test-run-child-proc.cc @@ -4,7 +4,7 @@ void service_record::run_child_proc(const char * const *args, const char *working_dir, const char *logfile, bool on_console, int wpipefd, int csfd, int socket_fd, - uid_t uid, gid_t gid) noexcept + int notify_fd, int forced_notify_fd, const char * notify_var, uid_t uid, gid_t gid) noexcept { } -- 2.25.1