trunk: crash on reselect after undo of stroke-to-path if 'Object Properties' dialog open

[su_v]

Inkscape crashes on reselecting an object after having undone a conversion of stroke to path, if the object properties dialog is open.

Steps to reproduce:
1) launch trunk with default (new) prefs
2) open 'Object Properties' dialog
2) draw a rectangle
3) convert stroke to path (Ctrl+Alt+C)
4) undo (Ctrl+Z)
5) select the restored rectangle

-> crash

Based on tests with archived builds on OS X 10.7.5:
- not reproduced with rev <= 10804
- reproduced with rev >= 10806
the regression seems to be related to the changes (C++ification) of the dialog in rev 10805/10806:
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/changes/10806>

[Alvin Penner (apenner)]

crash confirmed on Windows XP, Inkscape rev 13123.
I was not able to get a backtrace, just got a message saying "no stack"

[Alvin Penner (apenner)]

slightly modified crash sequence:

- object properties not open
- draw rectangle
- stroke to path
- undo
- select rectangle
- open Object->Object Properties
- get console messages:

...................................

(inkscape.exe:3616): Gtk-CRITICAL **: gtk_widget_realize: assertion `GTK_WIDGET_ANCHORED (widget) || GTK_IS_INVISIBLE (widget)' failed

(inkscape.exe:3616): Gdk-CRITICAL **: gdk_window_invalidate_rect_full: assertion `GDK_IS_WINDOW (window)' failed

(inkscape.exe:3616): Gtk-CRITICAL **: gtk_widget_realize: assertion `GTK_WIDGET_ANCHORED (widget) || GTK_IS_INVISIBLE (widget)' failed

(inkscape.exe:3616): Gtk-CRITICAL **: gtk_widget_realize: assertion `GTK_WIDGET_ANCHORED (widget) || GTK_IS_INVISIBLE (widget)' failed

(inkscape.exe:3616): Gtk-CRITICAL **: gtk_widget_realize: assertion `GTK_WIDGET_ANCHORED (widget) || GTK_IS_INVISIBLE (widget)' failed

(inkscape.exe:3616): Gtk-CRITICAL **: gtk_widget_realize: assertion `GTK_WIDGET_ANCHORED (widget) || GTK_IS_INVISIBLE (widget)' failed

(inkscape.exe:3616): Gtk-CRITICAL **: gtk_widget_realize: assertion `GTK_WIDGET_ANCHORED (widget) || GTK_IS_INVISIBLE (widget)' failed

(inkscape.exe:3616): Gtk-CRITICAL **: gtk_widget_realize: assertion `GTK_WIDGET_ANCHORED (widget) || GTK_IS_INVISIBLE (widget)' failed

Emergency save activated!

Emergency save document locations:
  C:\Documents and Settings\Alvin\New document 1.2014_03_10_18_52_25.0.svg
Emergency save completed. Inkscape will close now.
If you can reproduce this crash, please file a bug at www.inkscape.org
with a detailed description of the steps leading to the crash, so we can fix it.

**
Gtk:ERROR:gtkwidget.c:8828:gtk_widget_real_map: assertion failed: (gtk_widget_get_realized (widget))

[Liam P. White (liampwhite)]

Recreating the crash, and using GDB to get a backtrace, we see that in this instance the program crashes with SIGABRT. The portion of the code we are interested in is here:

#0 0x0000000000982ef4 in Inkscape::UI::Dialog::ObjectProperties::widget_setup (this=0x5b0aa40) at ui/dialog/object-properties.cpp:396
         EntryID.set_text (obj->getId());

Looking in the XML editor, directly before the crash, the object has NO path ID, which explains the crash (the XML node is trying to send back a Glib::ustring created with a NULL C string, which fails and usually sets SIGTRAP, for those developers out there)

The offending XML is this: <path style="fill:none;stroke:#000000;stroke-width:1px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1" d="[random point data]" inkscape:connector-curvature="0" />

