Bug 795257 - Segmentation fault crash using the clone tool

Commit f5cb1fed85, which performed
brush outline generation in GimpPaintTool in synchrony with the
paint thread, wasn't enough, since GimpSourceTool could still call
gimp_brush_tool_create_outline() directly during its
GimpDrawTool::draw() method, leading to the same race condition
when executed concurrently with the paint thread.

Partially revert the above commit, so that outline generation is
handled as before, as far as GimpPaintTool is concenered.  Instead,
add GimpPaintTool::{start,end,flush}_paint() virtual functions; the
first two are called when starting/ending painting using the paint
thread, while the third is called during the display-update
timeout, while the main thread and the paint thread are
synchronized.  This allows subclasses to perform non-thread-safe
actions while the threads are synchronized.

Override these functions in GimpBrushTool, and cache the brush
boundary in the flush() function.  Use the cached boundary in
gimp_brush_tool_create_outline() while painting, to avoid the above
race condition, both when this function is called through
GimpPaintTool, and through GimpSourceTool.

app/tools/gimpbrushtool.c

@@ -61,6 +61,9 @@ static void   gimp_brush_tool_options_notify  (GimpTool          *tool,
                                                GimpToolOptions   *options,
                                                const GParamSpec  *pspec);
 
+static void   gimp_brush_tool_start_paint     (GimpPaintTool     *paint_tool);
+static void   gimp_brush_tool_end_paint       (GimpPaintTool     *paint_tool);
+static void   gimp_brush_tool_flush_paint     (GimpPaintTool     *paint_tool);
 static GimpCanvasItem *
               gimp_brush_tool_get_outline     (GimpPaintTool     *paint_tool,
                                                GimpDisplay       *display,
@@ -74,6 +77,11 @@ static void   gimp_brush_tool_set_brush       (GimpBrushCore     *brush_core,
                                                GimpBrush         *brush,
                                                GimpBrushTool     *brush_tool);
 
+static const GimpBezierDesc *
+                 gimp_brush_tool_get_boundary (GimpBrushTool     *brush_tool,
+                                               gint              *width,
+                                               gint              *height);
+
 
 G_DEFINE_TYPE (GimpBrushTool, gimp_brush_tool, GIMP_TYPE_PAINT_TOOL)
 
@@ -93,6 +101,9 @@ gimp_brush_tool_class_init (GimpBrushToolClass *klass)
   tool_class->cursor_update     = gimp_brush_tool_cursor_update;
   tool_class->options_notify    = gimp_brush_tool_options_notify;
 
+  paint_tool_class->start_paint = gimp_brush_tool_start_paint;
+  paint_tool_class->end_paint   = gimp_brush_tool_end_paint;
+  paint_tool_class->flush_paint = gimp_brush_tool_flush_paint;
   paint_tool_class->get_outline = gimp_brush_tool_get_outline;
 }
 
@@ -221,6 +232,45 @@ gimp_brush_tool_options_notify (GimpTool         *tool,
     }
 }
 
