GNOME Bugzilla – Bug 755439
Memory leak in gdbusproxy.c
Last modified: 2016-06-15 14:41:54 UTC
When running gtk3-widget-factory: ==19632== 7 bytes in 1 blocks are definitely lost in loss record 285 of 19,133 ==19632== at 0x4C29E6F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==19632== by 0xAA3CF37: g_malloc (gmem.c:94) ==19632== by 0xAA3D1F2: g_malloc_n (gmem.c:330) ==19632== by 0xAA2242C: g_strdup (gstrfuncs.c:363) ==19632== by 0xA3DDD2E: on_name_owner_changed (gdbusproxy.c:1391) ==19632== by 0xA3F5127: emit_signal_instance_in_idle_cb (gdbusconnection.c:3701) ==19632== by 0xAA44982: g_idle_dispatch.lto_priv.230 (gmain.c:5441) ==19632== by 0xAA41FEC: g_main_dispatch (gmain.c:3154) ==19632== by 0xAA42E22: g_main_context_dispatch (gmain.c:3769) ==19632== by 0xAA43005: g_main_context_iterate (gmain.c:3840) ==19632== by 0xAA430C9: g_main_context_iteration (gmain.c:3901) ==19632== by 0xA434D9F: g_application_run (gapplication.c:2311) ==19632== by 0x40777E: main (widget-factory.c:1636) In gdbusproxy.c:1391 the name_owner gets correctly duped, and then assigned to data->proxy->priv->name_owner in line 1251. I don't know offhand where the correct place to free it would be, since g_dbus_proxy_finalize frees the name_owner.
Seeing a similar trace in gnome-shell. ==20126== 42 bytes in 6 blocks are definitely lost in loss record 15,174 of 48,256 ==20126== at 0x4C280F3: malloc (vg_replace_malloc.c:299) ==20126== by 0x7541D00: g_malloc (gmem.c:104) ==20126== by 0x7558FEE: g_strdup (gstrfuncs.c:364) ==20126== by 0x6DF8E4F: on_name_owner_changed (gdbusproxy.c:1399) ==20126== by 0x6DE94C4: emit_signal_instance_in_idle_cb (gdbusconnection.c:3743) ==20126== by 0x753C315: g_main_dispatch (gmain.c:3066) ==20126== by 0x753C315: g_main_context_dispatch (gmain.c:3642) ==20126== by 0x753C667: g_main_context_iterate.isra.24 (gmain.c:3713) ==20126== by 0x753CA69: g_main_loop_run (gmain.c:3907) ==20126== by 0x5E38000: meta_run (main.c:556) ==20126== by 0x401EC0: main (main.c:441) The problem seems to be that proxy->priv->name_owner gets overwritten in async_init_data_set_name_owner() on the assumption that it will always be NULL when we get there. However, on_name_owner_changed() can run first, and it does set name_owner. I have a patch that fixes the problem, which I will attach shortly.
Created attachment 329009 [details] [review] 0001-GDbusProxy-Plug-memory-leak.patch Patch that fixes the problem.
Reassigning for review.
In every other place where the variable is set, the variable is freed before overwritting it. But it is missed here. It looks good to me.
Review of attachment 329009 [details] [review]: I didn't trace this all the way through myself, but it looks sane.
Comment on attachment 329009 [details] [review] 0001-GDbusProxy-Plug-memory-leak.patch Thanks! Committed to master.