[gimp] app: fix core crash when a plug-in calling a GimpPdbDialog crashes.



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]