+static void
+gimp_brush_tool_start_paint (GimpPaintTool *paint_tool)
+{
+  if (GIMP_PAINT_TOOL_CLASS (parent_class)->start_paint)
+    GIMP_PAINT_TOOL_CLASS (parent_class)->start_paint (paint_tool);
+
+  gimp_brush_tool_flush_paint (paint_tool);
+}
+
+static void
+gimp_brush_tool_end_paint (GimpPaintTool *paint_tool)
+{
+  GimpBrushTool *brush_tool = GIMP_BRUSH_TOOL (paint_tool);
+
+  g_clear_pointer (&brush_tool->boundary, gimp_bezier_desc_free);
+
+  if (GIMP_PAINT_TOOL_CLASS (parent_class)->end_paint)
+    GIMP_PAINT_TOOL_CLASS (parent_class)->end_paint (paint_tool);
+}
+
+static void
+gimp_brush_tool_flush_paint (GimpPaintTool *paint_tool)
+{
+  GimpBrushTool        *brush_tool = GIMP_BRUSH_TOOL (paint_tool);
+  const GimpBezierDesc *boundary;
+
+  if (GIMP_PAINT_TOOL_CLASS (parent_class)->flush_paint)
+    GIMP_PAINT_TOOL_CLASS (parent_class)->flush_paint (paint_tool);
+
+  g_clear_pointer (&brush_tool->boundary, gimp_bezier_desc_free);
+
+  boundary = gimp_brush_tool_get_boundary (brush_tool,
+                                           &brush_tool->boundary_width,
+                                           &brush_tool->boundary_height);
+
+  if (boundary)
+    brush_tool->boundary = gimp_bezier_desc_copy (boundary);
+}
+
 static GimpCanvasItem *
 gimp_brush_tool_get_outline (GimpPaintTool *paint_tool,
                              GimpDisplay   *display,
@@ -259,7 +309,6 @@ gimp_brush_tool_create_outline (GimpBrushTool *brush_tool,
                                 gdouble        x,
                                 gdouble        y)
 {
-  GimpBrushCore        *brush_core;
   GimpPaintOptions     *options;
   GimpDisplayShell     *shell;
   const GimpBezierDesc *boundary = NULL;
@@ -269,29 +318,25 @@ gimp_brush_tool_create_outline (GimpBrushTool *brush_tool,
   g_return_val_if_fail (GIMP_IS_BRUSH_TOOL (brush_tool), NULL);
   g_return_val_if_fail (GIMP_IS_DISPLAY (display), NULL);
 
-  if (! GIMP_PAINT_TOOL (brush_tool)->draw_brush)
+  if (gimp_paint_tool_is_painting (GIMP_PAINT_TOOL (brush_tool)))
+    {
+      boundary = brush_tool->boundary;
+      width    = brush_tool->boundary_width;
+      height   = brush_tool->boundary_height;
+    }
+  else
+    {
+      boundary = gimp_brush_tool_get_boundary (brush_tool, &width, &height);
+    }
+
+  if (! boundary)
     return NULL;
 
-  brush_core = GIMP_BRUSH_CORE (GIMP_PAINT_TOOL (brush_tool)->core);
   options = GIMP_PAINT_TOOL_GET_OPTIONS (brush_tool);
   shell   = gimp_display_get_shell (display);
 
-  if (! brush_core->main_brush || ! brush_core->dynamics)
-    return NULL;
-
-  if (brush_core->scale > 0.0)
-    boundary = gimp_brush_transform_boundary (brush_core->main_brush,
-                                              brush_core->scale,
-                                              brush_core->aspect_ratio,
-                                              brush_core->angle,
-                                              brush_core->reflect,
-                                              brush_core->hardness,
-                                              &width,
-                                              &height);
-
   /*  don't draw the boundary if it becomes too small  */
-  if (boundary                   &&
+  if (SCALEX (shell, width)  > 4 &&
-      SCALEX (shell, width)  > 4 &&
       SCALEY (shell, height) > 4)
     {
       x -= width  / 2.0;
@@ -347,3 +392,29 @@ gimp_brush_tool_set_brush (GimpBrushCore *brush_core,
 
   gimp_draw_tool_resume (GIMP_DRAW_TOOL (brush_tool));
 }
+
+static const GimpBezierDesc *
+gimp_brush_tool_get_boundary (GimpBrushTool *brush_tool,
+                              gint          *width,
+                              gint          *height)
+{
+  GimpPaintTool *paint_tool = GIMP_PAINT_TOOL (brush_tool);
+  GimpBrushCore *brush_core = GIMP_BRUSH_CORE (paint_tool->core);
+
+  if (paint_tool->draw_brush &&
+      brush_core->main_brush &&
+      brush_core->dynamics   &&
+      brush_core->scale > 0.0)
+    {
+      return gimp_brush_transform_boundary (brush_core->main_brush,
+                                            brush_core->scale,
+                                            brush_core->aspect_ratio,
+                                            brush_core->angle,
+                                            brush_core->reflect,
+                                            brush_core->hardness,
+                                            width,
+                                            height);
+    }
+
+  return NULL;
+}

app/tools/gimpbrushtool.h

@@ -35,6 +35,10 @@ typedef struct _GimpBrushToolClass GimpBrushToolClass;
 struct _GimpBrushTool
 {
   GimpPaintTool   parent_instance;
+
+  GimpBezierDesc *boundary;
+  gint            boundary_width;
+  gint            boundary_height;
 };
 
 struct _GimpBrushToolClass

app/tools/gimppainttool-paint.c

@@ -73,8 +73,6 @@ static gpointer   gimp_paint_tool_paint_thread         (gpointer       data);
 
 static gboolean   gimp_paint_tool_paint_timeout    (GimpPaintTool *paint_tool);
 
-static void       gimp_paint_tool_paint_update_outline (GimpPaintTool *paint_tool);
-
 
 /*  static variables  */
 
@@ -182,8 +180,8 @@ gimp_paint_tool_paint_timeout (GimpPaintTool *paint_tool)
 
   update = gimp_drawable_flush_paint (drawable);
 
-  if (update)
+  if (update && GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->flush_paint)
-    gimp_paint_tool_paint_update_outline (paint_tool);
+    GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->flush_paint (paint_tool);
 
   paint_timeout_pending = FALSE;
   g_cond_signal (&paint_cond);
@@ -207,36 +205,6 @@ gimp_paint_tool_paint_timeout (GimpPaintTool *paint_tool)
   return G_SOURCE_CONTINUE;
 }
 
-static void
-gimp_paint_tool_paint_update_outline (GimpPaintTool *paint_tool)
-{
-  if (gimp_paint_tool_paint_use_thread (paint_tool))
-    {
-      gimp_paint_tool_set_draw_fallback (paint_tool, FALSE, 0.0);
-
-      if (paint_tool->draw_brush)
-        {
-          GimpPaintCore *core     = paint_tool->core;
-          GimpDisplay   *display  = paint_tool->display;
-          GimpDrawable  *drawable = paint_tool->drawable;
-          gint           off_x, off_y;
-          gdouble        x, y;
-
-          gimp_item_get_offset (GIMP_ITEM (drawable), &off_x, &off_y);
-
-          x = core->cur_coords.x + off_x;
-          y = core->cur_coords.y + off_y;
-
-          if (paint_tool->outline)
-            g_object_unref (paint_tool->outline);
-
-          paint_tool->outline =
-            GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->get_outline (paint_tool,
-                                                                 display, x, y);
-        }
-    }
-}
-
 
 /*  public functions  */
 
@@ -322,6 +290,13 @@ gimp_paint_tool_paint_start (GimpPaintTool     *paint_tool,
         gimp_display_shell_get_constrained_line_offset_angle (shell));
     }
 
+  /*  Notify subclasses  */
+  if (gimp_paint_tool_paint_use_thread (paint_tool) &&
+      GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->start_paint)
+    {
+      GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->start_paint (paint_tool);
+    }
+
   /*  Let the specific painting function initialize itself  */
   gimp_paint_core_paint (core, drawable, paint_options,
                          GIMP_PAINT_STATE_INIT, time);
@@ -338,9 +313,6 @@ gimp_paint_tool_paint_start (GimpPaintTool     *paint_tool,
                              GIMP_PAINT_STATE_MOTION, time);
     }
 
-  /*  Update the brush outline  */
-  gimp_paint_tool_paint_update_outline (paint_tool);
-
   gimp_projection_flush_now (gimp_image_get_projection (image));
   gimp_display_flush_now (display);
 
@@ -422,8 +394,12 @@ gimp_paint_tool_paint_end (GimpPaintTool *paint_tool,
   else
     gimp_paint_core_finish (core, drawable, TRUE);
 
-  /*  Clear the brush outline  */
+  /*  Notify subclasses  */
-  g_clear_object (&paint_tool->outline);
+  if (gimp_paint_tool_paint_use_thread (paint_tool) &&
+      GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->end_paint)
+    {
+      GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->end_paint (paint_tool);
+    }
 
   /*  Exit paint mode  */
   if (gimp_paint_tool_paint_use_thread (paint_tool))

app/tools/gimppainttool.c

@@ -672,6 +672,9 @@ gimp_paint_tool_draw (GimpDrawTool *draw_tool)
           line_drawn = TRUE;
         }
 
+      gimp_paint_tool_set_draw_fallback (paint_tool, FALSE, 0.0);
+
+      if (paint_tool->draw_brush)
         outline = gimp_paint_tool_get_outline (paint_tool,
                                                draw_tool->display,
                                                cur_x, cur_y);
@@ -767,22 +770,9 @@ gimp_paint_tool_get_outline (GimpPaintTool *paint_tool,
                              gdouble        x,
                              gdouble        y)
 {
-  if (paint_tool->drawable && gimp_drawable_is_painting (paint_tool->drawable))
+  if (GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->get_outline)
-    {
+    return GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->get_outline (paint_tool,
-      if (paint_tool->outline)
+                                                                display, x, y);
-        return g_object_ref (paint_tool->outline);
-    }
-  else
-    {
-      gimp_paint_tool_set_draw_fallback (paint_tool, FALSE, 0.0);
-
-      if (paint_tool->draw_brush &&
-          GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->get_outline)
-        {
-          return GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->get_outline (
-            paint_tool, display, x, y);
-        }
-    }
 
   return NULL;
 }
@@ -853,3 +843,9 @@ gimp_paint_tool_set_draw_circle (GimpPaintTool *tool,
   tool->draw_circle = draw_circle;
   tool->circle_size = circle_size;
 }
+
+gboolean
+gimp_paint_tool_is_painting (GimpPaintTool *tool)
+{
+  return tool->drawable != NULL && gimp_drawable_is_painting (tool->drawable);
+}

app/tools/gimppainttool.h

@@ -59,14 +59,16 @@ struct _GimpPaintTool
 
   GimpDisplay   *display;
   GimpDrawable  *drawable;
-
-  GimpCanvasItem *outline;
 };
 
 struct _GimpPaintToolClass
 {
   GimpColorToolClass  parent_class;
 
+  void             (* start_paint) (GimpPaintTool *paint_tool);
+  void             (* end_paint)   (GimpPaintTool *paint_tool);
+  void             (* flush_paint) (GimpPaintTool *paint_tool);
+
   GimpCanvasItem * (* get_outline) (GimpPaintTool *paint_tool,
                                     GimpDisplay   *display,
                                     gdouble        x,
@@ -86,4 +88,8 @@ void    gimp_paint_tool_set_draw_fallback   (GimpPaintTool     *tool,
 void       gimp_paint_tool_set_draw_circle     (GimpPaintTool     *tool,
                                                 gboolean           draw_circle,
                                                 gint               circle_size);
+
+gboolean   gimp_paint_tool_is_painting         (GimpPaintTool     *tool);
+
+
 #endif  /*  __GIMP_PAINT_TOOL_H__  */