After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 313030 - Leaks reported by valgrind
Leaks reported by valgrind
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: general
trunk
Other Linux
: High normal
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
Depends on:
Blocks:
 
 
Reported: 2005-08-09 18:46 UTC by Kjartan Maraas
Modified: 2005-10-03 19:23 UTC
See Also:
GNOME target: ---
GNOME version: 2.11/2.12


Attachments
Untested patch to potentially remove memory leak (513 bytes, patch)
2005-08-16 18:16 UTC, Elijah Newren
none Details | Review
Patch (569 bytes, patch)
2005-08-16 18:34 UTC, Soren Sandmann Pedersen
none Details | Review
patch (2.03 KB, patch)
2005-08-28 12:12 UTC, Kjartan Maraas
none Details | Review
Modified version of Kjartan's patch that compiles ;-) (1.62 KB, patch)
2005-10-01 19:21 UTC, Elijah Newren
committed Details | Review

Description Kjartan Maraas 2005-08-09 18:46:07 UTC
Here are the traces:

==4399== 31 bytes in 1 blocks are definitely lost in loss record 2368 of 4021
==4399==    at 0x1B903882: malloc (vg_replace_malloc.c:149)
==4399==    by 0x1BF8BCC5: g_malloc (gmem.c:137)
==4399==    by 0x1BF9C539: g_strdup (gstrfuncs.c:91)
==4399==    by 0x1BF91695: parse_arg (goption.c:794)
==4399==    by 0x1BF91C31: parse_long_option (goption.c:1073)
==4399==    by 0x1BF926F9: g_option_context_parse (goption.c:1323)
==4399==    by 0x806704A: main (main.c:296)

==4399== 32 bytes in 2 blocks are definitely lost in loss record 2376 of 4021
==4399==    at 0x1B903882: malloc (vg_replace_malloc.c:149)
==4399==    by 0x1BF8BCC5: g_malloc (gmem.c:137)
==4399==    by 0x806F09B: meta_screen_calc_workspace_layout (screen.c:1982)
==4399==    by 0x80884F2: meta_window_show_menu (window.c:6200)
==4399==    by 0x805F638: meta_frames_button_press_event (frames.c:1487)
==4399==    by 0x1BA564E5: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:83)
==4399==    by 0x1BEF03B0: g_type_class_meta_marshal (gclosure.c:569)
==4399==    by 0x1BEF0999: g_closure_invoke (gclosure.c:492)
==4399==    by 0x1BEFFF1E: signal_emit_unlocked_R (gsignal.c:2523)
==4399==    by 0x1BF01033: g_signal_emit_valist (gsignal.c:2254)
==4399==    by 0x1BF015EA: g_signal_emit (gsignal.c:2288)
==4399==    by 0x1BB3CC71: gtk_widget_event_internal (gtkwidget.c:3735)
==4399==    by 0x1BA537BC: gtk_propagate_event (gtkmain.c:2157)
==4399==    by 0x1BA53CDF: gtk_main_do_event (gtkmain.c:1395)
==4399==    by 0x1BC4A9BC: gdk_event_dispatch (gdkevents-x11.c:2291)
==4399==    by 0x1BF85829: g_main_context_dispatch (gmain.c:1934)
==4399==    by 0x1BF885F9: g_main_context_iterate (gmain.c:2565)

==4399== 48 bytes in 1 blocks are definitely lost in loss record 2966 of 4021
==4399==    at 0x1B904B64: calloc (vg_replace_malloc.c:279)
==4399==    by 0x1BF8BD27: g_malloc0 (gmem.c:154)
==4399==    by 0x805DE99: get_cache (frames.c:303)
==4399==    by 0x805DECD: invalidate_cache (frames.c:314)
==4399==    by 0x805F3D0: redraw_control (frames.c:1232)
==4399==    by 0x805FD04: meta_frames_button_release_event (frames.c:1596)
==4399==    by 0x1BA564E5: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:83)
==4399==    by 0x1BEF03B0: g_type_class_meta_marshal (gclosure.c:569)
==4399==    by 0x1BEF0999: g_closure_invoke (gclosure.c:492)
==4399==    by 0x1BEFFF1E: signal_emit_unlocked_R (gsignal.c:2523)
==4399==    by 0x1BF01033: g_signal_emit_valist (gsignal.c:2254)
==4399==    by 0x1BF015EA: g_signal_emit (gsignal.c:2288)
==4399==    by 0x1BB3CC71: gtk_widget_event_internal (gtkwidget.c:3735)
==4399==    by 0x1BA537BC: gtk_propagate_event (gtkmain.c:2157)
==4399==    by 0x1BA53CDF: gtk_main_do_event (gtkmain.c:1395)
==4399==    by 0x1BC4A9BC: gdk_event_dispatch (gdkevents-x11.c:2291)

