Infinite loop in `DrawAllShapes()` after saving, when live shapes belong to two different shape tools

[zornlemma]

Summary

If two or more un-finalized (still editable) shapes exist and they belong to different shape tools, saving the document writes the file correctly and then hangs the UI thread in an infinite loop. CPU pins to 100% and memory grows without bound, roughly 13 MB/s in my measurements, until the process is killed. The window never repaints again, so the app has to be killed from a terminal.

The cause is that DrawAllShapes() uses the mutable instance field SelectedShapeIndex as its for loop counter, and its loop body switches the active tool, which resets that same field to -1 via HandleDeactivated(). The loop counter can therefore never advance past the first shape.

Environment

• Pinta 3.1.2, Flatpak from Flathub (com.github.PintaProject.Pinta, commit a105c6c88f33ed7daf1cb2d28c75673bbfa8752e3a3ea3d44b587bee01a3ac72)
• Runtime org.gnome.Platform/x86_64/50
• Debian GNU/Linux 13 (trixie), kernel 6.12.95+deb13-amd64
• GNOME Shell 48.7 on X11

The offending code is identical in the 3.1.2 tag and current master, so master should still be affected.

Steps to reproduce

1. Open any image.
2. Select the Rounded Rectangle tool and drag out a shape. History shows Rounded Line Shape Added. Leave it un-finalized (control points still visible).
3. Select the Line/Curve tool and drag out a second shape. History shows Open Curve Shape Added. Both shapes are still live, because switching between two shape tools does not finalize them.
4. Press Ctrl+S (or close the window and choose Save).

Expected

The file is saved, both shapes stay editable, and the app stays responsive.

Actual

The file is written to disk correctly, then the UI freezes permanently. CPU goes to 100% on the main thread and RSS climbs continuously. In my case a real session reached 4.5 GB RSS before I killed it; at that point the memory pressure also made GNOME's "Application is not responding / Force Quit" path unusable, so kill -9 from a terminal was the only way out.

No Finalized history item appears, and the title bar keeps its unsaved-changes marker even though the file on disk is correct.

What determines whether it hangs

The trigger is not simply "more than one shape". It is "live shapes owned by more than one shape tool". Tested on 3.1.2 Flatpak with a scripted repro:

Scenario at save time	Result
1 live shape	Saves fine, stays responsive
2 live shapes, both from the same shape tool (2x Rounded Rectangle)	Saves fine, Finalized appears, stays responsive
2 live shapes, from different shape tools (Rounded Rectangle + Line/Curve)	Hangs, unbounded CPU and memory
2 live shapes from different tools, but Enter pressed before saving	Hangs (identical behaviour)
2 live shapes from different tools, but switch to a non-shape tool before saving	Saves fine, stays responsive

The last row is the workaround, and it also fits the diagnosis: HandleDeactivated() finalizes all shapes when switching to a non-shape tool, so SEngines is empty by the time the save path runs and DrawAllShapes() has nothing to iterate.

Root cause

Line numbers below are for the 3.1.2 tag, with master in parentheses.

SelectedShapeIndex is a mutable public instance field, and SEngines is static and shared between every shape tool's edit engine:

// BaseEditEngine.cs:130 (master 131)
public int SelectedShapeIndex;

// BaseEditEngine.cs:182 (master 183)
public static Collection<ShapeEngine> SEngines = [];

HandleAfterSave() runs DrawAllShapes() after the file has been written, which is why the save itself succeeds and only the UI dies:

// BaseEditEngine.cs:423 (master 436)
public virtual void HandleAfterSave ()
{
    actions.Edit.Undo.Activate ();
    //Redraw all of the editable shapes in case saving caused some extra/unexpected behavior.
    DrawAllShapes ();
}

DrawAllShapes() uses that shared field directly as the loop counter:

