Memory leak when opening context menu

[Kris]

When opening the context menu some memory is never released.

Steps to reproduce:
1. Draw a rectangle
2. Select retangle
3. Right click to open the context menu
4. Close context menu and go back to step 3

The memory consumption rises when opening the context menu. Issue is present in current trunk revision (r10815), trunk revision 10686 and Inkscape 0.48 (r9654). Observed on windows vista 64 bit.

[jazzynico (jazzynico)]

Confirmed on CrunchBang 10 R20111125, Inkscape revision 10923.
Each right click eats about 100kio of RAM.
No leak when opening the context menu on the canvas.

[Kris (kris-degussem)]

Will try c++ifying the context menu (deriving from Gtk::Menu) and solve the issue simultaneously ...

[Kris (kris-degussem)]

In attachment is a first iteration of my work on the context menu.
The current purpose is only to c++ify the code (later work will be to study the callbacks and menu release). No changes from user points of view are expected, although it is impossible to go for bug for bug compatibility.
So what's in:
- c++ified context-menu.cpp (and included in interface.cpp)
- removed dozens of pointer conversions (of which some were at least wrong)
- removed a memory leak (though not the one which I was hunting for)

[Kris (kris-degussem)]

Synchronisation with current trunk (r11148) after modification in revision 11146.

[Kris (kris-degussem)]

I added the patch so a select group of people could test it, but as there seem changes every day to the source files involved, I've committed the current status in trunk revision 11160.
Modifications, see comment 3 + some additional null pointer checks to prevent crashes.

[su_v (suv-lp)]

> added the patch so a select group of people could test it

Compiling r11160 with Apple's GCC 4.2 on Mac OS X 10.5.8 and Apple's llvm-gcc-4.2 on OS X 10.7.2, I get lots of these warnings:

../../src/interface.h:64: warning: ‘GtkWidget* sp_ui_menu_append_item_from_verb(GtkMenu*, Inkscape::Verb*, Inkscape::UI::View::View*, bool, GSList*)’ declared ‘static’ but never defined

> No changes from user points of view are expected

1) I miss 'Fill and Stroke…' from the context menu for regular objects (it's still present e.g. for text objects) - any reason to drop it?

2) What's the reasoning behind changing the layout and adding the dialogs at the bottom (e.g. for text objects), unlike before where all dialogs had been grouped in the same section as the 'Objects Properties…' entry (which is still present as single entry)?

[Kris (kris-degussem)]

> ../../src/interface.h:64: warning: ‘GtkWidget* sp_ui_menu_append_item_from_verb(GtkMenu*, Inkscape::Verb*, Inkscape::UI::View::View*, bool, GSList*)’ declared ‘static’ but never defined
Should be fixed in revision 11161.

[Kris (kris-degussem)]

>> No changes from user points of view are expected
>1) I miss 'Fill and Stroke…' from the context menu for regular objects (it's still present e.g. for text objects) - any reason to drop it?
Should not happen, though this entry was only defined for shapes and text items. Maybe it is due to some wrong pointer conversion ... Anyhow, quite probably there was something wrong with the object type checking. Could you check whether the correct entries appear now in revision 11162?

>2) What's the reasoning behind changing the layout and adding the dialogs at the bottom (e.g. for text objects), unlike before where all dialogs had been grouped in the same section as the 'Objects Properties…' entry (which is still present as single entry)?
Indeed the order is different. The order in which the entries are created is the same as before. I did not yet spot code that reorders the menu entries.
It would be nice if someone can point out why the order of creation and appearing in the menu was different, and where the order was changed in the old situation.

[jazzynico (jazzynico)]

@Kris - Inkscape doesn't built after 11160 on Ubuntu 11.04 and Debian Squeeze (stable), and fails with the following error:

  CXX helper/png-write.o
In file included from /usr/include/libpng12/png.h:518:0,
                 from helper/png-write.cpp:23:
/usr/include/libpng12/pngconf.h:371:12: error: ‘__pngconf’ does not name a type
/usr/include/libpng12/pngconf.h:372:12: error: ‘__dont__’ does not name a type
make[1]: *** [helper/png-write.o] Erreur 1

Any idea?

[Alex Valavanis (valavanisalex)]

Build failure is hopefully fixed in r11163. Please test!

This seems to be some weirdness, in which _SETJMP_H is being defined somewhere it shouldn't be. I just included png.h at the top of the file, and the error disappeared.

[jazzynico (jazzynico)]

png-write error fixed. Thanks Alex!

[Kris (kris-degussem)]

@Alex: Thanks for the fix of this weird issue.
@~suv in reply to comment 6: issue should be "fixed" now in revision 11167. The order of dialogs is set explicitely for dialog items (I do not now whether it is the correct way to do though).

[Kris (kris-degussem)]

With the commit in trunk revision 11290, the memory management of the context menu should be all right.
I do not know whether the situation is improved. Please comment.
However, I noticed something else:
1. create a rectangle
2. hover over the rectangle
3. wait for a few seconds
4. move the mouse pointer outside of the rectangle but on an other empty part of the canvas
5. wait for a few seconds
6. go back to 2

When moving in and also when moving out of the rectangle, the memory seems to increase. Other people who observe this?

[Kris (kris-degussem)]

Partially reverted my last commit because callbacks from the context menu were not working.
The context menu destructor get's never called now (though it is still possible that the biggest problem/leak is not in the context menu).