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 754157 - caps: fix memory leak when getting caps from static caps
caps: fix memory leak when getting caps from static caps
Status: RESOLVED NOTABUG
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-08-27 07:41 UTC by Vineeth
Modified: 2015-08-27 08:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix memory leak when getting caps from static caps (1.54 KB, patch)
2015-08-27 07:43 UTC, Vineeth
rejected Details | Review

Description Vineeth 2015-08-27 07:41:34 UTC
In case there is no caps in static caps, then it tries to get caps from string. But gst_caps_from_string creates a new reference, and increasing the reference again leads to memory leak


This can be tested with valgrind of the below pipeline
videotestsrc num-buffers=20 ! dvbsuboverlay ! fakesink

==10149== 1,477 (40 direct, 1,437 indirect) bytes in 1 blocks are definitely lost in loss record 3,633 of 3,658
==10149==    at 0x402C17C: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==10149==    by 0x43D1BE2: g_malloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10149==    by 0x43E8281: g_slice_alloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10149==    by 0x424F13B: gst_caps_new_empty (gstcaps.c:243)
==10149==    by 0x42527E2: gst_caps_from_string (gstcaps.c:2344)
==10149==    by 0x4252B3C: gst_static_caps_get (gstcaps.c:415)
==10149==    by 0x404CBFE: gst_dvbsub_overlay_event_video (gstdvbsuboverlay.c:652)
==10149==    by 0x41EBE8B: gst_validate_pad_monitor_downstream_event_check (gst-validate-pad-monitor.c:1788)
==10149==    by 0x41F024F: gst_validate_pad_monitor_sink_event_func (gst-validate-pad-monitor.c:2103)
==10149==    by 0x427D34C: gst_pad_send_event_unchecked (gstpad.c:5452)
==10149==    by 0x427DAD6: gst_pad_push_event_unchecked (gstpad.c:5123)
==10149==    by 0x427E127: push_sticky (gstpad.c:3690)
==10149==    by 0x427BEF6: events_foreach (gstpad.c:597)
==10149==    by 0x4287D0A: gst_pad_push_event (gstpad.c:3746)
==10149==    by 0x4876C05: gst_base_src_set_caps (gstbasesrc.c:925)
==10149==    by 0x48770B1: gst_base_src_default_negotiate (gstbasesrc.c:3227)
==10149==    by 0x4874439: gst_base_src_loop (gstbasesrc.c:3267)
==10149==    by 0x42B3138: gst_task_func (gsttask.c:331)
==10149==    by 0x42B41EE: default_func (gsttaskpool.c:68)
==10149==    by 0x43F3404: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
Comment 1 Vineeth 2015-08-27 07:43:20 UTC
Created attachment 310073 [details] [review]
fix memory leak when getting caps from static caps
Comment 2 Sebastian Dröge (slomo) 2015-08-27 08:07:42 UTC
Review of attachment 310073 [details] [review]:

::: gst/gstcaps.c
@@ +414,3 @@
       goto no_string;
 
+    string_caps = gst_caps_from_string (string);

This here is the whole point of the static caps :) On the first call, caps are created from the string and stored inside the static caps, and all further calls will just return a new reference of those previously created caps.

The "memory leak" valgrind complains about is a expected one-time allocation.
Comment 3 Tim-Philipp Müller 2015-08-27 08:08:38 UTC
Comment on attachment 310073 [details] [review]
fix memory leak when getting caps from static caps

I think you misunderstood the purposes of GstStaticCaps.

The caps inside static caps are 'leaked' (kept around forever) on purpose.

Use valgrind --show-reachable=no.
Comment 4 Sebastian Dröge (slomo) 2015-08-27 08:11:57 UTC
The actual bug it complained about is this btw

commit 50fc332ab5dafcc9ac4b0e2afe2207b1eca79b2c
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Thu Aug 27 11:10:05 2015 +0300

    dvbsuboverlay: Fix caps memory leak by making static caps actually static
    
    See https://bugzilla.gnome.org/show_bug.cgi?id=754157



I thought I checked for all of these a few weeks ago already and fixed them all :) Apparently I missed at least one
Comment 5 Vineeth 2015-08-27 08:22:59 UTC
:) thanks for the clarification
Comment 6 Vineeth 2015-08-27 08:37:12 UTC
just one more doubt :)..

can't we use sw_template_caps, instead of static_caps got from
static GstStaticCaps static_caps = GST_STATIC_CAPS (DVBSUB_OVERLAY_CAPS);
again?
Comment 7 Sebastian Dröge (slomo) 2015-08-27 08:40:22 UTC
We could get the caps from the template, yes.