]> Kevux Git Server - fll/commitdiff
Update: Improve sleeping logic, also replacing sleep() with nanosleep().
authorKevin Day <thekevinday@gmail.com>
Sun, 31 Oct 2021 13:00:17 +0000 (08:00 -0500)
committerKevin Day <thekevinday@gmail.com>
Sun, 31 Oct 2021 13:06:38 +0000 (08:06 -0500)
The previous time-related regression fix introduced unblocking the signals as an immediate solution to its problem.

The preferred and intended design is to have the signal handling done by controller rather than done automatically.

Redesign the sleep functions to only unblock the signals for the sleep call.
Then re-lock the signal so that the intended design may be continued.

This then allows all appropriate cleanup functionality to operate as expected.
This can be confirmed with valgrind before/after and sending an interrupt during a validation process that utilizes a sleep (such as the example htop rule).

Create several functions to standardize some of the processes involved.

This also resolves a "todo" designating to replace sleep() with nanosleep().

level_3/controller/c/private-common.c
level_3/controller/c/private-common.h
level_3/controller/c/private-rule.c
level_3/controller/c/private-thread.c

index b5b13d8623f5bbf636f17ec6d53f5913f065a223..832cf558543a22928e4d5336931590e7d7f3b2ef 100644 (file)
@@ -934,6 +934,34 @@ extern "C" {
   }
 #endif // _di_controller_time_milliseconds_
 
+#ifndef _di_controller_time_seconds_
+  struct timespec controller_time_seconds(const f_number_unsigned_t seconds) {
+
+    struct timespec time;
+    time.tv_sec = seconds;
+    time.tv_nsec = 0;
+
+    return time;
+  }
+#endif // _di_controller_time_seconds_
+
+#ifndef _di_controller_time_sleep_nanoseconds_
+  int controller_time_sleep_nanoseconds(controller_main_t * const main, controller_setting_t * const setting, struct timespec time) {
+
+    if (setting->interruptible) {
+      f_signal_mask(SIG_UNBLOCK, &main->signal.set, 0);
+    }
+
+    const int result = nanosleep(&time, 0);
+
+    if (setting->interruptible) {
+      f_signal_mask(SIG_UNBLOCK, &main->signal.set, 0);
+    }
+
+    return result;
+  }
+#endif // _di_controller_time_sleep_nanoseconds_
+
 #ifdef __cplusplus
 } // extern "C"
 #endif
index e5d7a3605425a8bafc7cc1f36c300064b6dfc883..c47f7f3219d661616b2f7c5fb8ceac6bae8889ac 100644 (file)
@@ -2477,11 +2477,53 @@ extern "C" {
  *
  * @return
  *   A time structure suitable for passing to nanosleep() and similar functions.
+ *
+ * @see nanosleep()
  */
 #ifndef _di_controller_time_milliseconds_
   extern struct timespec controller_time_milliseconds(const f_number_unsigned_t milliseconds) F_attribute_visibility_internal_d;
 #endif // _di_controller_time_milliseconds_
 
+/**
+ * Convert seconds to nanoseconds.
+ *
+ * @param seconds
+ *   The number of seconds.
+ *
+ * @return
+ *   A time structure suitable for passing to nanosleep() and similar functions.
+ *
+ * @see nanosleep()
+ */
+#ifndef _di_controller_time_seconds_
+  extern struct timespec controller_time_seconds(const f_number_unsigned_t seconds) F_attribute_visibility_internal_d;
+#endif // _di_controller_time_seconds_
+
+/**
+ * Sleep for a given number of nanoseconds.
+ *
+ * The nanosleep() function handles signals within itself.
+ * Temporarily unblock signals so that the nanosleep can receive the signal and then restore the signals once done.
+ *
+ * The signals will not be unblocked when in uninterruptible mode.
+ *
+ * @param main
+ *   The program main data.
+ * @param setting
+ *   The settings.
+ * @param time
+ *   The number of nanoseconds to sleep.
+ *
+ * @return
+ *   The results of nanosleep().
+ *
+ * @see nanosleep()
+ * @see controller_time_milliseconds()
+ */
+#ifndef _di_controller_time_sleep_nanoseconds_
+  extern int controller_time_sleep_nanoseconds(controller_main_t * const main, controller_setting_t * const setting, struct timespec time) F_attribute_visibility_internal_d;
+#endif // _di_controller_time_sleep_nanoseconds_
+
 #ifdef __cplusplus
 } // extern "C"
 #endif
