Put abort() in place to facilitate debugging 82/317382/5
authorKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Tue, 31 Dec 2024 09:36:55 +0000 (10:36 +0100)
committerKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Tue, 31 Dec 2024 12:35:47 +0000 (13:35 +0100)
This way the backtrace will show us the point of failure

Change-Id: I97f70db007661b91baa757442b01c0cdc3270a5b

src/client/client-security-manager.cpp

index 6a61030af05c213861c781e3ce385129dbc3fb35..553c0b928a4945bb939cdf08943d0479e42c418f 100644 (file)
@@ -654,14 +654,21 @@ static bool alive_threads_have_state(int status) noexcept
     return true;
 }
 
-static int get_alive_threads(int own_tid) noexcept
+static int get_alive_threads(int own_tid, bool abort_on_error) noexcept
 {
+       auto handle_error = [&]{
+               if (abort_on_error)
+                       abort();
+
+               return errno;
+       };
+
     // reset alive status
     memset(&g_thread_alive, 0, sizeof(g_thread_alive));
 
     auto dir_fd = open("/proc/self/task", O_DIRECTORY | O_CLOEXEC);
     if (dir_fd == -1)
-        return errno;
+        return handle_error();
 
     char buf[1024];
     struct dirent64 *d;
@@ -670,7 +677,7 @@ static int get_alive_threads(int own_tid) noexcept
         ssize_t nread = getdents64(dir_fd, buf, sizeof(buf));
         if (nread == -1) {
             close(dir_fd);
-            return errno;
+            return handle_error();
         }
 
         if (nread == 0)
@@ -693,7 +700,7 @@ static int get_alive_threads(int own_tid) noexcept
             long tid = strtol(d->d_name, &endptr, 10);
             if (errno != 0) {
                 close(dir_fd);
-                return errno;
+                return handle_error();
             }
 
             if (tid == own_tid)
@@ -718,13 +725,13 @@ static int get_alive_threads(int own_tid) noexcept
     return 0;
 }
 
-static int check_threads(int own_tid) noexcept
+static void check_threads(int own_tid) noexcept
 {
     int ret = 0;
-    for (unsigned i = 0; i < 10; ++i) {
-        ret = get_alive_threads(own_tid);
+    for (unsigned i = 0; i < 9; ++i) {
+        ret = get_alive_threads(own_tid, false);
         if (ret == 0)
-            break;
+            return;
 
         /*
          *  This may happen if a thread disappears, an entry is removed from /proc and getdents64
@@ -732,7 +739,7 @@ static int check_threads(int own_tid) noexcept
          */
     }
 
-    return ret;
+    get_alive_threads(own_tid, true);
 }
 
 static bool no_new_threads() noexcept
@@ -747,14 +754,11 @@ static bool no_new_threads() noexcept
     return true;
 }
 
-static int signal_and_wait_for_handlers(pid_t own_pid, int own_tid) noexcept
+static void signal_and_wait_for_handlers(pid_t own_pid, int own_tid) noexcept
 {
-    int ret = 0;
     int time_left = MAX_SIG_WAIT_TIME;
     do {
-        ret = check_threads(own_tid);
-        if (ret != 0)
-            break;
+        check_threads(own_tid);
 
         for (int i = 0; i < g_managed_tids_num; ++i) {
             if (!g_thread_alive[i])
@@ -774,13 +778,13 @@ static int signal_and_wait_for_handlers(pid_t own_pid, int own_tid) noexcept
                             if (ESRCH == err) { // thread already gone - noop
                                 continue;
                             } else {
-                                return err;
+                                abort();
                             }
                         }
                     } else if (ESRCH == err) { // thread already gone - noop
                         continue;
                     } else {
-                        return err;
+                        abort();
                     }
                 }
             }
@@ -794,9 +798,7 @@ static int signal_and_wait_for_handlers(pid_t own_pid, int own_tid) noexcept
             // threads that were read some lines above are all sleeping here - BUT - could have
             // spawned new threads between reading list from /proc and checking status with count_alive_tids_with_state()
             // to make sure the loop can end, here we read /proc again
-            ret = check_threads(own_tid);
-            if (ret != 0)
-                break;
+            check_threads(own_tid);
 
             if (no_new_threads())
                 // here indeed, no new threads are present and they are all waiting in the signal handler
@@ -804,9 +806,8 @@ static int signal_and_wait_for_handlers(pid_t own_pid, int own_tid) noexcept
         }
 
         if (time_left == 0)
-            return ETIME;
+            abort();
     } while (true);
-    return ret;
 }
 
 static inline int security_manager_sync_threads_internal(const std::string &app_label)
@@ -864,9 +865,7 @@ static inline int security_manager_sync_threads_internal(const std::string &app_
     g_th_barrier = 0;
 
     // No allocations allowed beyond this point
-    int ret = signal_and_wait_for_handlers(own_pid, own_tid);
-    if (ret != 0)
-        abort();
+    signal_and_wait_for_handlers(own_pid, own_tid);
 
     // here, all TIDs except current one are waiting to start changing attributes
     // We can assume these TIDs will continue to live (no need to read /proc again), since no logic