There are lots of variations of this last one, but I didn't want to post them
all here. Full log from valgrind is here:

http://www.gnome.org/~kmaraas/valgrind-logs/2005-08-09/metacity.log
Comment 1 Elijah Newren 2005-08-16 18:15:43 UTC
The first leak looks like it's from the stuff in bug 309178.

I'll attach a patch that I believe will fix the second, though I haven't tested
it or anything.

The third leak looks like it's from Soeren's patch in bug 141813, so I'll cc him.
Comment 2 Elijah Newren 2005-08-16 18:16:31 UTC
Created attachment 50806 [details] [review]
Untested patch to potentially remove memory leak
Comment 3 Soren Sandmann Pedersen 2005-08-16 18:34:03 UTC
Created attachment 50808 [details] [review]
Patch

This patch should fix the third leak. It's untested, beyond me starting
metacity and running it as I type this.
Comment 4 Kjartan Maraas 2005-08-18 19:16:35 UTC
I'm going to give these a run. I noticed this btw in my current metacity logs:

==2614== Invalid read of size 4
==2614==    at 0x80690E1: find_first_fit (place.c:599)
==2614==    by 0x8069793: meta_window_place (place.c:1027)
==2614==    by 0x805419D: meta_window_constrain (constraints.c:1293)
==2614==    by 0x8088EC3: meta_window_move_resize_internal (window.c:2577)
==2614==    by 0x8089EA4: meta_window_move_resize_now (window.c:2977)
==2614==    by 0x808C0EF: implement_showing (window.c:1766)
==2614==    by 0x808C351: idle_calc_showing (window.c:1438)
==2614==    by 0x1BF3889F: g_idle_dispatch (gmain.c:3813)
==2614==    by 0x1BF366FD: g_main_context_dispatch (gmain.c:1934)
==2614==    by 0x1BF394CD: g_main_context_iterate (gmain.c:2565)
==2614==    by 0x1BF399CE: g_main_loop_run (gmain.c:2769)
==2614==    by 0x8067224: main (main.c:483)
==2614==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==2614==
==2614== Process terminating with default action of signal 11 (SIGSEGV)
==2614==  Access not within mapped region at address 0x0
==2614==    at 0x80690E1: find_first_fit (place.c:599)
==2614==    by 0x8069793: meta_window_place (place.c:1027)
==2614==    by 0x805419D: meta_window_constrain (constraints.c:1293)
==2614==    by 0x8088EC3: meta_window_move_resize_internal (window.c:2577)
==2614==    by 0x8089EA4: meta_window_move_resize_now (window.c:2977)
==2614==    by 0x808C0EF: implement_showing (window.c:1766)
==2614==    by 0x808C351: idle_calc_showing (window.c:1438)
==2614==    by 0x1BF3889F: g_idle_dispatch (gmain.c:3813)
==2614==    by 0x1BF366FD: g_main_context_dispatch (gmain.c:1934)
==2614==    by 0x1BF394CD: g_main_context_iterate (gmain.c:2565)
==2614==    by 0x1BF399CE: g_main_loop_run (gmain.c:2769)
==2614==    by 0x8067224: main (main.c:483)
Comment 5 Elijah Newren 2005-08-18 19:24:04 UTC
Uh...haven't seen that one before.  From looking at line 599 here (are you using
CVS HEAD?) it looks like bug 307884 might be related but it seems odd that this
should happen.  There's lots of pointers on that line--any chance you could
check more closely which of them is actually NULL?
Comment 6 Kjartan Maraas 2005-08-28 12:12:35 UTC
Created attachment 51457 [details] [review]
patch
Comment 7 Elijah Newren 2005-09-01 16:20:37 UTC
Patch in bug 315000 should fix the invalid read in comment 4...