// BaseEditEngine.cs:1325 (master 1338)
public void DrawAllShapes ()
{
    int previousToolSI = SelectedShapeIndex;

    for (SelectedShapeIndex = 0; SelectedShapeIndex < SEngines.Count; ++SelectedShapeIndex) {
        DrawActiveShape (true, false, previousToolSI == SelectedShapeIndex, false, true);
    }
    ...

DrawActiveShape() switches the active tool when the shape at the current index belongs to a different tool:

// BaseEditEngine.cs:1012 (master 1025)
public void DrawActiveShape (bool calculateOrganizedPoints, bool finalize, bool drawHoverSelection, bool shiftKey, bool preventSwitchBack, bool ctrl_key = false)
{
    ShapeTool? oldTool = BaseEditEngine.ActivateCorrespondingTool (SelectedShapeIndex, calculateOrganizedPoints);

    if (oldTool != null) {
        if (tools.CurrentTool is ShapeTool tool)
            tool.EditEngine.DrawActiveShape (calculateOrganizedPoints, finalize, drawHoverSelection, shiftKey, preventSwitchBack);
        ...

That tool switch deactivates the engine that is currently running the loop, and HandleDeactivated() clears the loop counter:

// BaseEditEngine.cs:406 (master 419)
public virtual void HandleDeactivated (BaseTool? newTool)
{
    SelectedPointIndex = -1;
    SelectedShapeIndex = -1;   // <-- this is DrawAllShapes()'s loop counter
    ...

So on each iteration: SelectedShapeIndex is set to -1 inside the body, ++SelectedShapeIndex brings it back to 0, 0 < SEngines.Count still holds, and shape 0 is processed again forever. Note DrawAllShapes() passes preventSwitchBack: true, so the tool is not switched back, but the reset has already happened.

This explains every observation:

• One live shape: ActivateCorrespondingTool returns null (no switch needed), the field is never reset, the loop ends normally.
• Several live shapes from one tool: same, no switch, no reset, terminates.
• Shapes from two different tools: a switch happens, the counter resets, infinite loop.
• Unbounded memory and CPU: each iteration re-runs a full tool switch (rebuilding the tool's toolbar, including a Gtk.SpinButton) plus a full shape redraw through Cairo.

FinalizeAllShapes() at 1348 (master 1361) has the same shared-field-as-loop-counter pattern. It is not the loop that hangs here, because it calls correspondingEngine.DrawFinalized() directly instead of going through DrawActiveShape(), so no tool switch occurs. It looks fragile for the same reason and may be worth hardening too.

Evidence

Native backtraces sampled with gdb from the hung process (4.5 GB RSS, 95% CPU). The managed frames are unresolved because this is a Flatpak build without SOS, but the native frames show the two halves of the loop.

Repeated toolbar rebuild, which is the tool switching:

#0  pango_font_description_set_family ()   libpango-1.0.so.0
#1  gtk_css_style_get_pango_font ()        libgtk-4.so.1
#2  gtk_widget_update_pango_context ()
#3  gtk_widget_create_pango_context ()
#4  gtk_widget_get_pango_context ()
#5  gtk_widget_create_pango_layout ()
#6  gtk_text_ensure_layout ()
#7  gtk_text_get_scroll_limits ()
#8  gtk_text_adjust_scroll ()
#9  gtk_text_realize ()
...
#14 gtk_widget_realize ()
#15 gtk_widget_map ()
#16 gtk_widget_real_map ()
...
#22 gtk_widget_reposition_after ()
#23 0x00007fe63f2b9e6b in ??? ()            <-- managed frame

Repeated shape redraw, two consecutive samples of the same managed return address:

#0  sse2_fill.lto_priv ()                  libpixman-1.so.0
#1  pixman_fill ()
#2  fill_boxes ()                           libcairo.so.2
#3  clip_and_composite_boxes.part ()
#4  _cairo_spans_compositor_paint ()
#5  _cairo_compositor_paint ()
#6  _cairo_surface_paint ()
#7  _cairo_gstate_paint ()
#8  cairo_paint ()
#9  0x00007fe63ed60497 in ??? ()            <-- managed frame

#0  _cairo_tor_scan_converter_generate ()   libcairo.so.2
#1  composite_polygon ()
#2  _cairo_spans_compositor_stroke ()
#3  _cairo_compositor_stroke_impl ()
#4  _cairo_image_surface_stroke ()
#5  _cairo_surface_stroke ()
#6  _cairo_gstate_stroke ()
#7  _cairo_default_context_stroke ()
#8  cairo_stroke ()
#9  0x00007fe63ed60497 in ??? ()            <-- same managed frame

Memory growth measured once per second after Ctrl+S in the failing case:

t=1s   rss=301992 kB
t=5s   rss=355912 kB
t=10s  rss=420112 kB
t=14s  rss=469816 kB     (still climbing, killed at 4.5 GB in a real session)

Suggested fix

Use a local loop variable so that a mid-loop reset of the field cannot rewind iteration:

public void DrawAllShapes ()
{
    int previousToolSI = SelectedShapeIndex;

    for (int i = 0; i < SEngines.Count; ++i) {
        SelectedShapeIndex = i;
        DrawActiveShape (true, false, previousToolSI == i, false, true);
    }

    SelectedShapeIndex = previousToolSI;
    BaseEditEngine.ActivateCorrespondingTool (SelectedShapeIndex, false);
}

Because the tool switch retargets PintaCore.Tools.CurrentTool and the engine that owns the loop gets deactivated, it may be worth reasserting SelectedShapeIndex = i after the DrawActiveShape() call as well, or hoisting the iteration out of the per-engine instance state entirely. A guard against non-terminating iteration here would also prevent a hard hang if another reset path is found later.

Workaround for users

Before saving, click any non-shape tool (Move Selection, Rectangle Select, Paintbrush). That finalizes the live shapes through HandleDeactivated(), outside the save path, and the save then completes normally. Pressing Enter is not sufficient.