crash on cut n paste, or alt+scroll..

Bug #1171109 reported by insaner on 2013-04-21
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
High
insaner

Bug Description

r12232, and r11964
on linux (fedora 14, if it matters)

(using the attached file) cut and paste once, notice the gradients disappear, cut again and paste again and BOOM! Segmentation fault..

however, you can /copy/ paste to your heart's content and no crash, no problems. you can copy once, and then cut and paste (either) and if you have at least one of them present, it will not cause any of these problems either

you can also cut the single one, paste once, undo, undo.. and you will get your original back, no problems, no missing gradients.

ok, now, go to the original state.. cut once, paste once, now alt+scroll on the group.. BOOM! segfault.

these problems are not present on 0.48.4 r9939

some more testing: if i open the swatches dialog while im cutting pasting, i notice that a paste creates a new linear gradient swatch called "gradient-2" for some reason, a duplicate of "gradient".
renaming that gradient to "gradientx" before the cut and paste (after undoing back to the original state of the file) now creates yet another gradient (for a total of 2 new gradients created on paste) using a number as its name..

this process leads to the creation of a bunch more swatches on 0.48.4 (not the same names as in the later versions)

hope all this helps someone find the problem and fix it.

insaner (insaner) wrote :
su_v (suv-lp) wrote :

> (using the attached file) cut and paste once, notice the gradients disappear,
> cut again and paste again and BOOM! Segmentation fault..

Reproduced with Inkscape 0.48+devel r12292 on OS X 10.7.5 (GTK+/Quartz 2.24.17, glib 2.34.3), backtrace attached.

tags: added: clipboard crash gradient regression
Changed in inkscape:
status: New → Confirmed
importance: Undecided → High
jazzynico (jazzynico) wrote :

Cut and paste crash also reproduced on Debian testing, Inkscape trunk revision 12293.

Changed in inkscape:
status: Confirmed → Triaged
insaner (insaner) wrote :

possibly related to bug #643150 .. can someone confirm by testing some revision before r11677? (the closer the better obviously) and then trying the patch in comment #8?

if someone can confirm this (i wont have time to do that today), it will get me that much more ahead of the game and possibly fix it (im guessing john-smithi wont have time)

su_v (suv-lp) wrote :

> possibly related to bug #643150 .. can someone confirm by testing some revision
> before r11677?

Crash (Cut & Paste) not reproduced with r11674,
reproduced with r11679 on OS X 10.7.5 (tested with X11-based archived builds)

John Smith (john-smithi) wrote :

Please check if the attached patch works.

Changed in inkscape:
assignee: nobody → John Smith (john-smithi)
status: Triaged → In Progress
insaner (insaner) wrote :

hi john-smithi,

i tested the patch and it almost works.. i had been trying something similar, ie simply commenting out the line that reads

"if (gr->isSolid() || gr->getVector()->isSolid()) {"
(and the corresponding closing brace)

 in document.cpp, and while that prevented the crashes caused by pasting, when i do the alt+scroll part after pasting it would still crash.

with your patch, the pasting doesnt cause a crash, but it also doesnt properly paste the object (specifically, the objects that use the swatch named "light" do not show up, though in outline mode you can see that they are there), and doing the alt+scroll where those swatched objects/paths should be, triggers the crash.

using the trick above (commenting out the line and its corresponding brace) makes the alt+scroll problem worse, but it prevents the duplication of the swatches..

insaner (insaner) wrote :

i've almost resolved this issue (i hope) so i am assigning this to me.. patch to come soon

Changed in inkscape:
assignee: John Smith (john-smithi) → insaner (insaner)
su_v (suv-lp) on 2013-10-17
Changed in inkscape:
milestone: none → 0.49
su_v (suv-lp) wrote :

@insaner - are you still working on this issue?

insaner (insaner) wrote :

hi suv,
sorry, ive been busy with stuff.. after i posted that last message i realized my solution didnt work properly, and i got busied up soon after. not sure when i will have some time to devote to this. i saw the updates to the other bug as well..

su_v (suv-lp) wrote :

@insaner - in this case maybe you might consider unassigning yourself from the report for now? (There might be a slight chance that someone else volunteers to work on a fix (for 0.49) - which is less likely to happen if the bug is already assigned).

insaner (insaner) wrote :

let me look into it this weekend then, if by tuesday i dont see the possibility of progress by end of month, i'll unassign myself

Bryce Harrington (bryce) wrote :

Hi insaner, we're in the middle of a bug hunt for 0.91, and this bug is on the list of bugs needing fixed. Could you update us on your progress? If you're still interested in developing a fix, now would be the ideal time to get it submitted.

insaner (insaner) wrote :

hi,
sorry for the great delay, I haven't really had much time to devote to this, and when I last had time I had to spend most of it just catching up with some of the changes to the code that affected this part in particular.

 let me share with you guys what I have been able to narrow down the problem to so far, perhaps someone can whip something up with this information.

the problem is caused by the fact that when you cut and then paste an object, you are changing a bunch of its references, and not all of them are properly updated, so that when you re-paste the object it isn't pointing to existing definitions, and when you alt + scroll, it tries to access these, which of course causes a segfault.

eg, you have a <linearGradient id="dark" ..> gradient, and your object makes use of this gradient via a
<linearGradient xlink:href="#dark" id="linearGradient7822" ...>
when you cut and paste the object, the xlink:href property is then changed to
<linearGradient xlink:href="#dark-6" id="linearGradient7837" ...>

the file in question is src/document.cpp, in method void SPDocument::importDefs(SPDocument *source) which calls
   prevent_id_clashes(source, this) from src/id-clash.cpp
which then calls
 change_clashing_ids(imported_doc, current_doc, imported_root, refmap,
                        &id_changes)
which is what changes these references.

I have tried a couple of things, but my lack of familiarity with how the code works has caused it to be a bit trickier for me than it would be for one of you more seasoned guys.

the fix is to create a check of some sort inside the

    if (id && current_doc->getObjectById(id)) {...}

of the change_clashing_ids() method, whereby it can check if a pasted object's definitions with clashing ids are identical to the ones that already exist in the document and have those references not be modified. This is where I haven't been able to advance.

let me know if this sounds simple enough for one of you guys to solve this week.. otherwise I will try a few things later this week to see what I can do to move us toward a solution.

insaner (insaner) wrote :

yeah, a quick test, commenting out the call to
 change_clashing_ids()
in
 prevent_id_clashes()
completely "solves" the crashing, the missing gradients etc.. but of course, that means that any objects that are pasted with truly conflicting ids will be screwed up.
let me see if I can figure something out in the next few minutes

insaner (insaner) wrote :

ok.. it seems typing out the description of the problem made me find a quick solution.. I'm working on creating a clean patch for you to test out

by the way, I hit some delays trying to get this done because of Bug #1251951 , if someone could please take a look at that, that would be great.

Alvin Penner (apenner) wrote :

@insaner - compile problem not reproduced on Windows, could you update that report with complete information on
- Inkscape rev number
- devlibs rev number
- gcc compiler version

insaner (insaner) wrote :

hi Alvin, I included all that info in the original bug report, the only thing that has changed is that rev 12882 still has the problem.

insaner (insaner) wrote :

the attached patch prevents the crash, and resolves the duplicate identical gradients problem as well. ie, if you paste an object with gradients that have the same ids as gradients on the document, it will check them to see if they are equivalent, and if so, will not change the gradient id's in the pasted object.

issues remaining: this only does it for gradients, not filters, so each cut and paste creates a new identical filter. Maybe a new bug could be filed for that.

let me know if this solves it for you guys, and I will check in the changes.

as an aside, did someone remove my changes for Bug #1092313 ? I noticed the filter icon no longer appears.

insaner (insaner) wrote :

hi everyone, I plan on checking in the changes by Friday, unless someone finds a problem with the code as it stands.. please let me know if you do. thanks

insaner (insaner) wrote :

new patch with Johan's suggestions

jazzynico (jazzynico) wrote :

Patch from comment #21 tested on Windows XP, Inkscape trunk revision 12976.

The file from comment #1 no longer crashes, and identical gradient defs are reused, not duplicated.
But:

A - A new gradient is created when a copy of the original object is deleted:
1. Open the XML editor, and expand the defs element.
2. Copy and paste the original object.
3. Remove the copy.
4. A linearGradient (from the copy) is still in the defs.

The new gradient is an exact copy (except the ids) of the original gradient with id="gradient". It also has an obs:paint="gradient" attribute, is listed in the swatch list, and is not removed by File>Clean up Document.

B - Inkscape crashes when redoing (not reproduced with r12904):
1. Copy and paste the original object.
2. Undo.
3. Redo -> Crash.

No useful gdb trace, but an error dialog shows the following message:
----
glibmm-ERROR **:
unhandled exception (type std::exception) in signal handler:
what: basic_string::_S_contruct NULL not valid
aborting...
----

jazzynico (jazzynico) wrote :

> I noticed the filter icon no longer appears.

Everything is normal here. I still see your icon.

insaner (insaner) wrote :

hi, this patch should solve that. As before, filters still get duplicated, so maybe we need to file a feature request for that.

the following defs do not get removed when you delete the copied object: svg:clipPath and svg:filter

but the crashes and duplicate gradients are gone.

insaner (insaner) wrote :

committed as revision 13047

Changed in inkscape:
status: In Progress → Fix Committed
su_v (suv-lp) wrote :

Follow-up report (regression):
- Bug #1283193 “trunk: Text gradient fill/stroke break on copy (rev >= 13047)”
  <https://bugs.launchpad.net/inkscape/+bug/1283193>

Bryce Harrington (bryce) on 2015-02-21
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers