Merge pull request #2813 from matt335672/remove_signal_call

Remove dependency on signal() function

common/os_calls.c

@@ -2987,10 +2987,28 @@ g_set_alarm(void (*func)(int), unsigned int secs)
 #if defined(_WIN32)
     return 0;
 #else
+    struct sigaction action;
+
     /* Cancel any previous alarm to prevent a race */
     unsigned int rv = alarm(0);
-    signal(SIGALRM, func);
+
-    (void)alarm(secs);
+    if (func == NULL)
+    {
+        action.sa_handler = SIG_DFL;
+        action.sa_flags = 0;
+    }
+    else
+    {
+        action.sa_handler = func;
+        action.sa_flags = SA_RESTART;
+    }
+    sigemptyset (&action.sa_mask);
+
+    sigaction(SIGALRM, &action, NULL);
+    if (func != NULL && secs > 0)
+    {
+        (void)alarm(secs);
+    }
     return rv;
 #endif
 }
@@ -3002,7 +3020,22 @@ g_signal_child_stop(void (*func)(int))
 {
 #if defined(_WIN32)
 #else
-    signal(SIGCHLD, func);
+    struct sigaction action;
+
+    if (func == NULL)
+    {
+        action.sa_handler = SIG_DFL;
+        action.sa_flags = 0;
+    }
+    else
+    {
+        action.sa_handler = func;
+        // Don't need to know when children are stopped or started
+        action.sa_flags = (SA_RESTART | SA_NOCLDSTOP);
+    }
+    sigemptyset (&action.sa_mask);
+
+    sigaction(SIGCHLD, &action, NULL);
 #endif
 }
 
@@ -3013,7 +3046,21 @@ g_signal_segfault(void (*func)(int))
 {
 #if defined(_WIN32)
 #else
-    signal(SIGSEGV, func);
+    struct sigaction action;
+
+    if (func == NULL)
+    {
+        action.sa_handler = SIG_DFL;
+        action.sa_flags = 0;
+    }
+    else
+    {
+        action.sa_handler = func;
+        action.sa_flags = SA_RESETHAND; // This is a one-shot
+    }
+    sigemptyset (&action.sa_mask);
+
+    sigaction(SIGSEGV, &action, NULL);
 #endif
 }
 
@@ -3024,7 +3071,21 @@ g_signal_hang_up(void (*func)(int))
 {
 #if defined(_WIN32)
 #else
-    signal(SIGHUP, func);
+    struct sigaction action;
+
+    if (func == NULL)
+    {
+        action.sa_handler = SIG_DFL;
+        action.sa_flags = 0;
+    }
+    else
+    {
+        action.sa_handler = func;
+        action.sa_flags = SA_RESTART;
+    }
+    sigemptyset (&action.sa_mask);
+
+    sigaction(SIGHUP, &action, NULL);
 #endif
 }
 
@@ -3035,7 +3096,21 @@ g_signal_user_interrupt(void (*func)(int))
 {
 #if defined(_WIN32)
 #else
-    signal(SIGINT, func);
+    struct sigaction action;
+
+    if (func == NULL)
+    {
+        action.sa_handler = SIG_DFL;
+        action.sa_flags = 0;
+    }
+    else
+    {
+        action.sa_handler = func;
+        action.sa_flags = SA_RESTART;
+    }
+    sigemptyset (&action.sa_mask);
+
+    sigaction(SIGINT, &action, NULL);
 #endif
 }
 
@@ -3046,7 +3121,21 @@ g_signal_terminate(void (*func)(int))
 {
 #if defined(_WIN32)
 #else
-    signal(SIGTERM, func);
+    struct sigaction action;
+
+    if (func == NULL)
+    {
+        action.sa_handler = SIG_DFL;
+        action.sa_flags = 0;
+    }
+    else
+    {
+        action.sa_handler = func;
+        action.sa_flags = SA_RESTART;
+    }
+    sigemptyset (&action.sa_mask);
+
+    sigaction(SIGTERM, &action, NULL);
 #endif
 }
 
@@ -3057,7 +3146,21 @@ g_signal_pipe(void (*func)(int))
 {
 #if defined(_WIN32)
 #else
-    signal(SIGPIPE, func);
+    struct sigaction action;
+
+    if (func == NULL)
+    {
+        action.sa_handler = SIG_DFL;
+        action.sa_flags = 0;
+    }
+    else
+    {
+        action.sa_handler = func;
+        action.sa_flags = SA_RESTART;
+    }
+    sigemptyset (&action.sa_mask);
+
+    sigaction(SIGPIPE, &action, NULL);
 #endif
 }
 
