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 596808 - Fix several leaks reported by valgrind
Fix several leaks reported by valgrind
Status: RESOLVED FIXED
Product: devhelp
Classification: Applications
Component: General
unspecified
Other All
: Normal normal
: ---
Assigned To: devhelp-maint
devhelp-maint
Depends on:
Blocks:
 
 
Reported: 2009-09-30 04:06 UTC by Jonathon Jongsma
Modified: 2009-09-30 15:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix leak in view_setup_fonts() (2.11 KB, patch)
2009-09-30 04:06 UTC, Jonathon Jongsma
none Details | Review
Fix leak in search_combo_row_separator_func() (1.96 KB, patch)
2009-09-30 04:06 UTC, Jonathon Jongsma
none Details | Review
Fix a couple large leaks in DhBase (2.42 KB, patch)
2009-09-30 04:06 UTC, Jonathon Jongsma
none Details | Review
Fix small leak in search_complete_idle() (1.46 KB, patch)
2009-09-30 04:06 UTC, Jonathon Jongsma
none Details | Review
Fix leak of parser->base (2.31 KB, patch)
2009-09-30 04:06 UTC, Jonathon Jongsma
none Details | Review
Factor out the DhParser destructor (1.44 KB, patch)
2009-09-30 04:06 UTC, Jonathon Jongsma
none Details | Review
Fix ownership of DhLinks in DhBase (2.45 KB, patch)
2009-09-30 04:06 UTC, Jonathon Jongsma
none Details | Review
Clean up application windows on 'Quit' action (2.51 KB, patch)
2009-09-30 04:06 UTC, Jonathon Jongsma
none Details | Review

Description Jonathon Jongsma 2009-09-30 04:06:00 UTC
The following patches should fix a number of leaks in devhelp that were reported
by valgrind.  Several of these leaks are actually 'leaks' (since they don't grow
over time and they're only leaked at shutdown), so this probably won't result in
a large runtime memory savings, but it's nice to clean them up anyways.
Comment 1 Jonathon Jongsma 2009-09-30 04:06:03 UTC
Created attachment 144341 [details] [review]
Fix leak in view_setup_fonts()

The following two valgrind reports should be fixed by this patch:

==17510== 12 bytes in 1 blocks are definitely lost in loss record 664 of 9,665
==17510==    at 0x4C221A7: malloc (vg_replace_malloc.c:195)
==17510==    by 0x7BAC232: g_malloc (gmem.c:131)
==17510==    by 0x7BC3CFD: g_strdup (gstrfuncs.c:102)
==17510==    by 0x5044E96: split_font_string (dh-util.c:575)
==17510==    by 0x5044EE7: dh_util_font_get_fixed (dh-util.c:647)
==17510==    by 0x5045066: view_setup_fonts (dh-util.c:683)
==17510==    by 0x504627F: window_open_new_tab (dh-window.c:1194)
==17510==    by 0x5047BCC: dh_window_new (dh-window.c:902)
==17510==    by 0x504067D: dh_base_new_window (dh-base.c:441)
==17510==    by 0x4020E7: main (dh-main.c:286)

and

==17510== 11 bytes in 1 blocks are definitely lost in loss record 583 of 9,665
==17510==    at 0x4C221A7: malloc (vg_replace_malloc.c:195)
==17510==    by 0x7BAC232: g_malloc (gmem.c:131)
==17510==    by 0x7BC3CFD: g_strdup (gstrfuncs.c:102)
==17510==    by 0x5044E96: split_font_string (dh-util.c:575)
==17510==    by 0x5044F97: dh_util_font_get_variable (dh-util.c:615)
==17510==    by 0x5045053: view_setup_fonts (dh-util.c:681)
==17510==    by 0x504627F: window_open_new_tab (dh-window.c:1194)
==17510==    by 0x5047BCC: dh_window_new (dh-window.c:902)
==17510==    by 0x504067D: dh_base_new_window (dh-base.c:441)
==17510==    by 0x4020E7: main (dh-main.c:286)
Comment 2 Jonathon Jongsma 2009-09-30 04:06:06 UTC
Created attachment 144342 [details] [review]
Fix leak in search_combo_row_separator_func()

This patch should fix the following leak by freeing the values returned from
gtk_tree_model_get()

==17510== 2,879 bytes in 137 blocks are definitely lost in loss record 9,522 of 9,665
==17510==    at 0x4C221A7: malloc (vg_replace_malloc.c:195)
==17510==    by 0x7BAC232: g_malloc (gmem.c:131)
==17510==    by 0x7BC3CFD: g_strdup (gstrfuncs.c:102)
==17510==    by 0xFA3EADC: value_lcopy_string (gvaluetypes.c:313)
==17510==    by 0xB73A49F: gtk_tree_model_get_valist (gtktreemodel.c:1443)
==17510==    by 0xB73A6E8: gtk_tree_model_get (gtktreemodel.c:1405)
==17510==    by 0x5042643: search_combo_row_separator_func (dh-search.c:482)
==17510==    by 0xB5D8C51: gtk_combo_box_menu_fill_level (gtkcombobox.c:2944)
==17510==    by 0xB5D8FAB: gtk_combo_box_relayout (gtkcombobox.c:3139)
==17510==    by 0xB5DC3EF: gtk_combo_box_set_row_separator_func (gtkcombobox.c:5807)
==17510==    by 0x5042E0C: dh_search_new (dh-search.c:542)
==17510==    by 0x5047980: dh_window_new (dh-window.c:843)
==17510==    by 0x504067D: dh_base_new_window (dh-base.c:441)
==17510==    by 0x4020E7: main (dh-main.c:286)
Comment 3 Jonathon Jongsma 2009-09-30 04:06:08 UTC
Created attachment 144343 [details] [review]
Fix a couple large leaks in DhBase

The following leaks should be fixed by this patch:

==17510== 8,275 (72 direct, 8,203 indirect) bytes in 1 blocks are definitely lost in loss record 9,606 of 9,665
==17510==    at 0x4C221A7: malloc (vg_replace_malloc.c:195)
==17510==    by 0x7BAC232: g_malloc (gmem.c:131)
==17510==    by 0x7BC20A7: g_slice_alloc (gslice.c:824)
==17510==    by 0x7B95B92: g_hash_table_new_full (ghash.c:512)
==17510==    by 0x5040BE7: dh_base_init (dh-base.c:104)
==17510==    by 0xFA39A84: g_type_create_instance (gtype.c:1674)
==17510==    by 0xFA1E7EA: g_object_constructor (gobject.c:1338)
==17510==    by 0xFA1EE5C: g_object_newv (gobject.c:1215)
==17510==    by 0xFA1F9A6: g_object_new_valist (gobject.c:1278)
==17510==    by 0xFA1FAEB: g_object_new (gobject.c:1060)
==17510==    by 0x50409D3: dh_base_get (dh-base.c:413)
==17510==    by 0x402004: main (dh-main.c:280)

and

==17510== 136,240 (40 direct, 136,200 indirect) bytes in 1 blocks are definitely lost in loss record 9,654 of 9,665
==17510==    at 0x4C221A7: malloc (vg_replace_malloc.c:195)
==17510==    by 0x7BAC232: g_malloc (gmem.c:131)
==17510==    by 0x7BC20A7: g_slice_alloc (gslice.c:824)
==17510==    by 0x7BC23D5: g_slice_alloc0 (gslice.c:833)
==17510==    by 0x7BAFBFD: g_node_new (gnode.c:58)
==17510==    by 0x5040BC6: dh_base_init (dh-base.c:103)
==17510==    by 0xFA39A84: g_type_create_instance (gtype.c:1674)
==17510==    by 0xFA1E7EA: g_object_constructor (gobject.c:1338)
==17510==    by 0xFA1EE5C: g_object_newv (gobject.c:1215)
==17510==    by 0xFA1F9A6: g_object_new_valist (gobject.c:1278)
==17510==    by 0xFA1FAEB: g_object_new (gobject.c:1060)
==17510==    by 0x50409D3: dh_base_get (dh-base.c:413)
==17510==    by 0x402004: main (dh-main.c:280)
Comment 4 Jonathon Jongsma 2009-09-30 04:06:11 UTC
Created attachment 144344 [details] [review]
Fix small leak in search_complete_idle()

This patch should fix the following leak:

==17510== 15 bytes in 2 blocks are definitely lost in loss record 868 of 9,665
==17510==    at 0x4C214CB: calloc (vg_replace_malloc.c:418)
==17510==    by 0x7BAC1D9: g_malloc0 (gmem.c:151)
==17510==    by 0x7B8BE8E: g_completion_complete (gcompletion.c:160)
==17510==    by 0x504344B: search_complete_idle (dh-search.c:394)
==17510==    by 0x7BA3E09: g_main_context_dispatch (gmain.c:1960)
==17510==    by 0x7BA7667: g_main_context_iterate (gmain.c:2591)
==17510==    by 0x7BA7B3C: g_main_loop_run (gmain.c:2799)
==17510==    by 0xB6658E6: gtk_main (gtkmain.c:1205)
==17510==    by 0x402044: main (dh-main.c:299)
Comment 5 Jonathon Jongsma 2009-09-30 04:06:14 UTC
Created attachment 144345 [details] [review]
Fix leak of parser->base

There's no need to keep base as a member of the parser struct.  Just keep it as
a local variable and make sure to free it when we're done
Comment 6 Jonathon Jongsma 2009-09-30 04:06:17 UTC
Created attachment 144346 [details] [review]
Factor out the DhParser destructor

Just a little cleanup to make it easier for me to see where things are getting
freed in DhParser
Comment 7 Jonathon Jongsma 2009-09-30 04:06:20 UTC
Created attachment 144347 [details] [review]
Fix ownership of DhLinks in DhBase

Previously there were two collections of lists in DhBase: priv->keywords, and
priv->book_tree.  Neither of these lists had clear ownership of the DhLinks that
they held.  This patch adds a ref to the DhLink before adding it to each list,
and then unrefs the items from each list when destroying DhBase, in order to
prevent leaking of all of these objects.
Comment 8 Jonathon Jongsma 2009-09-30 04:06:23 UTC
Created attachment 144348 [details] [review]
Clean up application windows on 'Quit' action

Previously, when the 'Quit' action was activated, we just called
gtk_main_quit(), but that doesn't give the application a chance to clean up all
open windows, etc.  This resulted in a leak at exit (I know, not a big deal to
leak at exit, but still...) because each open window didn't get a chance to
unref their priv->base member, so the g_object_unref(base) in main() did not
actually result in the base object being destroyed, so the DhBase destructor was
never being called when the application was exited using the 'Quit' action (it
was behaving correctly when the windows were closed by the window manager).
Comment 9 Frederic Peters 2009-09-30 11:08:21 UTC
Thanks, I didn't check thoroughly but I spotted a tab/spaces mismatch in the DhParser patch, just fix it (and others you could notice) and commit them to master (you could also cherry pick them into gnome-2-28).
Comment 10 Jonathon Jongsma 2009-09-30 15:38:16 UTC
Thanks.  fixed indentation, and pushed to master (and cherry-picked to gnome-2-28)