proctests: add a new test, currently failing.

This also includes more OS abstraction in bp_sys so that we can test
different exit statuses.

diff --git a/src/includes/baseproc-sys.h b/src/includes/baseproc-sys.h
index 44da7628ce7dfb3c0f423a22bca1e83ccdc69a17..6bd0de4f906d70ad0a4c782c808078285b6fd7a1 100644 (file)
--- a/src/includes/baseproc-sys.h
+++ b/src/includes/baseproc-sys.h
@@ -5,6 +5,11 @@
  * for the functions, to avoid calling the real functions and thus allow for unit-level testing.
  */
 
+#ifndef BPSYS_INCLUDED
+#define BPSYS_INCLUDED
+
+#include "dasynq.h"
+
 namespace bp_sys {
 
 using dasynq::pipe2;
@@ -16,4 +21,48 @@ using ::getpgid;
 using ::tcsetpgrp;
 using ::getpgrp;
 
+// Wrapper around a POSIX exit status
+class exit_status
+{
+    friend pid_t waitpid(pid_t, exit_status *, int);
+
+    int status;
+
+    public:
+    exit_status() : status(0) { }
+    explicit exit_status(int status_p) : status(status_p) { }
+
+    bool did_exit()
+    {
+        return WIFEXITED(status);
+    }
+
+    bool did_exit_clean()
+    {
+        return status == 0;
+    }
+
+    bool was_signalled()
+    {
+        return WIFSIGNALED(status);
+    }
+
+    int get_exit_status()
+    {
+        return WEXITSTATUS(status);
+    }
+
+    int get_term_sig()
+    {
+        return WTERMSIG(status);
+    }
+};
+
+inline pid_t waitpid(pid_t p, exit_status *statusp, int flags)
+{
+    return ::waitpid(p, &statusp->status, flags);
 }
+
+}
+
+#endif  // BPSYS_INCLUDED

diff --git a/src/includes/proc-service.h b/src/includes/proc-service.h
index d734c7ed6a2486c8d62cffc8ab5f6a0ea45e4f0a..2ebe32eedf8d05a8a7404f3eabe3b58bd17e3529 100644 (file)
--- a/src/includes/proc-service.h
+++ b/src/includes/proc-service.h
@@ -1,5 +1,6 @@
 #include <sys/types.h>
 
+#include "baseproc-sys.h"
 #include "service.h"
 
 // This header defines base_proc_service (base process service) and several derivatives, as well as some
@@ -74,7 +75,7 @@ class base_process_service : public service_record
     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).
+    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.
 
@@ -99,7 +100,7 @@ class base_process_service : public service_record
 
     // Called when the process exits. The exit_status is the status value yielded by
     // the "wait" system call.
-    virtual void handle_exit_status(int exit_status) noexcept = 0;
+    virtual void handle_exit_status(bp_sys::exit_status exit_status) noexcept = 0;
 
     // Called if an exec fails.
     virtual void exec_failed(int errcode) noexcept = 0;
