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 782784 - Weak references bug in gst-validate
Weak references bug in gst-validate
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-devtools
1.12.0
Other Linux
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-05-18 12:18 UTC by Sebastian Dröge (slomo)
Modified: 2017-06-01 21:03 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Sebastian Dröge (slomo) 2017-05-18 12:18:12 UTC
This makes running gst-validate in valgrind rather useless. Problem here is that something (the monitor) has a weak reference to something else (some tracer?), but the monitor is destroyed first without properly destroying the weak reference. So that at a later point when the tracer (?) is destroyed, the weak reference in the already destroyed monitor is tried to be set to NULL.


==5001== Invalid write of size 8
==5001==    at 0x614DE45: g_nullify_pointer (gutils.c:2059)
==5001==    by 0x5E8FB0E: weak_refs_notify (gobject.c:2638)
==5001==    by 0x5E90C5B: g_object_unref (gobject.c:3174)
==5001==    by 0x5BEF4E3: _priv_gst_tracing_deinit (gsttracerutils.c:157)
==5001==    by 0x5B7303F: gst_deinit (gst.c:1033)
==5001==    by 0x10ABF7: main (gst-validate.c:525)
==5001==  Address 0x9bf0f00 is 0 bytes inside a block of size 40 free'd
==5001==    at 0x4C2CDDB: free (vg_replace_malloc.c:530)
==5001==    by 0x60FCFE2: g_datalist_clear (gdataset.c:273)
==5001==    by 0x5E90C79: g_object_unref (gobject.c:3185)
==5001==    by 0x10ABB5: main (gst-validate.c:515)
==5001==  Block was alloc'd at
==5001==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==5001==    by 0x611EE08: g_malloc (gmem.c:94)
==5001==    by 0x6137342: g_slice_alloc (gslice.c:1025)
==5001==    by 0x613796D: g_slice_alloc0 (gslice.c:1051)
==5001==    by 0x51DEA41: gst_validate_reporter_get_priv (gst-validate-reporter.c:87)
==5001==    by 0x51DEBED: gst_validate_reporter_set_name (gst-validate-reporter.c:321)
==5001==    by 0x5E90E2C: object_set_property (gobject.c:1423)
==5001==    by 0x5E90E2C: g_object_constructor (gobject.c:2082)
==5001==    by 0x51E00F7: gst_validate_monitor_constructor (gst-validate-monitor.c:187)
==5001==    by 0x5E91028: g_object_new_with_custom_constructor (gobject.c:1701)
==5001==    by 0x5E91028: g_object_new_internal (gobject.c:1781)
==5001==    by 0x5E9310D: g_object_new_valist (gobject.c:2042)
==5001==    by 0x5E933B0: g_object_new (gobject.c:1626)
==5001==    by 0x51E29FD: gst_validate_pipeline_monitor_new (gst-validate-pipeline-monitor.c:641)
==5001==    by 0x51EB876: gst_validate_monitor_factory_create (gst-validate-monitor-factory.c:73)
==5001==    by 0x10A92F: main (gst-validate.c:436)
==5001==
Comment 1 Sebastian Dröge (slomo) 2017-05-19 09:27:44 UTC
Thibault sent this patch on IRC: https://www.irccloud.com/pastebin/Kj1hOrbj/

Unfortunately that does not solve it, but it looks correct nonetheless. I assume there are similar problems with the other weak refs, on a short look it seems like that it least.

Also it might make sense to use GWeakRef instead, as that one is actually thread-safe.
Comment 2 Thibault Saunier 2017-06-01 21:03:09 UTC
commit 86e9135b56bb23e8535ad3524bfe101cc4e49f33
Author: Thibault Saunier <thibault.saunier@osg.samsung.com>
Date:   Tue May 30 16:13:08 2017 -0400

    validate: Use GWeakRefs on monitor target and pipeline
    
    Making it thread safe and more future proof (though having them point
    to NULL might not be handled all around).
    
    https://bugzilla.gnome.org/show_bug.cgi?id=782784


---

This breaks the API, but well, I always warned that the API was not really stable and anyay better break the API than to have a broken API I believe.