Notice something missing there? It's the Object's path ID. Keep in mind that everything is fine and dandy if the Object Properties window is not open (I added in a little try/catchall code to keep Inkscape going) but when it is the XML editor displays that the object now has no path ID.

Using a slightly different method suggested by Alvin Penner we see:

(inkscape:27779): Pango-WARNING **: Invalid UTF-8 string passed to pango_layout_set_text()

terminate called after throwing an instance of 'std::logic_error'
  what(): basic_string::_S_construct null not valid

Program received signal SIGABRT, Aborted.
...
#0 0x0000000000982ef4 in Inkscape::UI::Dialog::ObjectProperties::widget_setup (this=0x5a742c0) at ui/dialog/object-properties.cpp:396
         EntryID.set_text (obj->getId());

Running the debugging code again allows us to open the XML Editor after the crash, where we see that the object seems to have a path ID. Wrong, the XML Editor is lying. Using the node tool to tweak the path then causes the path ID to disappear from the XML editor, which is actually what happened.

Looking into object-properties.cpp, nothing appears to be out of place (except for proper exception handlers for this type of error). Looking into splivarot.cpp (the file that calls path outliner) once again nothing appears to be wrong. Checking document-undo.cpp, nothing appears to be wrong. Not sure why this is happening, but it's rather serious.

[Liam P. White (liampwhite)]

Diving deeper into the obstructed operations. Perform these steps:

1. Open Inkscape
2. Draw a new path with the Bezier tool
3. Convert stroke to path
4. Undo
5. Open the XML Editor

Display looks as expected, right? There's a surprise waiting for you. Without closing the XML Editor, select the Node tool and move one of the points on the path. In the XML Editor: bam, no path ID. No console output.
In one of my tests the XML editor glitched out and created five copies of each true XML node in the display window. The console output was several of these:

(inkscape:28124): Gtk-CRITICAL **: gtk_tree_store_move: assertion `VALID_ITER (iter, tree_store)' failed

[su_v (suv-lp)]

On 2014-03-11 02:28 +0100, Liam P. White wrote:
> In one of my tests the XML editor glitched out and created five
> copies of each true XML node in the display window.

Tracked in bug #1251056 “def showing twice in xml editor, no duplicate when saving” (report lacks steps to reproduce though).

[Liam P. White (liampwhite)]

Attached patch file seems to fix this bug. Tell livarot to completely destroy the old object before creating a new one and prevent causing an ID clash.

[su_v (suv-lp)]

Patch 'fix-undo.patch' tested successfully with r13135 on OS X 10.7.5 - haven't had the time to look out for unintended side-effects though (the reported crash and variations of it no longer reproduce with the patch applied).

--
[ replaces earlier comment #8 to fix misleading typo ]

[jazzynico (jazzynico)]

Patch also tested successfully with r13135 on Crunchbang Waldorf. No regression found so far.

[Liam P. White (liampwhite)]

Patch seems to change the object's ID on stroke to path. Question: why are we destroying the old object anyway? Seems like we could just modify the repr of the old object instead...

[Liam P. White (liampwhite)]

Note that this is also reproducible in 0.48.4 (and probably 0.48.5 as well)

Perform the above steps, and try to set the object's ID -> crash

[su_v (suv-lp)]

On 2014-04-15 18:40 +0100, Liam P. White wrote:
> Note that this is also reproducible in 0.48.4 (and probably 0.48.5
> as well)
>
> Perform the above steps, and try to set the object's ID -> crash

Confirmed with 0.48.4 r9939 and with r10025 from lp:inkscape/0.48.x on OS X 10.7.5 as described.

Attached patch backports the fix from comment #7 for trunk to the stable branch. The crash as described is no longer reproducible with the backported patch applied. Note: not tested at runtime for possible regressions or unintended side-effects.

[Liam P. White (liampwhite)]

Fix committed in r13323 of lp:inkscape.

Replaces earlier comment #14 to clarify which branch it was merged into.

[Krzysztof Kosinski (tweenk)]

Patch applied to stable in r10036.