LivePathEffect memory leak

Bug #1293827 reported by Kris on 2014-03-17
This bug affects 1 person
Affects Status Importance Assigned to Milestone

Bug Description

All classes derived from LivePathEffect::Parameter have a function param_getSVGValue. Generally, this param_getSVGValue function contains a g_strdup call. There is only one location where param_getSVGValue is called, namely in src/live_effects/parameter/parameter.h:52

However, I do not see a g_free call. So possibly this is quite a significant memory leak.
If leak is confirmed (please double check!) this patch should be able to solve it on the short term (on the longer term C++ string classes should be used).

Concerns trunk r13157

Kris (kris-degussem) wrote :
su_v (suv-lp) on 2014-03-18
tags: added: code-design performance
jazzynico (jazzynico) on 2014-03-24
Changed in inkscape:
assignee: nobody → Kris (kris-degussem)
status: New → In Progress
jazzynico (jazzynico) on 2014-07-02
Changed in inkscape:
milestone: none → 0.91
Kris (kris-degussem) wrote :

In case of no comments, I propose to check in the patch ...

jazzynico (jazzynico) wrote :

Leak confirmed with mtrace (6 bytes not freed).
And mtrace also confirms the patch fixes the leak.

Thanks Kris, and sorry for the delay -I had to learn mtrace first ;)

Kris (kris-degussem) wrote :

Np. Finally starting to recreate my build environment after serious PC problems.

Committed as rev 13520.

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

Follow-up report (critical regression, crash without emergency save):
- Bug #1367520 “[trunk] crash on resize document, copy to clipboard”

Kris (kris-degussem) wrote :

Hmmm, this probably indicates some design problem. I can not look into it for now. Best "solution" for now is imho to revert 13520, and fix the real issue later on when 0.91 is out. (the little memory leak won't hurt that many people ...)

Kris (kris-degussem) on 2014-09-10
Changed in inkscape:
status: Fix Committed → Triaged
assignee: Kris (kris-degussem) → nobody
milestone: 0.91 → none
Jabiertxof (jabiertxof) wrote :

Seems patch applied in 0.92, without related crashed. Maybe we can move to fix released.

Jabiertxof (jabiertxof) wrote :

0.92/0.91+devel r.14968

Kris (kris-degussem) wrote :

No the patch was reverted. So as far as I know, the memory leak and solution should be checked again (did not look at the code now) ... In my opinion, changing something now is risky because we have the new release hopelully landing soon ...

Jabiertxof (jabiertxof) wrote :

Kris, as far I could see your patch is applied in current trunk, and the derived bugs on apply fixed.

Kris (kris-degussem) wrote :

You're right, I have to retract my previous comment. ;-)
I was checking code: I thought my patch was reverted, but this is not the case.

Kris (kris-degussem) on 2016-06-10
Changed in inkscape:
status: Triaged → Fix Committed
assignee: nobody → Kris (kris-degussem)
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers