[gimp] app: fix warnings and crashes in rectangle select and crop



commit ff915b68c967c6d3f83119e5e5bf4840ef710446
Author: Michael Natterer <mitch gimp org>
Date:   Thu Jun 29 15:06:27 2017 +0200

    app: fix warnings and crashes in rectangle select and crop
    
    We can't rely on g_object_unref() in halt() for breaking all property
    GBindings between the tool options and GimpToolRectangle, because we
    might be in the middle of a signal emission which also refs and keeps
    the rectangle alive until the last callback returns. So we had
    dangling rectangles interacting with tool options.
    
    Remember all bindings in a list, and break them explicitly when we
    shut down the rectangle in halt().
    
    Also, forgot to unset the display's highlight in the rectangle
    selection tool.

 app/tools/gimpcroptool.c            |   24 +++++++++++++----
 app/tools/gimpcroptool.h            |    1 +
 app/tools/gimprectangleselecttool.c |   47 ++++++++++++++++++++++------------
 3 files changed, 49 insertions(+), 23 deletions(-)
---
diff --git a/app/tools/gimpcroptool.c b/app/tools/gimpcroptool.c
index 676d631..fe273ff 100644
--- a/app/tools/gimpcroptool.c
+++ b/app/tools/gimpcroptool.c
@@ -218,8 +218,8 @@ gimp_crop_tool_button_press (GimpTool            *tool,
       gimp_tool_widget_hover (crop_tool->widget, coords, state, TRUE);
 
       /* HACK: force CREATING on a newly created rectangle; otherwise,
-       * the above binding of properties would cause the rectangle to
-       * start with the size from tool options.
+       * property bindings would cause the rectangle to start with the
+       * size from tool options.
        */
       gimp_tool_rectangle_set_function (GIMP_TOOL_RECTANGLE (crop_tool->widget),
                                         GIMP_TOOL_RECTANGLE_CREATING);
@@ -357,10 +357,15 @@ gimp_crop_tool_start (GimpCropTool *crop_tool,
   gimp_draw_tool_set_widget (GIMP_DRAW_TOOL (tool), widget);
 
   for (i = 0; i < G_N_ELEMENTS (properties); i++)
-    g_object_bind_property (G_OBJECT (options), properties[i],
-                            G_OBJECT (widget),  properties[i],
-                            G_BINDING_SYNC_CREATE |
-                            G_BINDING_BIDIRECTIONAL);
+    {
+      GBinding *binding =
+        g_object_bind_property (G_OBJECT (options), properties[i],
+                                G_OBJECT (widget),  properties[i],
+                                G_BINDING_SYNC_CREATE |
+                                G_BINDING_BIDIRECTIONAL);
+
+      crop_tool->bindings = g_list_prepend (crop_tool->bindings, binding);
+    }
 
   g_signal_connect (widget, "changed",
                     G_CALLBACK (gimp_crop_tool_rectangle_changed),
@@ -467,6 +472,13 @@ gimp_crop_tool_halt (GimpCropTool *crop_tool)
   if (gimp_draw_tool_is_active (GIMP_DRAW_TOOL (tool)))
     gimp_draw_tool_stop (GIMP_DRAW_TOOL (tool));
 
+  /*  disconnect bindings manually so they are really gone *now*, we
+   *  might be in the middle of a signal emission that keeps the
+   *  widget and its bindings alive.
+   */
+  g_list_free_full (crop_tool->bindings, (GDestroyNotify) g_object_unref);
+  crop_tool->bindings = NULL;
+
   gimp_draw_tool_set_widget (GIMP_DRAW_TOOL (tool), NULL);
   g_clear_object (&crop_tool->widget);
 
diff --git a/app/tools/gimpcroptool.h b/app/tools/gimpcroptool.h
index 719ac9f..75e618c 100644
--- a/app/tools/gimpcroptool.h
+++ b/app/tools/gimpcroptool.h
@@ -43,6 +43,7 @@ struct _GimpCropTool
 
   GimpToolWidget *widget;
   GimpToolWidget *grab_widget;
+  GList          *bindings;
 };
 
 struct _GimpCropToolClass
diff --git a/app/tools/gimprectangleselecttool.c b/app/tools/gimprectangleselecttool.c
index 2d0a1ed..1863a1d 100644
--- a/app/tools/gimprectangleselecttool.c
+++ b/app/tools/gimprectangleselecttool.c
@@ -63,6 +63,7 @@ struct _GimpRectangleSelectToolPrivate
 
   GimpToolWidget    *widget;
   GimpToolWidget    *grab_widget;
+  GList             *bindings;
 };
 
 
@@ -678,10 +679,15 @@ gimp_rectangle_select_tool_start (GimpRectangleSelectTool *rect_tool,
   gimp_draw_tool_set_widget (GIMP_DRAW_TOOL (tool), widget);
 
   for (i = 0; i < G_N_ELEMENTS (properties); i++)
-    g_object_bind_property (G_OBJECT (options), properties[i],
-                            G_OBJECT (widget),  properties[i],
-                            G_BINDING_SYNC_CREATE |
-                            G_BINDING_BIDIRECTIONAL);
+    {
+      GBinding *binding =
+        g_object_bind_property (G_OBJECT (options), properties[i],
+                                G_OBJECT (widget),  properties[i],
+                                G_BINDING_SYNC_CREATE |
+                                G_BINDING_BIDIRECTIONAL);
+
+      private->bindings = g_list_prepend (private->bindings, binding);
+    }
 
   g_signal_connect (widget, "response",
                     G_CALLBACK (gimp_rectangle_select_tool_rectangle_response),
@@ -821,9 +827,16 @@ gimp_rectangle_select_tool_halt (GimpRectangleSelectTool *rect_tool)
 
   if (tool->display)
     {
-      GimpImage     *image      = gimp_display_get_image (tool->display);
-      GimpUndoStack *undo_stack = gimp_image_get_undo_stack (image);
-      GimpUndo      *undo       = gimp_undo_stack_peek (undo_stack);
+      GimpDisplayShell *shell      = gimp_display_get_shell (tool->display);
+      GimpImage        *image      = gimp_display_get_image (tool->display);
+      GimpUndoStack    *undo_stack = gimp_image_get_undo_stack (image);
+      GimpUndo         *undo       = gimp_undo_stack_peek (undo_stack);
+
+      gimp_display_shell_set_highlight (shell, NULL);
+
+      gimp_rectangle_options_disconnect (GIMP_RECTANGLE_OPTIONS (options),
+                                         G_CALLBACK (gimp_rectangle_select_tool_auto_shrink),
+                                         rect_tool);
 
       /* if we have an existing rectangle in the current display, then
        * we have already "executed", and need to undo at this point,
@@ -839,24 +852,27 @@ gimp_rectangle_select_tool_halt (GimpRectangleSelectTool *rect_tool)
 
           gimp_tool_control_pop_preserve (tool->control);
         }
-
-      gimp_rectangle_options_disconnect (GIMP_RECTANGLE_OPTIONS (options),
-                                         G_CALLBACK (gimp_rectangle_select_tool_auto_shrink),
-                                         rect_tool);
     }
 
-  gimp_rectangle_select_tool_update_option_defaults (rect_tool, TRUE);
-
   priv->undo = NULL;
   priv->redo = NULL;
 
   if (gimp_draw_tool_is_active (GIMP_DRAW_TOOL (tool)))
     gimp_draw_tool_stop (GIMP_DRAW_TOOL (tool));
 
+  /*  disconnect bindings manually so they are really gone *now*, we
+   *  might be in the middle of a signal emission that keeps the
+   *  widget and its bindings alive.
+   */
+  g_list_free_full (priv->bindings, (GDestroyNotify) g_object_unref);
+  priv->bindings = NULL;
+
   gimp_draw_tool_set_widget (GIMP_DRAW_TOOL (tool), NULL);
   g_clear_object (&priv->widget);
 
   tool->display = NULL;
+
+  gimp_rectangle_select_tool_update_option_defaults (rect_tool, TRUE);
 }
 
 static GimpChannelOps
@@ -891,10 +907,7 @@ gimp_rectangle_select_tool_update_option_defaults (GimpRectangleSelectTool *rect
 
   rect_options = GIMP_RECTANGLE_OPTIONS (gimp_tool_get_options (tool));
 
-  if (! priv->widget)
-    return;
-
-  if (tool->display != NULL && ! ignore_pending)
+  if (priv->widget && ! ignore_pending)
     {
       /* There is a pending rectangle and we should not ignore it, so
        * set default Fixed: Size to the same as the current pending


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]