@@ -207,7 +208,7 @@ class base_process_service : public service_record
 // Standard process service.
 class process_service : public base_process_service
 {
-    virtual void handle_exit_status(int exit_status) noexcept override;
+    virtual void handle_exit_status(bp_sys::exit_status exit_status) noexcept override;
     virtual void exec_failed(int errcode) noexcept override;
     virtual void exec_succeeded() noexcept override;
     virtual void bring_down() noexcept override;
@@ -229,7 +230,7 @@ class process_service : public base_process_service
 // Bgproc (self-"backgrounding", i.e. double-forking) process service
 class bgproc_service : public base_process_service
 {
-    virtual void handle_exit_status(int exit_status) noexcept override;
+    virtual void handle_exit_status(bp_sys::exit_status exit_status) noexcept override;
     virtual void exec_failed(int errcode) noexcept override;
     virtual void bring_down() noexcept override;
 
@@ -240,7 +241,7 @@ class bgproc_service : public base_process_service
     };
 
     // Read the pid-file, return false on failure
-    pid_result_t read_pid_file(int *exit_status) noexcept;
+    pid_result_t read_pid_file(bp_sys::exit_status *exit_status) noexcept;
 
     public:
     bgproc_service(service_set *sset, string name, string &&command,
@@ -259,7 +260,7 @@ class bgproc_service : public base_process_service
 // Service which is started and stopped via separate commands
 class scripted_service : public base_process_service
 {
-    virtual void handle_exit_status(int exit_status) noexcept override;
+    virtual void handle_exit_status(bp_sys::exit_status exit_status) noexcept override;
     virtual void exec_failed(int errcode) noexcept override;
     virtual void bring_down() noexcept override;
 

diff --git a/src/proc-service.cc b/src/proc-service.cc
index 07ef7ee9f2dc38ab841819819411823b1ca610c2..d482fde7e3abaa182efb67fb40d5881cc4f4ae11 100644 (file)
--- a/src/proc-service.cc
+++ b/src/proc-service.cc
@@ -97,7 +97,7 @@ dasynq::rearm service_child_watcher::status_change(eventloop_t &loop, pid_t chil
     base_process_service *sr = service;
 
     sr->pid = -1;
-    sr->exit_status = status;
+    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
@@ -119,30 +119,30 @@ dasynq::rearm service_child_watcher::status_change(eventloop_t &loop, pid_t chil
         sr->stop_timer_armed = false;
     }
 
-    sr->handle_exit_status(status);
+    sr->handle_exit_status(bp_sys::exit_status(status));
     return dasynq::rearm::NOOP;
 }
 
-void process_service::handle_exit_status(int exit_status) noexcept
+void process_service::handle_exit_status(bp_sys::exit_status exit_status) noexcept
 {
-    bool did_exit = WIFEXITED(exit_status);
-    bool was_signalled = WIFSIGNALED(exit_status);
+    bool did_exit = exit_status.did_exit();
+    bool was_signalled = exit_status.was_signalled();
     restarting = false;
     auto service_state = get_state();
 
-    if (exit_status != 0 && service_state != service_state_t::STOPPING) {
+    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 ",
-                    WEXITSTATUS(exit_status));
+                    exit_status.get_exit_status());
         }
         else if (was_signalled) {
             log(loglevel_t::ERROR, "Service ", get_name(), " terminated due to signal ",
-                    WTERMSIG(exit_status));
+                    exit_status.get_term_sig());
         }
     }
 
     if (service_state == service_state_t::STARTING) {
-        if (did_exit && WEXITSTATUS(exit_status) == 0) {
+        if (exit_status.did_exit_clean()) {
             started();
         }
         else {
@@ -180,21 +180,21 @@ void process_service::exec_failed(int errcode) noexcept
     }
 }
 
-void bgproc_service::handle_exit_status(int exit_status) noexcept
+void bgproc_service::handle_exit_status(bp_sys::exit_status exit_status) noexcept
 {
     begin:
-    bool did_exit = WIFEXITED(exit_status);
-    bool was_signalled = WIFSIGNALED(exit_status);
+    bool did_exit = exit_status.did_exit();
+    bool was_signalled = exit_status.was_signalled();
     auto service_state = get_state();
 
-    if (exit_status != 0 && service_state != service_state_t::STOPPING) {
+    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 ",
-                    WEXITSTATUS(exit_status));
+                    exit_status.get_exit_status());
         }
         else if (was_signalled) {
             log(loglevel_t::ERROR, "Service ", get_name(), " terminated due to signal ",
-                    WTERMSIG(exit_status));
+                    exit_status.get_term_sig());
         }
     }
 
