[gimp] app: fix core crash when a plug-in calling a GimpPdbDialog crashes.
- From: Jehan <jehanp src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gimp] app: fix core crash when a plug-in calling a GimpPdbDialog crashes.
- Date: Wed, 28 Apr 2021 00:51:34 +0000 (UTC)
commit 4ee3a9caa110ab41a5985ae5497c18dfab27016d
Author: Jehan <jehan girinstud io>
Date: Wed Apr 28 02:04:41 2021 +0200
app: fix core crash when a plug-in calling a GimpPdbDialog crashes.
There are 2 parts for this fix:
- First expect the GimpPdbDialog to possibly disappear while
gimp_pdb_dialog_run_callback() is running. This can indeed happen as
this core dialog is tied to a PDB call. If the calling processus
crashes (which may happen, and has to be expected for third-party
plug-ins), then this dialog may just end up closing at anytime (signal
"plug-in-closed" from the plug-in manager which implies a
GTK_RESPONSE_CLOSE, hence dialog destruction).
To account for this, we check the dialog availability with a weak
pointer and returns the info to the caller as well.
- Don't connect to "value-changed" on the spacing adjustment because
when a crash happened, I had cases when the adjustment was finalized
while being set (crash in GTK code). This one is a bit harder to
explain (I had to look long at backtraces) but having a proper signal
"spacing-changed" on the GimpBrushFactoryView is actually much cleaner
code than relying on a public object anyway and it fixes this crash.
Note: this fix is related to my previous commit. When running
gimp_brush_select_new() from Python code, the plug-in crashed when
trying to run the callback, which also resulted into core crash (and
this part is obviously not acceptable at all).
app/widgets/gimpbrushfactoryview.c | 23 +++++++++++++++++++
app/widgets/gimpbrushfactoryview.h | 4 ++++
app/widgets/gimpbrushselect.c | 18 +++++++--------
app/widgets/gimppdbdialog.c | 47 ++++++++++++++++++++++----------------
app/widgets/gimppdbdialog.h | 2 +-
5 files changed, 64 insertions(+), 30 deletions(-)
---
diff --git a/app/widgets/gimpbrushfactoryview.c b/app/widgets/gimpbrushfactoryview.c
index f3e3a11107..c1e7f67b3b 100644
--- a/app/widgets/gimpbrushfactoryview.c
+++ b/app/widgets/gimpbrushfactoryview.c
@@ -42,6 +42,12 @@
#include "gimp-intl.h"
+enum
+{
+ SPACING_CHANGED,
+ LAST_SIGNAL
+};
+
static void gimp_brush_factory_view_dispose (GObject *object);
@@ -59,6 +65,8 @@ G_DEFINE_TYPE (GimpBrushFactoryView, gimp_brush_factory_view,
#define parent_class gimp_brush_factory_view_parent_class
+static guint gimp_brush_factory_view_signals[LAST_SIGNAL] = { 0 };
+
static void
gimp_brush_factory_view_class_init (GimpBrushFactoryViewClass *klass)
@@ -69,6 +77,20 @@ gimp_brush_factory_view_class_init (GimpBrushFactoryViewClass *klass)
object_class->dispose = gimp_brush_factory_view_dispose;
editor_class->select_item = gimp_brush_factory_view_select_item;
+
+ /**
+ * GimpBrushFactoryView::spacing-changed:
+ * @view: the #GimpBrushFactoryView.
+ *
+ * Emitted when the spacing changed.
+ */
+ gimp_brush_factory_view_signals[SPACING_CHANGED] =
+ g_signal_new ("spacing-changed",
+ G_TYPE_FROM_CLASS (klass),
+ G_SIGNAL_RUN_FIRST,
+ G_STRUCT_OFFSET (GimpBrushFactoryViewClass, spacing_changed),
+ NULL, NULL, NULL,
+ G_TYPE_NONE, 0);
}
static void
@@ -249,4 +271,5 @@ gimp_brush_factory_view_spacing_update (GtkAdjustment *adjustment,
gimp_brush_factory_view_spacing_changed,
view);
}
+ g_signal_emit (view, gimp_brush_factory_view_signals[SPACING_CHANGED], 0);
}
diff --git a/app/widgets/gimpbrushfactoryview.h b/app/widgets/gimpbrushfactoryview.h
index dce4c3c837..e3d547d1e4 100644
--- a/app/widgets/gimpbrushfactoryview.h
+++ b/app/widgets/gimpbrushfactoryview.h
@@ -48,6 +48,10 @@ struct _GimpBrushFactoryView
struct _GimpBrushFactoryViewClass
{
GimpDataFactoryViewClass parent_class;
+
+ /* Signals */
+
+ void (* spacing_changed) (GimpBrushFactoryView *view);
};
diff --git a/app/widgets/gimpbrushselect.c b/app/widgets/gimpbrushselect.c
index a2bf9d71bc..b5e374ecb2 100644
--- a/app/widgets/gimpbrushselect.c
+++ b/app/widgets/gimpbrushselect.c
@@ -76,8 +76,8 @@ static void gimp_brush_select_mode_changed (GimpContext *context
static void gimp_brush_select_opacity_update (GtkAdjustment *adj,
GimpBrushSelect *select);
-static void gimp_brush_select_spacing_update (GtkAdjustment *adj,
- GimpBrushSelect *select);
+static void gimp_brush_select_spacing_update (GimpBrushFactoryView *view,
+ GimpBrushSelect *select);
G_DEFINE_TYPE (GimpBrushSelect, gimp_brush_select, GIMP_TYPE_PDB_DIALOG)
@@ -207,7 +207,7 @@ gimp_brush_select_constructed (GObject *object)
if (select->spacing >= 0)
gtk_adjustment_set_value (spacing_adj, select->spacing);
- g_signal_connect (spacing_adj, "value-changed",
+ g_signal_connect (dialog->view, "spacing-changed",
G_CALLBACK (gimp_brush_select_spacing_update),
select);
}
@@ -313,7 +313,7 @@ gimp_brush_select_opacity_changed (GimpContext *context,
gimp_brush_select_opacity_update,
select);
- gimp_pdb_dialog_run_callback (GIMP_PDB_DIALOG (select), FALSE);
+ gimp_pdb_dialog_run_callback ((GimpPdbDialog **) &select, FALSE);
}
static void
@@ -321,7 +321,7 @@ gimp_brush_select_mode_changed (GimpContext *context,
GimpLayerMode paint_mode,
GimpBrushSelect *select)
{
- gimp_pdb_dialog_run_callback (GIMP_PDB_DIALOG (select), FALSE);
+ gimp_pdb_dialog_run_callback ((GimpPdbDialog **) &select, FALSE);
}
static void
@@ -333,15 +333,15 @@ gimp_brush_select_opacity_update (GtkAdjustment *adjustment,
}
static void
-gimp_brush_select_spacing_update (GtkAdjustment *adjustment,
- GimpBrushSelect *select)
+gimp_brush_select_spacing_update (GimpBrushFactoryView *view,
+ GimpBrushSelect *select)
{
- gdouble value = gtk_adjustment_get_value (adjustment);
+ gdouble value = gtk_adjustment_get_value (view->spacing_adjustment);
if (select->spacing != value)
{
select->spacing = value;
- gimp_pdb_dialog_run_callback (GIMP_PDB_DIALOG (select), FALSE);
+ gimp_pdb_dialog_run_callback ((GimpPdbDialog **) &select, FALSE);
}
}
diff --git a/app/widgets/gimppdbdialog.c b/app/widgets/gimppdbdialog.c
index 3e78162f32..b885fe2e0f 100644
--- a/app/widgets/gimppdbdialog.c
+++ b/app/widgets/gimppdbdialog.c
@@ -244,34 +244,37 @@ gimp_pdb_dialog_response (GtkDialog *gtk_dialog,
{
GimpPdbDialog *dialog = GIMP_PDB_DIALOG (gtk_dialog);
- gimp_pdb_dialog_run_callback (dialog, TRUE);
- gtk_widget_destroy (GTK_WIDGET (dialog));
+ gimp_pdb_dialog_run_callback (&dialog, TRUE);
+ if (dialog)
+ gtk_widget_destroy (GTK_WIDGET (dialog));
}
void
-gimp_pdb_dialog_run_callback (GimpPdbDialog *dialog,
- gboolean closing)
+gimp_pdb_dialog_run_callback (GimpPdbDialog **dialog,
+ gboolean closing)
{
- GimpPdbDialogClass *klass = GIMP_PDB_DIALOG_GET_CLASS (dialog);
+ GimpPdbDialogClass *klass = GIMP_PDB_DIALOG_GET_CLASS (*dialog);
GimpObject *object;
- object = gimp_context_get_by_type (dialog->context, dialog->select_type);
+ g_object_add_weak_pointer (G_OBJECT (*dialog), (gpointer) dialog);
+ object = gimp_context_get_by_type ((*dialog)->context, (*dialog)->select_type);
- if (object &&
- klass->run_callback &&
- dialog->callback_name &&
- ! dialog->callback_busy)
+ if (*dialog && object &&
+ klass->run_callback &&
+ (*dialog)->callback_name &&
+ ! (*dialog)->callback_busy)
{
- dialog->callback_busy = TRUE;
+ (*dialog)->callback_busy = TRUE;
- if (gimp_pdb_lookup_procedure (dialog->pdb, dialog->callback_name))
+ if (gimp_pdb_lookup_procedure ((*dialog)->pdb, (*dialog)->callback_name))
{
GimpValueArray *return_vals;
GError *error = NULL;
- return_vals = klass->run_callback (dialog, object, closing, &error);
+ return_vals = klass->run_callback (*dialog, object, closing, &error);
- if (g_value_get_enum (gimp_value_array_index (return_vals, 0)) !=
+ if (*dialog &&
+ g_value_get_enum (gimp_value_array_index (return_vals, 0)) !=
GIMP_PDB_SUCCESS)
{
const gchar *message;
@@ -281,15 +284,15 @@ gimp_pdb_dialog_run_callback (GimpPdbDialog *dialog,
else
message = _("The corresponding plug-in may have crashed.");
- gimp_message (dialog->context->gimp, G_OBJECT (dialog),
+ gimp_message ((*dialog)->context->gimp, G_OBJECT (*dialog),
GIMP_MESSAGE_ERROR,
_("Unable to run %s callback.\n%s"),
- g_type_name (G_TYPE_FROM_INSTANCE (dialog)),
+ g_type_name (G_TYPE_FROM_INSTANCE (*dialog)),
message);
}
- else if (error)
+ else if (*dialog && error)
{
- gimp_message_literal (dialog->context->gimp, G_OBJECT (dialog),
+ gimp_message_literal ((*dialog)->context->gimp, G_OBJECT (*dialog),
GIMP_MESSAGE_ERROR,
error->message);
g_error_free (error);
@@ -298,8 +301,12 @@ gimp_pdb_dialog_run_callback (GimpPdbDialog *dialog,
gimp_value_array_unref (return_vals);
}
- dialog->callback_busy = FALSE;
+ if (*dialog)
+ (*dialog)->callback_busy = FALSE;
}
+
+ if (*dialog)
+ g_object_remove_weak_pointer (G_OBJECT (*dialog), (gpointer) dialog);
}
GimpPdbDialog *
@@ -332,7 +339,7 @@ gimp_pdb_dialog_context_changed (GimpContext *context,
GimpPdbDialog *dialog)
{
if (object)
- gimp_pdb_dialog_run_callback (dialog, FALSE);
+ gimp_pdb_dialog_run_callback (&dialog, FALSE);
}
static void
diff --git a/app/widgets/gimppdbdialog.h b/app/widgets/gimppdbdialog.h
index 9b5b16253f..b0f83cc2fc 100644
--- a/app/widgets/gimppdbdialog.h
+++ b/app/widgets/gimppdbdialog.h
@@ -74,7 +74,7 @@ struct _GimpPdbDialogClass
GType gimp_pdb_dialog_get_type (void) G_GNUC_CONST;
-void gimp_pdb_dialog_run_callback (GimpPdbDialog *dialog,
+void gimp_pdb_dialog_run_callback (GimpPdbDialog **dialog,
gboolean closing);
GimpPdbDialog * gimp_pdb_dialog_get_by_callback (GimpPdbDialogClass *klass,
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]