fix usage of pthread_cond_timedwait (#491)

* fix usage of pthread_cond_timedwait pthread_cond_timedwait has to be called with a locked mutex using an unlocked mutex is undefined behaviour although it works on many platforms Signed-off-by: Alexander Mohr <[email protected]> Signed-off-by: Alexander Mohr <[email protected]> * startup: check housekeeper thread every 500ms this commit makes sure that we do not wait for up to 10s if we missed the signal for the condition variable the first time Signed-off-by: Alexander Mohr <[email protected]> * Remove unneccesary mutex log --------- Signed-off-by: Alexander Mohr <[email protected]> Co-authored-by: Alexander Mohr <[email protected]> Co-authored-by: michael-methner <[email protected]>
COVESA · Aug 9, 2023 · d3d4bc3 · d3d4bc3
1 parent 774fe31
commit d3d4bc3
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 14 deletions.
diff --git a/src/console/dlt-control-common.c b/src/console/dlt-control-common.c
@@ -565,12 +565,18 @@ int dlt_control_send_message(DltControlMsgBody *body, int timeout)
         pr_error("Sending message to daemon failed\n");
         dlt_message_free(msg, get_verbosity());
         free(msg);
+
+        /* make sure the mutex is unlocked to prevent deadlocks */
+        pthread_mutex_unlock(&answer_lock);
         return -1;
     }
 
-    /* If we timeout the lock is not taken back */
-    if (!pthread_cond_timedwait(&answer_cond, &answer_lock, &t))
-        pthread_mutex_unlock(&answer_lock);
+    /*
+    * When a timeouts occurs, pthread_cond_timedwait()
+    * shall nonetheless release and re-acquire the mutex referenced by mutex
+    */
+    pthread_cond_timedwait(&answer_cond, &answer_lock, &t);
+    pthread_mutex_unlock(&answer_lock);
 
     /* Destroying the message */
     dlt_message_free(msg, get_verbosity());

diff --git a/src/lib/dlt_user.c b/src/lib/dlt_user.c
@@ -4957,15 +4957,15 @@ void dlt_user_test_corrupt_message_size(int enable, int16_t size)
 
 int dlt_start_threads()
 {
-    struct timespec time_to_wait;
+    struct timespec time_to_wait, single_wait;
     struct timespec now;
-    int signal_status;
+    int signal_status = 1;
     atomic_bool dlt_housekeeper_running = false;
 
     /*
     * Configure the condition varibale to use CLOCK_MONOTONIC.
     * This makes sure we're protected against changes in the system clock
-    */
+     */
     pthread_condattr_t attr;
     pthread_condattr_init(&attr);
     pthread_condattr_setclock(&attr, CLOCK_MONOTONIC);
@@ -4982,7 +4982,7 @@ int dlt_start_threads()
     clock_gettime(CLOCK_MONOTONIC, &now);
     /* wait at most 10s */
     time_to_wait.tv_sec = now.tv_sec + 10;
-    time_to_wait.tv_nsec = 0;
+    time_to_wait.tv_nsec = now.tv_nsec;
 
     /*
     * wait until the house keeper is up and running
@@ -4993,19 +4993,30 @@ int dlt_start_threads()
     * (spurious wakeup)
     * To protect against this, a while loop with a timeout is added
     * */
-    while (!dlt_housekeeper_running 
-        && now.tv_sec <= time_to_wait.tv_sec) {
+    while (!dlt_housekeeper_running
+           && now.tv_sec <= time_to_wait.tv_sec) {
+
+        /*
+        * wait 500ms at a time
+        * this makes sure we don't block too long
+        * even if we missed the signal
+         */
+        clock_gettime(CLOCK_MONOTONIC, &now);
+        single_wait.tv_sec = now.tv_sec;
+        single_wait.tv_nsec = now.tv_nsec + 500000000;
+
+        // pthread_cond_timedwait has to be called on a locked mutex
+        pthread_mutex_lock(&dlt_housekeeper_running_mutex);
         signal_status = pthread_cond_timedwait(
-                &dlt_housekeeper_running_cond,
-                &dlt_housekeeper_running_mutex,
-                &time_to_wait);
+            &dlt_housekeeper_running_cond,
+            &dlt_housekeeper_running_mutex,
+            &single_wait);
+        pthread_mutex_unlock(&dlt_housekeeper_running_mutex);
 
         /* otherwise it might be a spurious wakeup, try again until the time is over */
         if (signal_status == 0) {
             break;
         }
-
-        clock_gettime(CLOCK_MONOTONIC, &now);
      }
 
      if (signal_status != 0 && !dlt_housekeeper_running) {