[gimp] app: a few improvements to the GimpBacktrace Linux backend

[Ell <ell src gnome org>]

commit a29d040db52706d4e26f3b7955d5e1677528702e
Author: Ell <ell_se yahoo com>
Date:   Wed Nov 7 14:08:17 2018 -0500

    app: a few improvements to the GimpBacktrace Linux backend
    
    Blacklist the "threaded-ml" thread, which seems to mask the
    backtrace signal.
    
    Improve signal-handler synchronozation, to avoid segfaulting when
    giving up on waiting for all threads to handle the signal.
    Furthermore, when one or more threads fail to handle the signal in
    time, return a GimpBacktrace instance with backtraces for all the
    other threads, and with empty backtraces for all the non-responding
    threads, instead of returning NULL and leaking the allocated
    instance.  Don't blacklist threads that failed to handle the signal
    in time, and instead shorten the wait period for handling the
    signal, and yield execution during waiting to lower the CPU usage.

 app/core/gimpbacktrace-linux.c | 93 +++++++++++++++++++++++++++---------------
 1 file changed, 59 insertions(+), 34 deletions(-)
---
diff --git a/app/core/gimpbacktrace-linux.c b/app/core/gimpbacktrace-linux.c
index 3491e87852..3d3c6afc0d 100644
--- a/app/core/gimpbacktrace-linux.c
+++ b/app/core/gimpbacktrace-linux.c
@@ -62,7 +62,7 @@
 #define MAX_N_FRAMES         256
 #define MAX_THREAD_NAME_SIZE 32
 #define N_SKIPPED_FRAMES     2
-#define MAX_WAIT_TIME        (G_TIME_SPAN_SECOND / 2)
+#define MAX_WAIT_TIME        (G_TIME_SPAN_SECOND / 20)
 #define BACKTRACE_SIGNAL     SIGUSR1
 
 
@@ -113,6 +113,7 @@ static struct sigaction  orig_action;
 static pid_t             blacklisted_threads[MAX_N_THREADS];
 static gint              n_blacklisted_threads;
 static GimpBacktrace    *handler_backtrace;
+static gint              handler_lock;
 
 #ifdef HAVE_LIBBACKTRACE
 static struct backtrace_state *backtrace_state;
@@ -120,7 +121,8 @@ static struct backtrace_state *backtrace_state;
 
 static const gchar *blacklisted_thread_names[] =
 {
-  "gmain"
+  "gmain",
+  "threaded-ml"
 };
 
 
@@ -252,24 +254,42 @@ gimp_backtrace_read_thread_state (pid_t tid)
 static void
 gimp_backtrace_signal_handler (gint signum)
 {
-  GimpBacktrace *curr_backtrace = g_atomic_pointer_get (&handler_backtrace);
-  pid_t          tid            = syscall (SYS_gettid);
-  gint           i;
+  GimpBacktrace *curr_backtrace;
+  gint           lock;
 
-  for (i = 0; i < curr_backtrace->n_threads; i++)
+  do
     {
-      GimpBacktraceThread *thread = &curr_backtrace->threads[i];
+      lock = g_atomic_int_get (&handler_lock);
+
+      if (lock < 0)
+        continue;
+    }
+  while (! g_atomic_int_compare_and_exchange (&handler_lock, lock, lock + 1));
+
+  curr_backtrace = g_atomic_pointer_get (&handler_backtrace);
 
-      if (thread->tid == tid)
+  if (curr_backtrace)
+    {
+      pid_t tid = syscall (SYS_gettid);
+      gint  i;
+
+      for (i = 0; i < curr_backtrace->n_threads; i++)
         {
-          thread->n_frames = backtrace ((gpointer *) thread->frames,
-                                        MAX_N_FRAMES);
+          GimpBacktraceThread *thread = &curr_backtrace->threads[i];
 
-          g_atomic_int_dec_and_test (&curr_backtrace->n_remaining_threads);
+          if (thread->tid == tid)
+            {
+              thread->n_frames = backtrace ((gpointer *) thread->frames,
+                                            MAX_N_FRAMES);
 
-          break;
+              g_atomic_int_dec_and_test (&curr_backtrace->n_remaining_threads);
+
+              break;
+            }
         }
     }
+
+  g_atomic_int_dec_and_test (&handler_lock);
 }
 
 
@@ -368,8 +388,6 @@ gimp_backtrace_new (gboolean include_current_thread)
   pid_t          pid;
   pid_t         *threads;
   gint           n_threads;
-  gint           n_remaining_threads;
-  gint           prev_n_remaining_threads;
   gint64         start_time;
   gint           i;
 
@@ -398,8 +416,12 @@ gimp_backtrace_new (gboolean include_current_thread)
   backtrace->n_threads           = n_threads;
   backtrace->n_remaining_threads = n_threads;
 
+  while (! g_atomic_int_compare_and_exchange (&handler_lock, 0, -1));
+
   g_atomic_pointer_set (&handler_backtrace, backtrace);
 
+  g_atomic_int_set (&handler_lock, 0);
+
   for (i = 0; i < n_threads; i++)
     {
       GimpBacktraceThread *thread = &backtrace->threads[i];
@@ -419,30 +441,27 @@ gimp_backtrace_new (gboolean include_current_thread)
 
   start_time = g_get_monotonic_time ();
 
-  prev_n_remaining_threads =
-    g_atomic_int_get (&backtrace->n_remaining_threads);
-
-  while ((n_remaining_threads =
-            g_atomic_int_get (&backtrace->n_remaining_threads) > 0))
+  while (g_atomic_int_get (&backtrace->n_remaining_threads) > 0)
     {
       gint64 time = g_get_monotonic_time ();
 
-      if (n_remaining_threads < prev_n_remaining_threads)
-        {
-          prev_n_remaining_threads = n_remaining_threads;
+      if (time - start_time > MAX_WAIT_TIME)
+        break;
 
-          start_time = time;
-        }
-      else if (time - start_time > MAX_WAIT_TIME)
-        {
-          break;
-        }
+      g_usleep (1000);
     }
 
-  handler_backtrace = NULL;
+  while (! g_atomic_int_compare_and_exchange (&handler_lock, 0, -1));
 
-  if (n_remaining_threads > 0)
+  g_atomic_pointer_set (&handler_backtrace, NULL);
+
+  g_atomic_int_set (&handler_lock, 0);
+
+#if 0
+  if (backtrace->n_remaining_threads > 0)
     {
+      gint j = 0;
+
       for (i = 0; i < n_threads; i++)
         {
           if (backtrace->threads[i].n_frames == 0)
@@ -452,13 +471,19 @@ gimp_backtrace_new (gboolean include_current_thread)
                   blacklisted_threads[n_blacklisted_threads++] =
                     backtrace->threads[i].tid;
                 }
+            }
+          else
+            {
+              if (j < i)
+                backtrace->threads[j] = backtrace->threads[i];
 
-              g_mutex_unlock (&mutex);
-
-              return NULL;
+              j++;
             }
         }
+
+      n_threads = j;
     }
+#endif
 
   g_mutex_unlock (&mutex);
 
@@ -557,7 +582,7 @@ gimp_backtrace_get_n_frames (GimpBacktrace *backtrace,
   g_return_val_if_fail (backtrace != NULL, 0);
   g_return_val_if_fail (thread >= 0 && thread < backtrace->n_threads, 0);
 
-  return backtrace->threads[thread].n_frames - N_SKIPPED_FRAMES;
+  return MAX (backtrace->threads[thread].n_frames - N_SKIPPED_FRAMES, 0);
 }
 
 guintptr