Crash in Flood Tool

Steps to reproduce:

  • open Inkscape with attached file
  • set zoom level to 1.0
  • select Flood tool
  • set Close gaps to Large
  • choose a color for fill
  • click in circle and verify flood tool works in about five seconds
  • undo fill
  • redo fill and quickly switch to another tool

What happened?

Crash!

What should have happened?

Duh!

Analysis

The cursor is being changed to "Wait" which requires calling the Gtk main loop. This allows the user to change tools and thus the pointer to 'this'. There is an attempt to verify that 'this' hasn't changed by checking that the pointer is still a pointer to an EventContext (a.k.a. ToolBase), but this doesn't prevent crashes:

    case GDK_BUTTON_RELEASE:
        if (event->button.button == 1 && !this->space_panning) {
            Inkscape::Rubberband *r = Inkscape::Rubberband::get(desktop);

            if (r->is_started()) {
                // set "busy" cursor
                desktop->setWaitingCursor();

                if (SP_IS_EVENT_CONTEXT(this)) { 
                    // Since setWaitingCursor runs main loop iterations, we may have already left this tool!
                    // So check if the tool is valid before doing anything
                    dragging = false;

                    bool is_point_fill = this->within_tolerance;
                    bool is_touch_fill = event->button.state & GDK_MOD1_MASK;
                    
                    sp_flood_do_flood_fill(this, event, event->button.state & GDK_SHIFT_MASK, is_point_fill, is_touch_fill);
                    
                    desktop->clearWaitingCursor();
                    // restore cursor when done; note that it may already be different if e.g. user 
                    // switched to another tool during interruptible tracing or drawing, in which case do nothing

                    ret = TRUE;
                }

                r->stop();

                if (SP_IS_EVENT_CONTEXT(this)) {
                    this->defaultMessageContext()->clear();
                }
            }
        }
        break;

Note: I plan on removing SP_IS_EVENT_CONTEXT so it may already be gone.

Inkscape Version and Operating System:

  • Inkscape Version: master, etc.
  • Operating System: Linux
  • Operating System version: Fedora 32

flood_crash.svg