mirror of
https://gitlab.gnome.org/GNOME/gimp.git
synced 2025-07-03 17:33:25 +00:00
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).
This commit is contained in:
Jehan
parent
2967ab7f82
commit
4ee3a9caa1
5 changed files with 64 additions and 30 deletions
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -48,6 +48,10 @@ struct _GimpBrushFactoryView
|
|||
struct _GimpBrushFactoryViewClass
|
||||
{
|
||||
GimpDataFactoryViewClass parent_class;
|
||||
|
||||
/* Signals */
|
||||
|
||||
void (* spacing_changed) (GimpBrushFactoryView *view);
|
||||
};
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -76,7 +76,7 @@ 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,
|
||||
static void gimp_brush_select_spacing_update (GimpBrushFactoryView *view,
|
||||
GimpBrushSelect *select);
|
||||
|
||||
|
||||
|
|
@ -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,
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
gimp_pdb_dialog_run_callback (&dialog, TRUE);
|
||||
if (dialog)
|
||||
gtk_widget_destroy (GTK_WIDGET (dialog));
|
||||
}
|
||||
|
||||
void
|
||||
gimp_pdb_dialog_run_callback (GimpPdbDialog *dialog,
|
||||
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 &&
|
||||
if (*dialog && object &&
|
||||
klass->run_callback &&
|
||||
dialog->callback_name &&
|
||||
! dialog->callback_busy)
|
||||
(*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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue