Bug 25582 - Old-style Properties mixin leaks, has suspicious behaviour
Summary: Old-style Properties mixin leaks, has suspicious behaviour
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/sm...
Whiteboard: review+
Keywords: patch
Depends on:
Blocks:
 
Reported: 2009-12-11 04:45 UTC by Simon McVittie
Modified: 2010-09-10 08:09 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Simon McVittie 2009-12-11 04:45:49 UTC
19:55:13 <daf> smcv: any idea on http://paste.debian.net/53691/ ?
19:58:33 <smcv> daf: I think telepathy-glib/properties-mixin.c line 
433 leaks the GValue itself
19:58:58 <smcv> daf: (but not its contents!)
19:59:12 <daf> smcv: want me to file a bug?
19:59:23 <daf> or do you want to instafix it?
20:00:11 <smcv> daf: please, with a tested patch (which I think is to g_free it as well, or use g_boxed_free (G_TYPE_VALUE, prop_val))
20:00:32 <smcv> daf: check with lcov that the offending code was executed without crashing
20:01:01 <smcv> daf: I *think* the GValue boxed type is defined as being g_malloc'd but I can't be sure

Contents of that paste:

=15935== 40 bytes in 2 blocks are definitely lost in loss record 2,958 of 4,627
==15935==    at 0x4023F8B: calloc (vg_replace_malloc.c:418)
==15935==    by 0x42820CB: g_malloc0 (gmem.c:151)
==15935==    by 0x41B22B8: value_copy (gboxed.c:94)
==15935==    by 0x41B26A9: boxed_proxy_lcopy_value (gboxed.c:399)
==15935==    by 0x414F7C5: dbus_g_type_struct_get (in /usr/lib/libdbus-glib-1.so.2.1.0)
==15935==    by 0x4396D92: tp_properties_mixin_set_properties (properties-mixin.c:425)
==15935==    by 0x4396F1A: set_properties (properties-mixin.c:1137)
==15935==    by 0x43AFC86: tp_svc_properties_interface_set_properties (tp-svc-generic.c:465)
==15935==    by 0x43A2E62: _tp_marshal_VOID__BOXED_POINTER (signals-marshal.c:494)
==15935==    by 0x41441BA: ??? (in /usr/lib/libdbus-glib-1.so.2.1.0)
==15935==    by 0x4174694: ??? (in /lib/libdbus-1.so.3.4.0)
==15935==    by 0x4165DC3: dbus_connection_dispatch (in /lib/libdbus-1.so.3.4.0)
==15935==    by 0x4141BDC: ??? (in /usr/lib/libdbus-glib-1.so.2.1.0)
==15935==    by 0x4279E97: g_main_context_dispatch (gmain.c:1960)
==15935==    by 0x427D622: g_main_context_iterate (gmain.c:2591)
==15935==    by 0x427DAE9: g_main_loop_run (gmain.c:2799)
==15935==    by 0x439E47F: tp_run_connection_manager (run.c:281)
==15935==    by 0x8063975: gabble_main (gabble.c:161)
==15935==    by 0x8056E8F: main (main-debug.c:48)

Further comments:

20:28:18 <      daf> smcv: hm
20:28:36 <      daf> smcv: I guess I'd need to build an lcov-enabled tp-glib
                     and then run the Gabble test suite against it
20:29:44 <      daf> smcv: I don't understand this code though
20:30:21 <      daf> smcv: the GValue gets stashed in the conctext on line
                     442, but it's immediately followed by things that return
                     errors
Comment 1 Simon McVittie 2010-04-26 05:09:47 UTC
I think this is mostly a matter of constructing a regression test?
Comment 2 Simon McVittie 2010-09-10 04:44:48 UTC
Stealing this bug.
Comment 3 Simon McVittie 2010-09-10 05:49:25 UTC
Fixed in a branch:

http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=shortlog;h=refs/heads/leaky-25582

There are no regression tests in that branch, but a Gabble built against the patched telepathy-glib passes tests and doesn't crash... I also added this extra test to improve coverage:

http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=shortlog;h=refs/heads/muc-invalid-test
Comment 4 Will Thompson 2010-09-10 05:54:01 UTC
both of these look fine!
Comment 5 Simon McVittie 2010-09-10 08:09:51 UTC
Fixed in git for 0.11.15


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.