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 628482 - e_shell_settings_install_property_for_key memory leak
e_shell_settings_install_property_for_key memory leak
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: Shell
2.32.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: Evolution Shell Maintainers Team
Evolution QA team
Depends on:
Blocks: 627707
 
 
Reported: 2010-09-01 08:54 UTC by David Woodhouse
Modified: 2013-09-13 01:04 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description David Woodhouse 2010-09-01 08:54:05 UTC
==20949== 124 (32 direct, 92 indirect) bytes in 1 blocks are definitely lost in loss record 22,216 of 28,897
==20949==    at 0x4A0515D: malloc (vg_replace_malloc.c:195)
==20949==    by 0x3D3D645C80: g_malloc (gmem.c:134)
==20949==    by 0x3D3D65C796: g_slice_alloc (gslice.c:836)
==20949==    by 0x3D4121D57D: gconf_entry_new_nocopy (in /usr/lib64/libgconf-2.so.4.1.5)
==20949==    by 0x3D4122181B: gconf_engine_get_entry (in /usr/lib64/libgconf-2.so.4.1.5)
==20949==    by 0x3D41227C7D: ??? (in /usr/lib64/libgconf-2.so.4.1.5)
==20949==    by 0x3D41227E41: gconf_client_get_entry (in /usr/lib64/libgconf-2.so.4.1.5)
==20949==    by 0x4C2B4C6: e_shell_settings_install_property_for_key (e-shell-settings.c:61)
==20949==    by 0x1048AFAC: e_cal_shell_backend_init_settings (e-cal-shell-settings.c:497)
==20949==    by 0x104887A2: cal_shell_backend_constructed (e-cal-shell-backend.c:708)
==20949==    by 0x3D3DE1388F: g_object_newv (gobject.c:1375)
==20949==    by 0x3D3DE14310: g_object_new_valist (gobject.c:1463)
Comment 2 David Woodhouse 2010-09-01 11:20:55 UTC
There are still ways out of that function which leak 'entry'. Any of the 'goto fail' paths, for example.

Is there a reason not to free 'entry' as soon as it's been used for the last time, and then forget about it?

--- a/shell/e-shell-settings.c
+++ b/shell/e-shell-settings.c
@@ -66,12 +66,12 @@ shell_settings_pspec_for_key (const gchar *property_name,
        }
 
        schema_name = gconf_entry_get_schema_name (entry);
+       gconf_entry_unref(entry);
        g_return_val_if_fail (schema_name != NULL, NULL);
 
        schema = gconf_client_get_schema (client, schema_name, &error);
        if (error != NULL) {
                g_warning ("%s", error->message);
-               gconf_entry_unref (entry);
                g_error_free (error);
                return NULL;
        }
@@ -160,7 +160,6 @@ shell_settings_pspec_for_key (const gchar *property_name,
 
        gconf_value_free (default_value);
        gconf_schema_free (schema);
-       gconf_entry_unref (entry);
 
        return pspec;
Comment 3 David Woodhouse 2010-09-01 11:23:29 UTC
Oh, it would have aborted in the 'goto fail' case anyway, so I suppose there's no real leak.
Comment 4 Matthew Barnes 2010-09-01 11:35:40 UTC
gconf_entry_get_schema_name() returns a constant string, which I assume GConfEntry owns.  Destroying GConfEntry before we're done with the string would likely leave a dangling 'schema_name' pointer.