Kjartan: it looks like you incorporated Soeren's patch and my patch into yours,
right?  Shouldn't the other two patches be marked as obsolete?  Also, why the
change to META_WIREFRAME_XOR_LINE_WIDTH?
Comment 8 Kjartan Maraas 2005-09-02 00:19:34 UTC
Yeah, I forgot I had those in my tree. The last one is to make the wireframe
when using reduced resources mode a lot more appealing and less chubby. Try it
on for size. I think it looks much better.
Comment 9 Elijah Newren 2005-10-01 19:21:25 UTC
Created attachment 52920 [details] [review]
Modified version of Kjartan's patch that compiles  ;-)

This is basically just Kjartan's patch with the needed semicolon for
compiling--as such, it incorporates a leak fix from him, the one from Soeren
above, and the one from me above.  I've tested in valgrind and it looks good.
Comment 10 Elijah Newren 2005-10-01 19:23:32 UTC
Note that while testing in valgrind, I saw a new leak not mentioned
above--although this one kind of baffles me because I believe we always do the
necessary gtk_widget_destroy() call...

==17071== 188 bytes in 1 blocks are definitely lost in loss record 3570 of 4001
==17071==    at 0x1B905301: calloc (vg_replace_malloc.c:176)
==17071==    by 0x1BE5DF13: IA__g_malloc0 (gmem.c:154)
==17071==    by 0x1BDE6A63: IA__g_type_create_instance (gtype.c:1576)
==17071==    by 0x1BDD10FB: g_object_constructor (gobject.c:1011)
==17071==    by 0x1BDD05C1: IA__g_object_newv (gobject.c:908)
==17071==    by 0x1BDD0FF7: IA__g_object_new_valist (gobject.c:951)
==17071==    by 0x1BDD10D1: IA__g_object_new (gobject.c:789)
==17071==    by 0x1BA29CA6: IA__gtk_menu_new (gtkmenu.c:1159)
==17071==    by 0x806E57A: meta_window_menu_new (menu.c:277)
==17071==    by 0x808EE83: meta_window_show_menu (window.c:6276)
==17071==    by 0x806CDF0: handle_activate_menu (keybindings.c:2973)
==17071==    by 0x806BDD6: meta_display_process_key_event (keybindings.c:1604)
==17071==    by 0x8060B74: event_callback (display.c:1595)
==17071==    by 0x808AE5B: filter_func (ui.c:85)
==17071==    by 0x1BBFD33A: gdk_event_apply_filters (gdkevents-x11.c:328)
==17071==    by 0x1BBFDC87: gdk_event_translate (gdkevents-x11.c:874)
==17071==    by 0x1BBFF6AD: _gdk_events_queue (gdkevents-x11.c:2225)
==17071==    by 0x1BBFF843: gdk_event_dispatch (gdkevents-x11.c:2285)
==17071==    by 0x1BE57F18: IA__g_main_context_dispatch (gmain.c:1934)
==17071==    by 0x1BE597C4: g_main_context_iterate (gmain.c:2565)
==17071==    by 0x1BE59A1B: IA__g_main_loop_run (gmain.c:2769)
==17071==    by 0x806DE9D: main (main.c:483)
Comment 11 Elijah Newren 2005-10-03 19:23:29 UTC
Alright, committed the last patch.  If there are any other issues (e.g. if my
comment #10 turns out it wasn't just some weird fluke) then let's open another bug.

2005-10-03  Elijah Newren  <newren@gmail.com>

	A combination of a couple memory leaks fixes, from Kjartan,
	Soeren, and I.  Fixes #313030.

	* src/bell.c (meta_bell_flash_screen): call XFreeGC()

	* src/frames.c (invalidate_cache): free pixels

	* src/window.c (meta_window_show_menu): call
	meta_screen_free_workspace_layout()