From 370fc509d9e9c8b47c216fd9de917911597a4541 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Sun, 14 Jan 2018 23:34:21 +0000 Subject: [PATCH] Stub out system functions for testing (WIP). This is a more thorough and should be an overall better approach than stubbing/mocking the base_proc_service class itself. This commit introduces a baseproc-sys.h header which holds aliases for the system functions that are used (kill, fcntl, etc). When running tests the functions will be mocked (currently they are rigged to abort when called). --- src/baseproc-service.cc | 22 +-- src/includes/baseproc-sys.h | 9 + src/tests/Makefile | 12 +- src/tests/test-baseproc.cc | 235 ------------------------- src/tests/test-includes/baseproc-sys.h | 31 ++++ src/tests/test-run-child-proc.cc | 9 + 6 files changed, 67 insertions(+), 251 deletions(-) create mode 100644 src/includes/baseproc-sys.h delete mode 100644 src/tests/test-baseproc.cc create mode 100644 src/tests/test-includes/baseproc-sys.h create mode 100644 src/tests/test-run-child-proc.cc diff --git a/src/baseproc-service.cc b/src/baseproc-service.cc index 1645b05..a424c2a 100644 --- a/src/baseproc-service.cc +++ b/src/baseproc-service.cc @@ -5,6 +5,8 @@ #include "dinit-socket.h" #include "proc-service.h" +#include "baseproc-sys.h" + /* * Base process implementation (base_process_service). * @@ -55,7 +57,7 @@ bool base_process_service::start_ps_process(const std::vector &cmd event_loop.get_time(last_start_time, clock_type::MONOTONIC); int pipefd[2]; - if (dasynq::pipe2(pipefd, O_CLOEXEC)) { + if (bp_sys::pipe2(pipefd, O_CLOEXEC)) { log(loglevel_t::ERROR, get_name(), ": can't create status check pipe: ", strerror(errno)); return false; } @@ -76,8 +78,8 @@ bool base_process_service::start_ps_process(const std::vector &cmd } // Make the server side socket close-on-exec: - int fdflags = fcntl(control_socket[0], F_GETFD); - fcntl(control_socket[0], F_SETFD, fdflags | FD_CLOEXEC); + int fdflags = bp_sys::fcntl(control_socket[0], F_GETFD); + bp_sys::fcntl(control_socket[0], F_SETFD, fdflags | FD_CLOEXEC); try { control_conn = new control_conn_t(event_loop, services, control_socket[0]); @@ -113,9 +115,9 @@ bool base_process_service::start_ps_process(const std::vector &cmd } else { // Parent process - close(pipefd[1]); // close the 'other end' fd + bp_sys::close(pipefd[1]); // close the 'other end' fd if (control_socket[1] != -1) { - close(control_socket[1]); + bp_sys::close(control_socket[1]); } pid = forkpid; @@ -134,13 +136,13 @@ bool base_process_service::start_ps_process(const std::vector &cmd delete control_conn; out_cs: - close(control_socket[0]); - close(control_socket[1]); + bp_sys::close(control_socket[0]); + bp_sys::close(control_socket[1]); } out_p: - close(pipefd[0]); - close(pipefd[1]); + bp_sys::close(pipefd[0]); + bp_sys::close(pipefd[1]); return false; } @@ -312,7 +314,7 @@ void base_process_service::kill_pg(int signo) noexcept log(loglevel_t::ERROR, get_name(), ": can't signal process: ", strerror(errno)); return; } - kill(-pgid, signo); + bp_sys::kill(-pgid, signo); } void base_process_service::timer_expired() noexcept diff --git a/src/includes/baseproc-sys.h b/src/includes/baseproc-sys.h new file mode 100644 index 0000000..05ebbd3 --- /dev/null +++ b/src/includes/baseproc-sys.h @@ -0,0 +1,9 @@ +namespace bp_sys { + +using dasynq::pipe2; + +using ::fcntl; +using ::close; +using ::kill; + +} diff --git a/src/tests/Makefile b/src/tests/Makefile index a49dc4a..b4b1153 100644 --- a/src/tests/Makefile +++ b/src/tests/Makefile @@ -1,7 +1,7 @@ -include ../../mconfig -objects = tests.o test-dinit.o proctests.o test-baseproc.o -parent_objs = service.o proc-service.o dinit-log.o load_service.o # baseproc-service.o +objects = tests.o test-dinit.o proctests.o test-run-child-proc.o +parent_objs = service.o proc-service.o dinit-log.o load_service.o baseproc-service.o check: build-tests ./tests @@ -16,11 +16,11 @@ prepare-incdir: cd includes; ln -sf ../../includes/*.h . cd includes; ln -sf ../test-includes/*.h . -tests: prepare-incdir $(parent_objs) tests.o test-dinit.o test-baseproc.o - $(CXX) $(SANITIZEOPTS) -o tests $(parent_objs) tests.o test-dinit.o test-baseproc.o $(EXTRA_LIBS) +tests: prepare-incdir $(parent_objs) tests.o test-dinit.o + $(CXX) $(SANITIZEOPTS) -o tests $(parent_objs) tests.o test-dinit.o $(EXTRA_LIBS) -proctests: prepare-incdir $(parent_objs) proctests.o test-dinit.o test-baseproc.o - $(CXX) $(SANITIZEOPTS) -o proctests $(parent_objs) proctests.o test-dinit.o test-baseproc.o $(EXTRA_LIBS) +proctests: prepare-incdir $(parent_objs) proctests.o test-dinit.o + $(CXX) $(SANITIZEOPTS) -o proctests $(parent_objs) proctests.o test-dinit.o $(EXTRA_LIBS) $(objects): %.o: %.cc $(CXX) $(CXXOPTS) $(SANITIZEOPTS) -Iincludes -I../dasynq -c $< -o $@ diff --git a/src/tests/test-baseproc.cc b/src/tests/test-baseproc.cc deleted file mode 100644 index c1a4103..0000000 --- a/src/tests/test-baseproc.cc +++ /dev/null @@ -1,235 +0,0 @@ -#include "dinit.h" -#include "proc-service.h" - -// This is a mock implementation of the base_process_service class, for testing purposes. - -base_process_service::base_process_service(service_set *sset, string name, - service_type_t service_type_p, string &&command, - std::list> &command_offsets, - const std::list &deplist_p) - : service_record(sset, name, service_type_p, deplist_p), child_listener(this), - child_status_listener(this), restart_timer(this) -{ - program_name = std::move(command); - exec_arg_parts = separate_args(program_name, command_offsets); - - restart_interval_count = 0; - restart_interval_time = {0, 0}; - restart_timer.service = this; - //restart_timer.add_timer(event_loop); - - // By default, allow a maximum of 3 restarts within 10.0 seconds: - restart_interval.seconds() = 10; - restart_interval.nseconds() = 0; - max_restart_interval_count = 3; - - waiting_restart_timer = false; - reserved_child_watch = false; - tracking_child = false; - stop_timer_armed = false; - start_is_interruptible = false; -} - -bool base_process_service::bring_up() noexcept -{ - if (restarting) { - if (pid == -1) { - return restart_ps_process(); - } - return true; - } - else { - //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)) { - if (start_timeout != time_val(0,0)) { - //restart_timer.arm_timer_rel(event_loop, start_timeout); - stop_timer_armed = true; - } - else if (stop_timer_armed) { - //restart_timer.stop_timer(event_loop); - stop_timer_armed = false; - } - return true; - } - return false; - } -} - -void base_process_service::bring_down() noexcept -{ - waiting_for_deps = false; - if (pid != -1) { - // The process is still kicking on - must actually kill it. We signal the process - // group (-pid) rather than just the process as there's less risk then of creating - // an orphaned process group: - if (! onstart_flags.no_sigterm) { - //kill_pg(SIGTERM); - } - if (term_signal != -1) { - //kill_pg(term_signal); - } - - // In most cases, the rest is done in handle_exit_status. - // If we are a BGPROCESS and the process is not our immediate child, however, that - // won't work - check for this now: - if (get_type() == service_type_t::BGPROCESS && ! tracking_child) { - stopped(); - } - else if (stop_timeout != time_val(0,0)) { - //restart_timer.arm_timer_rel(event_loop, stop_timeout); - stop_timer_armed = true; - } - } - else { - // The process is already dead. - stopped(); - } -} - -void base_process_service::do_smooth_recovery() noexcept -{ - if (! restart_ps_process()) { - emergency_stop(); - services->process_queues(); - } -} - -bool base_process_service::start_ps_process(const std::vector &cmd, bool on_console) noexcept -{ - return false; -} - -void base_process_service::kill_with_fire() noexcept -{ - if (pid != -1) { - //log(loglevel_t::WARN, "Service ", get_name(), " with pid ", pid, " exceeded allowed stop time; killing."); - //kill_pg(SIGKILL); - } -} - -void base_process_service::kill_pg(int signo) noexcept -{ - //pid_t pgid = getpgid(pid); - //if (pgid == -1) { - // only should happen if pid is invalid, which should never happen... - //log(loglevel_t::ERROR, get_name(), ": can't signal process: ", strerror(errno)); - //return; - //} - //kill(-pgid, signo); -} - -bool base_process_service::restart_ps_process() noexcept -{ - using time_val = dasynq::time_val; - - time_val current_time; - event_loop.get_time(current_time, clock_type::MONOTONIC); - - if (max_restart_interval_count != 0) { - // Check whether we're still in the most recent restart check interval: - time_val int_diff = current_time - restart_interval_time; - if (int_diff < restart_interval) { - if (restart_interval_count >= max_restart_interval_count) { - //log(loglevel_t::ERROR, "Service ", get_name(), " restarting too quickly; stopping."); - return false; - } - } - else { - restart_interval_time = current_time; - restart_interval_count = 0; - } - } - - // Check if enough time has lapsed since the prevous restart. If not, start a timer: - time_val tdiff = current_time - last_start_time; - if (restart_delay <= tdiff) { - // > restart delay (normally 200ms) - do_restart(); - } - else { - time_val timeout = restart_delay - tdiff; - //restart_timer.arm_timer_rel(event_loop, timeout); - waiting_restart_timer = true; - } - return true; -} - -void base_process_service::do_restart() noexcept -{ - waiting_restart_timer = false; - restart_interval_count++; - auto service_state = get_state(); - - // We may be STARTING (regular restart) or STARTED ("smooth recovery"). This affects whether - // the process should be granted access to the console: - bool on_console = service_state == service_state_t::STARTING - ? onstart_flags.starts_on_console : onstart_flags.runs_on_console; - - if (service_state == service_state_t::STARTING) { - // for a smooth recovery, we want to check dependencies are available before actually - // starting: - if (! check_deps_started()) { - waiting_for_deps = true; - return; - } - } - - if (! start_ps_process(exec_arg_parts, on_console)) { - restarting = false; - if (service_state == service_state_t::STARTING) { - failed_to_start(); - } - else { - // desired_state = service_state_t::STOPPED; - forced_stop(); - } - services->process_queues(); - } -} - -bool base_process_service::interrupt_start() noexcept -{ - if (waiting_restart_timer) { - //restart_timer.stop_timer(event_loop); - waiting_restart_timer = false; - return service_record::interrupt_start(); - } - else { - //log(loglevel_t::WARN, "Interrupting start of service ", get_name(), " with pid ", pid, " (with SIGINT)."); - kill_pg(SIGINT); - if (stop_timeout != time_val(0,0)) { - restart_timer.arm_timer_rel(event_loop, stop_timeout); - stop_timer_armed = true; - } - else if (stop_timer_armed) { - restart_timer.stop_timer(event_loop); - stop_timer_armed = false; - } - set_state(service_state_t::STOPPING); - notify_listeners(service_event_t::STARTCANCELLED); - return false; - } -} - -void base_process_service::timer_expired() noexcept -{ - stop_timer_armed = false; - - // Timer expires if: - // We are stopping, including after having startup cancelled (stop timeout, state is STOPPING); We are - // starting (start timeout, state is STARTING); We are waiting for restart timer before restarting, - // including smooth recovery (restart timeout, state is STARTING or STARTED). - if (get_state() == service_state_t::STOPPING) { - kill_with_fire(); - } - else if (pid != -1) { - // Starting, start timed out. - stop_dependents(); - interrupt_start(); - } - else { - // STARTING / STARTED, and we have a pid: must be restarting (smooth recovery if STARTED) - do_restart(); - } -} diff --git a/src/tests/test-includes/baseproc-sys.h b/src/tests/test-includes/baseproc-sys.h new file mode 100644 index 0000000..b12c495 --- /dev/null +++ b/src/tests/test-includes/baseproc-sys.h @@ -0,0 +1,31 @@ +#include + +// Mock system functions for testing. + +namespace bp_sys { + +inline int pipe2(int pipefd[2], int flags) +{ + abort(); + return 0; +} + +inline int fcntl(int fd, int cmd, ...) +{ + // This is used for setting the CLOEXEC flag, we can just return 0: + return 0; +} + +inline int close(int fd) +{ + abort(); + return 0; +} + +inline int kill(pid_t pid, int sig) +{ + abort(); + return 0; +} + +} diff --git a/src/tests/test-run-child-proc.cc b/src/tests/test-run-child-proc.cc new file mode 100644 index 0000000..3cd81fc --- /dev/null +++ b/src/tests/test-run-child-proc.cc @@ -0,0 +1,9 @@ +#include "service.h" + +// 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 +{ + +} -- 2.25.1