@@ -203,7 +203,7 @@ void bgproc_service::handle_exit_status(int exit_status) noexcept
     if (restarting && service_state == service_state_t::STARTED) {
         restarting = false;
         bool need_stop = false;
-        if ((did_exit && WEXITSTATUS(exit_status) != 0) || was_signalled) {
+        if ((did_exit && exit_status.get_exit_status() != 0) || was_signalled) {
             need_stop = true;
         }
         else {
@@ -236,7 +236,7 @@ void bgproc_service::handle_exit_status(int exit_status) noexcept
     if (service_state == service_state_t::STARTING) {
         // 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) {
+        if (exit_status.did_exit_clean()) {
             auto pid_result = read_pid_file(&exit_status);
             switch (pid_result) {
                 case pid_result_t::FAILED:
@@ -285,10 +285,10 @@ void bgproc_service::exec_failed(int errcode) noexcept
     failed_to_start();
 }
 
-void scripted_service::handle_exit_status(int exit_status) noexcept
+void scripted_service::handle_exit_status(bp_sys::exit_status exit_status) noexcept
 {
-    bool did_exit = WIFEXITED(exit_status);
-    bool was_signalled = WIFSIGNALED(exit_status);
+    bool did_exit = exit_status.did_exit();
+    bool was_signalled = exit_status.was_signalled();
     auto service_state = get_state();
 
     // For a scripted service, a termination occurs in one of three main cases:
@@ -301,16 +301,16 @@ void scripted_service::handle_exit_status(int exit_status) noexcept
         // a cancel order via SIGINT:
         if (interrupting_start) {
             // We issued a start interrupt, so we expected this failure:
-            if (did_exit && WEXITSTATUS(exit_status) != 0) {
+            if (did_exit && exit_status.get_exit_status() != 0) {
                 log(loglevel_t::INFO, "Service ", get_name(), " start cancelled; exit code ",
-                        WEXITSTATUS(exit_status));
+                        exit_status.get_exit_status());
                 // Assume that a command terminating normally requires no cleanup:
                 stopped();
             }
             else {
                 if (was_signalled) {
                     log(loglevel_t::INFO, "Service ", get_name(), " start cancelled from signal ",
-                            WTERMSIG(exit_status));
+                            exit_status.get_term_sig());
                 }
                 // If the start script completed successfully, or was interrupted via our signal,
                 // we want to run the stop script to clean up:
@@ -318,7 +318,7 @@ void scripted_service::handle_exit_status(int exit_status) noexcept
             }
             interrupting_start = false;
         }
-        else if (did_exit && WEXITSTATUS(exit_status) == 0) {
+        else if (exit_status.did_exit_clean()) {
             // We were running the stop script and finished successfully
             stopped();
         }
@@ -326,11 +326,11 @@ void scripted_service::handle_exit_status(int exit_status) noexcept
             // ??? failed to stop! Let's log it as warning:
             if (did_exit) {
                 log(loglevel_t::WARN, "Service ", get_name(), " stop command failed with exit code ",
-                        WEXITSTATUS(exit_status));
+                        exit_status.get_exit_status());
             }
             else if (was_signalled) {
                 log(loglevel_t::WARN, "Service ", get_name(), " stop command terminated due to signal ",
-                        WTERMSIG(exit_status));
+                        exit_status.get_term_sig());
             }
             // Even if the stop script failed, assume that service is now stopped, so that any dependencies
             // can be stopped. There's not really any other useful course of action here.
@@ -339,18 +339,18 @@ void scripted_service::handle_exit_status(int exit_status) noexcept
         services->process_queues();
     }
     else { // STARTING
-        if (exit_status == 0) {
+        if (exit_status.did_exit_clean()) {
             started();
         }
         else {
             // failed to start
             if (did_exit) {
                 log(loglevel_t::ERROR, "Service ", get_name(), " command failed with exit code ",
-                        WEXITSTATUS(exit_status));
+                        exit_status.get_exit_status());
             }
             else if (was_signalled) {
                 log(loglevel_t::ERROR, "Service ", get_name(), " command terminated due to signal ",
-                        WTERMSIG(exit_status));
+                        exit_status.get_term_sig());
             }
             failed_to_start();
         }
@@ -373,7 +373,7 @@ void scripted_service::exec_failed(int errcode) noexcept
 }
 
 bgproc_service::pid_result_t
-bgproc_service::read_pid_file(int *exit_status) noexcept
+bgproc_service::read_pid_file(bp_sys::exit_status *exit_status) noexcept
 {
     const char *pid_file_c = pid_file.c_str();
     int fd = open(pid_file_c, O_CLOEXEC);

diff --git a/src/tests/proctests.cc b/src/tests/proctests.cc
index 3c533cf00fd9a1465120f718a02f93feb608935d..2c1a35e586aa110cd80461129dc1dfdb3d635773 100644 (file)
--- a/src/tests/proctests.cc
+++ b/src/tests/proctests.cc
@@ -14,6 +14,10 @@
 
 extern eventloop_t event_loop;
 
+constexpr static auto REG = dependency_type::REGULAR;
+constexpr static auto WAITS = dependency_type::WAITS_FOR;
+constexpr static auto MS = dependency_type::MILESTONE;
+
 // Friend interface to access base_process_service private/protected members.
 class base_process_service_test
 {
@@ -27,7 +31,7 @@ class base_process_service_test
     static void handle_exit(base_process_service *bsp, int exit_status)
     {
         bsp->pid = -1;
-        bsp->handle_exit_status(exit_status);
+        bsp->handle_exit_status(bp_sys::exit_status(true, false, exit_status));
     }
 };
 
@@ -414,6 +418,49 @@ void test_scripted_stop_timeout()
     sset.remove_service(&p);
 }
 
+void test_scripted_start_fail()
+{
+    using namespace std;
+
+    service_set sset;
+
+    string command = "test-command";
+    string stopcommand = "stop-command";
+    list<pair<unsigned,unsigned>> command_offsets;
+    command_offsets.emplace_back(0, command.length());
+    std::list<prelim_dep> depends;
+
+    scripted_service p = scripted_service(&sset, "testscripted", std::move(command), command_offsets, depends);
+    init_service_defaults(p);
+    p.set_stop_command(stopcommand, command_offsets);
+    sset.add_service(&p);
+
+    service_record *s2 = new service_record(&sset, "test-service-2", service_type_t::INTERNAL, {{&p, REG}});
+    service_record *s3 = new service_record(&sset, "test-service-3", service_type_t::INTERNAL, {{&p, REG}, {s2, REG}});
+    sset.add_service(s2);
+    sset.add_service(s3);
+
+    s3->start(true);
+    sset.process_queues();
+
+    assert(p.get_state() == service_state_t::STARTING);
+
+    base_process_service_test::exec_succeeded(&p);
+    sset.process_queues();
+    base_process_service_test::handle_exit(&p, 0x1);  // exit fail
+    sset.process_queues();
+
+    // failed to start:
+    assert(p.get_state() == service_state_t::STOPPED);
+    assert(s2->get_state() == service_state_t::STOPPED);
+    assert(s3->get_state() == service_state_t::STOPPED);
+
+    event_loop.active_timers.clear();
+    sset.remove_service(&p);
+
+    assert(sset.count_active_services() == 0);
+}
+
 
 #define RUN_TEST(name, spacing) \
     std::cout << #name "..." spacing; \
@@ -431,4 +478,5 @@ int main(int argc, char **argv)
     RUN_TEST(test_proc_smooth_recovery1, "");
     RUN_TEST(test_proc_smooth_recovery2, "");
     RUN_TEST(test_scripted_stop_timeout, "");
+    RUN_TEST(test_scripted_start_fail, "  ");
 }

diff --git a/src/tests/test-includes/baseproc-sys.h b/src/tests/test-includes/baseproc-sys.h
index 3ca25e02029fc94bf479db6c644fbe1c7dfbf811..8a5674a1eac54f7308217f328ec81b2a6cb49f24 100644 (file)
--- a/src/tests/test-includes/baseproc-sys.h
+++ b/src/tests/test-includes/baseproc-sys.h
@@ -1,3 +1,7 @@
+#ifndef BPSYS_INCLUDED
+#define BPSYS_INCLUDED
+
+#include <string>
 #include <sys/types.h>
 #include <unistd.h>
 
@@ -5,6 +9,7 @@
 
 namespace bp_sys {
 
+// implementations elsewhere:
 int pipe2(int pipefd[2], int flags);
 int close(int fd);
 int kill(pid_t pid, int sig);
@@ -30,4 +35,58 @@ inline pid_t getpgrp()
     return getpid();
 }
 
+class exit_status
+{
+    friend pid_t waitpid(pid_t, exit_status *, int);
+
+    bool did_exit_v;
+    bool was_signalled_v;
+    int status;
+
+    public:
+    exit_status() : did_exit_v(true), was_signalled_v(false), status(0) { }
+
+    // status_p is either the exit status or termination signal:
+    exit_status(bool did_exit_p, bool was_signalled_p, int status_p)
+        : did_exit_v(did_exit_p), was_signalled_v(was_signalled_p), status(status_p)
+    { }
+
+    explicit exit_status(int status_p)
+    {
+        throw std::string("initalised exit_status with integer argument");
+    }
+
+    bool did_exit()
+    {
+        return did_exit_v;
+    }
+
+    bool did_exit_clean()
+    {
+        return did_exit_v && status == 0;
+    }
+
+    bool was_signalled()
+    {
+        return was_signalled_v;
+    }
+
+    int get_exit_status()
+    {
+        return status;
+    }
+
+    int get_term_sig()
+    {
+        return status;
+    }
+};
+
+inline pid_t waitpid(pid_t p, exit_status *statusp, int flags)
+{
+    throw std::string("not implemented");
+}
+
 }
+
+#endif