index f71eb0f0d6f5a24257aeb1e9f09b17686e778f1a..2654e2ea6c4a4cab4a5c70e438b587076aca894b 100644 (file)
@@ -1645,9 +1645,9 @@ extern "C" {
 
       // sleep for less than a second to better show simulation of synchronous vs asynchronous.
       {
-        struct timespec delay = controller_time_milliseconds(controller_thread_simulation_timeout_d);
+        const struct timespec delay = controller_time_milliseconds(controller_thread_simulation_timeout_d);
 
-        if (nanosleep(&delay, 0) == -1) {
+        if (controller_time_sleep_nanoseconds(main, (controller_setting_t *) process->main_setting, delay) == -1) {
           status = F_signal;
         }
       }
@@ -1901,9 +1901,9 @@ extern "C" {
 
       // sleep for less than a second to better show simulation of synchronous vs asynchronous.
       {
-        struct timespec delay = controller_time_milliseconds(controller_thread_simulation_timeout_d);
+        const struct timespec delay = controller_time_milliseconds(controller_thread_simulation_timeout_d);
 
-        if (nanosleep(&delay, 0) == -1) {
+        if (controller_time_sleep_nanoseconds(main, (controller_setting_t *) process->main_setting, delay) == -1) {
           status = F_signal;
         }
       }
@@ -2090,9 +2090,9 @@ extern "C" {
         }
 
         if (rerun_item->delay) {
-          struct timespec time = controller_time_milliseconds(rerun_item->delay);
+          const struct timespec delay = controller_time_milliseconds(rerun_item->delay);
 
-          if (nanosleep(&time, 0) == -1) {
+          if (controller_time_sleep_nanoseconds(main, (controller_setting_t *) process->main_setting, delay) == -1) {
             return -1;
           }
 
index 3812f512196ac1ba3ca80dc6358697aedf61473e..71bc19f233058836ac2eee0bc04c695a72e8415b 100644 (file)
@@ -18,14 +18,13 @@ extern "C" {
 
     if (global->thread->enabled != controller_thread_enabled) return 0;
 
-    const unsigned int interval = global->main->parameters[controller_parameter_simulate].result == f_console_result_found ? controller_thread_cleanup_interval_short_d : controller_thread_cleanup_interval_long_d;
+    const struct timespec delay = controller_time_seconds(global->main->parameters[controller_parameter_simulate].result == f_console_result_found ? controller_thread_cleanup_interval_short_d : controller_thread_cleanup_interval_long_d);
 
     f_status_t status = F_none;
 
     while (global->thread->enabled == controller_thread_enabled) {
 
-      // @todo consider switching to nanosleep() which may act better with interrupts and not require f_thread_cancel().
-      sleep(interval);
+      controller_time_sleep_nanoseconds(global->main, global->setting, delay);
 
       if (global->thread->enabled != controller_thread_enabled) break;
 
@@ -348,18 +347,6 @@ extern "C" {
       if (!controller_thread_is_enabled(is_normal, thread)) return;
     }
 
-    // Threads are restricted during the main thread so that the signals can be processed.
-    // This thread must regain control over the signals within its own thread scope.
-    {
-      controller_main_t *main = (controller_main_t *) process->main_data;
-      controller_setting_t *setting = (controller_setting_t *) process->main_setting;
-
-      if (setting->interruptible) {
-        f_signal_mask(SIG_UNBLOCK, &main->signal.set, 0);
-        f_signal_close(&main->signal);
-      }
-    }
-
     const f_status_t status = controller_rule_process_do(controller_process_option_asynchronous_d, process);
 
     if (status == F_child) {
@@ -444,7 +431,7 @@ extern "C" {
     f_array_length_t j = 0;
     pid_t pid = 0;
 
-    // the sleep() function that is run inside the cleanup function must be interrupted via the f_thread_cancel().
+    // The sleep functions that are run inside the cleanup function must be interrupted via the f_thread_cancel().
     if (global->thread->id_cleanup) {
       f_thread_cancel(global->thread->id_cleanup);
       f_thread_join(global->thread->id_cleanup, 0);