@@ -3068,7 +3171,21 @@ g_signal_usr1(void (*func)(int))
 {
 #if defined(_WIN32)
 #else
-    signal(SIGUSR1, func);
+    struct sigaction action;
+
+    if (func == NULL)
+    {
+        action.sa_handler = SIG_DFL;
+        action.sa_flags = 0;
+    }
+    else
+    {
+        action.sa_handler = func;
+        action.sa_flags = SA_RESTART;
+    }
+    sigemptyset (&action.sa_mask);
+
+    sigaction(SIGUSR1, &action, NULL);
 #endif
 }
 

common/os_calls.h

@@ -176,6 +176,14 @@ g_sck_get_peer_ip_address(int sck,
 const char *
 g_sck_get_peer_description(int sck,
                            char *desc, unsigned int bytes);
+/**
+ * Sleep for the specified number of milli-seconds
+ * @param msecs Milli-seconds
+ *
+ * If a signal is processed, it is possible that this call will
+ * sleep for less than the specified number of milli-seconds. This
+ * is platform-specific
+ */
 void     g_sleep(int msecs);
 int      g_pipe(int fd[2]);
 
@@ -275,13 +283,56 @@ int      g_execvp(const char *p1, char *args[]);
  */
 int      g_execvp_list(const char *file, struct list *argv);
 int      g_execlp3(const char *a1, const char *a2, const char *a3);
+/**
+ * Set an alarm using SIGALRM
+ * @param func Signal handler, or NULL to cancel an alarm
+ * @param secs Number of seconds until an alarm is raised
+ * @return Number of seconds remaining before a previously requested
+ *         alarm is raised
+ */
 unsigned int g_set_alarm(void (*func)(int), unsigned int secs);
+/**
+ * Set a handler up for SIGCHLD
+ * @param func signal handler, or NULL to restore the default handler
+ * The handler remains in place until explicitly replaced.
+ */
 void     g_signal_child_stop(void (*func)(int));
+/**
+ * Set a handler up for SIGSEGV
+ * @param func signal handler, or NULL to restore the default handler
+ * The handler can only be called once, at which point the
+ * default handler is restored. This is to avoid infinite loops
+ */
 void     g_signal_segfault(void (*func)(int));
+/**
+ * Set a handler up for SIGHUP
+ * @param func signal handler, or NULL to restore the default handler
+ * The handler remains in place until explicitly replaced.
+ */
 void     g_signal_hang_up(void (*func)(int));
+/**
+ * Set a handler up for SIGINT
+ * @param func signal handler, or NULL to restore the default handler
+ * The handler remains in place until explicitly replaced.
+ */
 void     g_signal_user_interrupt(void (*func)(int));
+/**
+ * Set a handler up for SIGTERM
+ * @param func signal handler, or NULL to restore the default handler
+ * The handler remains in place until explicitly replaced.
+ */
 void     g_signal_terminate(void (*func)(int));
+/**
+ * Set a handler up for SIGPIPE
+ * @param func signal handler, or NULL to restore the default handler
+ * The handler remains in place until explicitly replaced.
+ */
 void     g_signal_pipe(void (*func)(int));
+/**
+ * Set a handler up for SIGUSR1
+ * @param func signal handler, or NULL to restore the default handler
+ * The handler remains in place until explicitly replaced.
+ */
 void     g_signal_usr1(void (*func)(int));
 int      g_fork(void);
 int      g_setgid(int pid);

tests/common/Makefile.am

@@ -18,6 +18,7 @@ test_common_SOURCES = \
     test_list_calls.c \
     test_string_calls.c \
     test_os_calls.c \
+    test_os_calls_signals.c \
     test_ssl_calls.c \
     test_base64.c \
     test_guid.c

tests/common/test_common.h

@@ -15,4 +15,9 @@ Suite *make_suite_test_ssl_calls(void);
 Suite *make_suite_test_base64(void);
 Suite *make_suite_test_guid(void);
 
+TCase *make_tcase_test_os_calls_signals(void);
+
+void os_calls_signals_init(void);
+void os_calls_signals_deinit(void);
+
 #endif /* TEST_COMMON_H */

tests/common/test_common_main.c

@@ -64,14 +64,17 @@ int main (void)
      * reporting when running in libcheck fork mode */
     setvbuf(stdout, NULL, _IONBF, 0);
 
-    /* Initialise the ssl module */
+    /* Initialise modules */
     ssl_init();
+    os_calls_signals_init();
 
     srunner_run_all (sr, CK_ENV);
     number_failed = srunner_ntests_failed(sr);
     srunner_free(sr);
 
     ssl_finish();
+    os_calls_signals_deinit();
+
     log_end();
     return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
 }

