[gimp] app: make backtrace processed in the thread where error happens.
- From: Jehan Pagès <jehanp src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gimp] app: make backtrace processed in the thread where error happens.
- Date: Mon, 12 Feb 2018 04:31:09 +0000 (UTC)
commit b0cd4412e03e80c33b2b2649c0881573eeac10d7
Author: Jehan <jehan girinstud io>
Date: Mon Feb 12 05:13:22 2018 +0100
app: make backtrace processed in the thread where error happens.
Slight back step from commit 34fe992f44. I don't keep track anymore of
the number of errors inside GimpCriticalDialog. The problem is that GTK+
calls must happen in the main thread, and errors in another thread will
be delayed into the main thread through gdk_threads_add_idle_full().
This makes any backtrace generated as a consequence of a threaded error
useless (in particular any error happening in GEGL since we always
process these as multi-threaded, whether they are or not).
Instead I now keep track of the number of errors in gui-message.c, which
still allows to reset the counters when the unique debug dialog is
closed. Therefore I can now generate backtraces conditionally to the
error counters inside the problematic thread (and right when the error
happened), without any GTK+ call.
This finally makes GEGL backtraces useful in the debug dialog! :-)
app/gui/gui-message.c | 184 ++++++++++++++++++++++++++------------
app/widgets/gimpcriticaldialog.c | 25 -----
app/widgets/gimpcriticaldialog.h | 5 -
3 files changed, 127 insertions(+), 87 deletions(-)
---
diff --git a/app/gui/gui-message.c b/app/gui/gui-message.c
index bbaa3ca..1c2512a 100644
--- a/app/gui/gui-message.c
+++ b/app/gui/gui-message.c
@@ -50,33 +50,47 @@
#include "gimp-intl.h"
+#define MAX_TRACES 3
+#define MAX_ERRORS 10
+
+static GMutex mutex;
+static gint n_traces = 0;
+static gint n_errors = 0;
typedef struct
{
Gimp *gimp;
gchar *domain;
gchar *message;
+ gchar *trace;
GObject *handler;
GimpMessageSeverity severity;
} GimpLogMessageData;
-static gboolean gui_message_idle (gpointer user_data);
-static gboolean gui_message_error_console (Gimp *gimp,
- GimpMessageSeverity severity,
- const gchar *domain,
- const gchar *message);
-static gboolean gui_message_error_dialog (Gimp *gimp,
- GObject *handler,
- GimpMessageSeverity severity,
- const gchar *domain,
- const gchar *message);
-static void gui_message_console (GimpMessageSeverity severity,
- const gchar *domain,
- const gchar *message);
-static gchar * gui_message_format (GimpMessageSeverity severity,
- const gchar *domain,
- const gchar *message);
+static gboolean gui_message_idle (gpointer user_data);
+static gboolean gui_message_error_console (Gimp *gimp,
+ GimpMessageSeverity severity,
+ const gchar *domain,
+ const gchar *message);
+static gboolean gui_message_error_dialog (Gimp *gimp,
+ GObject *handler,
+ GimpMessageSeverity severity,
+ const gchar *domain,
+ const gchar *message,
+ const gchar *trace);
+static void gui_message_console (GimpMessageSeverity severity,
+ const gchar *domain,
+ const gchar *message);
+static gchar * gui_message_format (GimpMessageSeverity severity,
+ const gchar *domain,
+ const gchar *message);
+
+static GtkWidget * global_error_dialog (void);
+static GtkWidget * global_critical_dialog (void);
+
+static void gui_message_reset_errors (GtkObject *object,
+ gpointer user_data);
void
@@ -86,6 +100,9 @@ gui_message (Gimp *gimp,
const gchar *domain,
const gchar *message)
{
+ gchar *trace = NULL;
+ gboolean gen_trace = FALSE;
+
switch (gimp->message_handler)
{
case GIMP_ERROR_CONSOLE:
@@ -96,6 +113,30 @@ gui_message (Gimp *gimp,
/* fallthru */
case GIMP_MESSAGE_BOX:
+ if (severity == GIMP_MESSAGE_ERROR)
+ {
+ g_mutex_lock (&mutex);
+ /* Trace creation can be time consuming so don't block the
+ * mutex for too long and only increment and set a boolean
+ * here.
+ */
+ if (n_traces < MAX_TRACES)
+ {
+ gen_trace = TRUE;
+ n_traces++;
+ }
+ g_mutex_unlock (&mutex);
+ }
+ if (gen_trace)
+ {
+ /* We need to create the trace here because for multi-thread
+ * errors (i.e. non-GIMP ones), the backtrace after a idle
+ * function will simply be useless. It needs to happen in the
+ * buggy thread to be meaningful.
+ */
+ gimp_print_stack_trace (NULL, NULL, &trace);
+ }
+
if (g_strcmp0 (GIMP_ACRONYM, domain) != 0)
{
/* Handle non-GIMP messages in a multi-thread safe way,
@@ -108,6 +149,7 @@ gui_message (Gimp *gimp,
data->gimp = gimp;
data->domain = g_strdup (domain);
data->message = g_strdup (message);
+ data->trace = trace;
data->handler = handler? g_object_ref (handler) : NULL;
data->severity = severity;
@@ -117,8 +159,8 @@ gui_message (Gimp *gimp,
return;
}
if (gui_message_error_dialog (gimp, handler, severity,
- domain, message))
- return;
+ domain, message, trace))
+ break;
gimp->message_handler = GIMP_CONSOLE;
/* fallthru */
@@ -127,6 +169,8 @@ gui_message (Gimp *gimp,
gui_message_console (severity, domain, message);
break;
}
+ if (trace)
+ g_free (trace);
}
static gboolean
@@ -138,7 +182,8 @@ gui_message_idle (gpointer user_data)
data->handler,
data->severity,
data->domain,
- data->message))
+ data->message,
+ data->trace))
{
gui_message_console (data->severity,
data->domain,
@@ -146,6 +191,8 @@ gui_message_idle (gpointer user_data)
}
g_free (data->domain);
g_free (data->message);
+ if (data->trace)
+ g_free (data->trace);
if (data->handler)
g_object_unref (data->handler);
@@ -245,42 +292,13 @@ progress_error_dialog (GimpProgress *progress)
return dialog;
}
-static GtkWidget *
-global_error_dialog (void)
-{
- GdkScreen *screen;
- gint monitor;
-
- monitor = gimp_get_monitor_at_pointer (&screen);
-
- return gimp_dialog_factory_dialog_new (gimp_dialog_factory_get_singleton (),
- screen, monitor,
- NULL /*ui_manager*/,
- "gimp-error-dialog", -1,
- FALSE);
-}
-
-static GtkWidget *
-global_critical_dialog (void)
-{
- GdkScreen *screen;
- gint monitor;
-
- monitor = gimp_get_monitor_at_pointer (&screen);
-
- return gimp_dialog_factory_dialog_new (gimp_dialog_factory_get_singleton (),
- screen, monitor,
- NULL /*ui_manager*/,
- "gimp-critical-dialog", -1,
- FALSE);
-}
-
static gboolean
gui_message_error_dialog (Gimp *gimp,
GObject *handler,
GimpMessageSeverity severity,
const gchar *domain,
- const gchar *message)
+ const gchar *message,
+ const gchar *trace)
{
GtkWidget *dialog;
GtkMessageType type = GTK_MESSAGE_ERROR;
@@ -300,15 +318,21 @@ gui_message_error_dialog (Gimp *gimp,
* will require its own dedicated dialog which will encourage
* people to report the bug.
*/
- dialog = global_critical_dialog ();
+ gboolean gui_error = FALSE;
+
+ g_mutex_lock (&mutex);
+ if (n_errors < MAX_ERRORS)
+ {
+ gui_error = TRUE;
+ n_errors++;
+ }
+ g_mutex_unlock (&mutex);
- if (gimp_critical_dialog_want_errors (dialog))
+ if (gui_error || trace)
{
gchar *text;
- gchar *trace = NULL;
- if (gimp_critical_dialog_want_traces (dialog))
- gimp_print_stack_trace (NULL, NULL, &trace);
+ dialog = global_critical_dialog ();
text = gui_message_format (severity, domain, message);
gimp_critical_dialog_add (dialog, text, trace, FALSE, NULL, 0);
@@ -316,8 +340,6 @@ gui_message_error_dialog (Gimp *gimp,
gtk_widget_show (dialog);
g_free (text);
- if (trace)
- g_free (trace);
return TRUE;
}
@@ -410,3 +432,51 @@ gui_message_format (GimpMessageSeverity severity,
return formatted_message;
}
+
+static GtkWidget *
+global_error_dialog (void)
+{
+ GdkScreen *screen;
+ gint monitor;
+
+ monitor = gimp_get_monitor_at_pointer (&screen);
+
+ return gimp_dialog_factory_dialog_new (gimp_dialog_factory_get_singleton (),
+ screen, monitor,
+ NULL /*ui_manager*/,
+ "gimp-error-dialog", -1,
+ FALSE);
+}
+
+static GtkWidget *
+global_critical_dialog (void)
+{
+ GtkWidget *dialog;
+ GdkScreen *screen;
+ gint monitor;
+
+ monitor = gimp_get_monitor_at_pointer (&screen);
+
+ dialog = gimp_dialog_factory_dialog_new (gimp_dialog_factory_get_singleton (),
+ screen, monitor,
+ NULL /*ui_manager*/,
+ "gimp-critical-dialog", -1,
+ FALSE);
+ g_signal_handlers_disconnect_by_func (dialog,
+ gui_message_reset_errors,
+ NULL);
+ g_signal_connect (dialog, "destroy",
+ G_CALLBACK (gui_message_reset_errors),
+ NULL);
+ return dialog;
+}
+
+static void
+gui_message_reset_errors (GtkObject *object,
+ gpointer user_data)
+{
+ g_mutex_lock (&mutex);
+ n_errors = 0;
+ n_traces = 0;
+ g_mutex_unlock (&mutex);
+}
diff --git a/app/widgets/gimpcriticaldialog.c b/app/widgets/gimpcriticaldialog.c
index 5e989ca..20b3164 100644
--- a/app/widgets/gimpcriticaldialog.c
+++ b/app/widgets/gimpcriticaldialog.c
@@ -54,9 +54,6 @@
#define BUTTON1_TEXT _("Copy Bug Information")
#define BUTTON2_TEXT _("Open Bug Tracker")
-#define MAX_TRACES 3
-#define MAX_ERRORS 10
-
static void gimp_critical_dialog_finalize (GObject *object);
static void gimp_critical_dialog_response (GtkDialog *dialog,
gint response_id);
@@ -188,8 +185,6 @@ gimp_critical_dialog_init (GimpCriticalDialog *dialog)
dialog->pid = 0;
dialog->program = NULL;
- dialog->n_errors = 0;
- dialog->n_traces = 0;
}
static void
@@ -408,14 +403,6 @@ gimp_critical_dialog_add (GtkWidget *dialog,
}
critical = GIMP_CRITICAL_DIALOG (dialog);
- if (critical->n_errors > MAX_ERRORS ||
- (trace && critical->n_traces > MAX_TRACES))
- return;
-
- critical->n_errors++;
- if (trace)
- critical->n_traces++;
-
/* The user text, which should be localized. */
if (is_fatal)
{
@@ -492,15 +479,3 @@ gimp_critical_dialog_add (GtkWidget *dialog,
critical->pid = pid;
}
}
-
-gboolean
-gimp_critical_dialog_want_traces (GtkWidget *dialog)
-{
- return (GIMP_CRITICAL_DIALOG (dialog)->n_traces <= MAX_TRACES);
-}
-
-gboolean
-gimp_critical_dialog_want_errors (GtkWidget *dialog)
-{
- return (GIMP_CRITICAL_DIALOG (dialog)->n_errors <= MAX_ERRORS);
-}
diff --git a/app/widgets/gimpcriticaldialog.h b/app/widgets/gimpcriticaldialog.h
index 5418e79..7e94ea2 100644
--- a/app/widgets/gimpcriticaldialog.h
+++ b/app/widgets/gimpcriticaldialog.h
@@ -45,9 +45,6 @@ struct _GimpCriticalDialog
gchar *program;
gint pid;
-
- gint n_errors;
- gint n_traces;
};
struct _GimpCriticalDialogClass
@@ -65,8 +62,6 @@ void gimp_critical_dialog_add (GtkWidget *dialog,
gboolean is_fatal,
const gchar *program,
gint pid);
-gboolean gimp_critical_dialog_want_traces (GtkWidget *dialog);
-gboolean gimp_critical_dialog_want_errors (GtkWidget *dialog);
G_END_DECLS
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