tests/common/test_os_calls.c

@@ -512,5 +512,9 @@ make_suite_test_os_calls(void)
     tcase_add_test(tc_os_calls, test_g_file_is_open);
     tcase_add_test(tc_os_calls, test_g_sck_fd_passing);
     tcase_add_test(tc_os_calls, test_g_sck_fd_overflow);
+
+    // Add other test cases in other files
+    suite_add_tcase(s, make_tcase_test_os_calls_signals());
+
     return s;
 }

tests/common/test_os_calls_signals.c

@@ -0,0 +1,251 @@
+
+#if defined(HAVE_CONFIG_H)
+#include "config_ac.h"
+#endif
+
+#include <signal.h>
+
+#include "os_calls.h"
+
+#include "test_common.h"
+
+static tintptr g_wobj1 = 0;
+
+/******************************************************************************/
+void
+os_calls_signals_init(void)
+{
+    g_wobj1 = g_create_wait_obj("");
+}
+
+/******************************************************************************/
+void
+os_calls_signals_deinit(void)
+{
+    g_delete_wait_obj(g_wobj1);
+    g_wobj1 = 0;
+}
+
+/******************************************************************************/
+/**
+ * Set the global wait object g_wobj1
+ */
+static void
+set_wobj1(int signum)
+{
+    g_set_wait_obj(g_wobj1);
+}
+
+/******************************************************************************/
+/**
+ * Sends a number of signals to the process and checks they are all delivered
+ *
+ * @param sig Signal number
+ * @param count Number of signals to send
+ *
+ * The caller is expected to establish a signal handler before this call
+ * which sets the global g_wobj1 on signal delivery */
+
+static
+void send_multiple_signals(int sig, unsigned int count)
+{
+    while (count-- > 0)
+    {
+        g_reset_wait_obj(g_wobj1);
+        ck_assert_int_eq(g_is_wait_obj_set(g_wobj1), 0);
+        // Expect the signal to be delivered synchronously
+        raise(sig);
+        ck_assert_int_ne(g_is_wait_obj_set(g_wobj1), 0);
+    }
+}
+
+/******************************************************************************/
+START_TEST(test_g_set_alarm)
+{
+    g_reset_wait_obj(g_wobj1);
+    ck_assert_int_eq(g_is_wait_obj_set(g_wobj1), 0);
+
+    g_set_alarm(set_wobj1, 1);
+
+    g_obj_wait(&g_wobj1, 1, NULL, 0, 2000);
+
+    ck_assert_int_ne(g_is_wait_obj_set(g_wobj1), 0);
+
+    // Clean up
+    g_set_alarm(NULL, 0);
+}
+END_TEST
+
+/******************************************************************************/
+START_TEST(test_g_signal_child_stop_1)
+{
+    struct exit_status e;
+
+    g_reset_wait_obj(g_wobj1);
+    ck_assert_int_eq(g_is_wait_obj_set(g_wobj1), 0);
+
+    g_signal_child_stop(set_wobj1);
+
+    int pid = g_fork();
+
+    if (pid == 0)
+    {
+        g_exit(45);
+    }
+    ck_assert_int_ne(pid, 0);
+    g_obj_wait(&g_wobj1, 1, NULL, 0, 2000);
+    ck_assert_int_ne(g_is_wait_obj_set(g_wobj1), 0);
+
+    e = g_waitpid_status(pid);
+
+    ck_assert_int_eq(e.reason, E_XR_STATUS_CODE);
+    ck_assert_int_eq(e.val, 45);
+
+    // Try another one to make sure the signal handler is still in place.
+    // This one can generate a signal
+    g_reset_wait_obj(g_wobj1);
+
+    pid = g_fork();
+    if (pid == 0)
+    {
+        raise(SIGSEGV);
+    }
+    ck_assert_int_ne(pid, 0);
+    g_obj_wait(&g_wobj1, 1, NULL, 0, 2000);
+    ck_assert_int_ne(g_is_wait_obj_set(g_wobj1), 0);
+
+    e = g_waitpid_status(pid);
+
+    ck_assert_int_eq(e.reason, E_XR_SIGNAL);
+    ck_assert_int_eq(e.val, SIGSEGV);
+
+    // Clean up
+    g_signal_child_stop(NULL);
+}
+END_TEST
+
+/******************************************************************************/
+/* Checks that multiple children finishing do not interrupt
+ * g_waitpid_status() */
+START_TEST(test_g_signal_child_stop_2)
+{
+#define CHILD_COUNT 20
+    int pids[CHILD_COUNT];
+    unsigned int i;
+
+    struct exit_status e;
+
+    g_reset_wait_obj(g_wobj1);
+    ck_assert_int_eq(g_is_wait_obj_set(g_wobj1), 0);
+
+    g_signal_child_stop(set_wobj1);
+
+    for (i = 0 ; i < CHILD_COUNT; ++i)
+    {
+        int pid = g_fork();
+        if (pid == 0)
+        {
+            g_sleep((i + 1) * 100);
+            g_exit(i + 1);
+        }
+        ck_assert_int_ne(pid, 0);
+        pids[i] = pid;
+    }
+    g_obj_wait(&g_wobj1, 1, NULL, 0, 2000);
+    ck_assert_int_ne(g_is_wait_obj_set(g_wobj1), 0);
+
+    for (i = 0 ; i < CHILD_COUNT; ++i)
+    {
+        e = g_waitpid_status(pids[i]);
+        ck_assert_int_eq(e.reason, E_XR_STATUS_CODE);
+        ck_assert_int_eq(e.val, (i + 1));
+    }
+
+    // Clean up
+    g_signal_child_stop(NULL);
+#undef CHILD_COUNT
+}
+END_TEST
+
+
+/******************************************************************************/
+START_TEST(test_g_signal_segfault)
+{
+    g_signal_segfault(set_wobj1);
+
+    // Only one signal can be received in this way. After handling
+    // the signal the handler should be automatically reset.
+    send_multiple_signals(SIGSEGV, 1);
+
+    g_signal_segfault(NULL);
+}
+END_TEST
+
+/******************************************************************************/
+START_TEST(test_g_signal_hang_up)
+{
+    g_signal_hang_up(set_wobj1);
+
+    send_multiple_signals(SIGHUP, 5);
+
+    g_signal_hang_up(NULL);
+}
+
+/******************************************************************************/
+START_TEST(test_g_signal_user_interrupt)
+{
+    g_signal_user_interrupt(set_wobj1);
+
+    send_multiple_signals(SIGINT, 5);
+
+    g_signal_user_interrupt(NULL);
+}
+
+/******************************************************************************/
+START_TEST(test_g_signal_terminate)
+{
+    g_signal_terminate(set_wobj1);
+
+    send_multiple_signals(SIGTERM, 5);
+
+    g_signal_terminate(NULL);
+}
+
+/******************************************************************************/
+START_TEST(test_g_signal_pipe)
+{
+    g_signal_pipe(set_wobj1);
+
+    send_multiple_signals(SIGPIPE, 5);
+
+    g_signal_pipe(NULL);
+}
+
+/******************************************************************************/
+START_TEST(test_g_signal_usr1)
+{
+    g_signal_usr1(set_wobj1);
+
+    send_multiple_signals(SIGUSR1, 5);
+
+    g_signal_usr1(NULL);
+}
+
+/******************************************************************************/
+TCase *
+make_tcase_test_os_calls_signals(void)
+{
+    TCase *tc = tcase_create("oscalls-signals");
+
+    tcase_add_test(tc, test_g_set_alarm);
+    tcase_add_test(tc, test_g_signal_child_stop_1);
+    tcase_add_test(tc, test_g_signal_child_stop_2);
+    tcase_add_test(tc, test_g_signal_segfault);
+    tcase_add_test(tc, test_g_signal_hang_up);
+    tcase_add_test(tc, test_g_signal_user_interrupt);
+    tcase_add_test(tc, test_g_signal_terminate);
+    tcase_add_test(tc, test_g_signal_pipe);
+    tcase_add_test(tc, test_g_signal_usr1);
+
+    return tc;
+}