GNOME Bugzilla – Bug 755867
playsink: fix leak of audio sink
Last modified: 2015-10-17 08:04:57 UTC
Created attachment 312413 [details] [review] playsink: fix leak of audio sink The leak was introduced in commit f99a24f8b3 (playsink: Require the streamvolume interface on the sink when using the sink).
It looks right at first glance. I tried to write a unit test for it and it executes that code path, but valgrind shows no leak. But then it doesn't either if I add extra refs, so I don't know what's going on there.
I don't know about valgrind, I'm using refdbg to trace GObjects leaks. Are you using G_SLICE=always-malloc when running valgrind ?
I also had it a few times that valgrind didn't detect obvious leaks. So let's get this in
Review of attachment 312413 [details] [review]: ::: gst/playback/gstplaysink.c @@ +2768,3 @@ if (elem) { chain->volume = elem; + gst_object_unref (elem); Why is this correct? And there's the same code in line 2971 :) I wonder why we don't keep another reference around of the chain->volume thing... and unref it later. That would seem more consistent.
*** Bug 756552 has been marked as a duplicate of this bug. ***
commit b424bc2e4f9a3bc80f713194419bdb3153d4f08d Author: Vineeth TM <vineeth.tm@samsung.com> Date: Thu Oct 15 10:01:38 2015 +0900 playsink: Fix volume element leak In case sink implements a streamvolume interface, volume element is being got from the sink. But this is transfer full. So the memory should be freed before setting it to NULL. This was resulting in major memory leaks https://bugzilla.gnome.org/show_bug.cgi?id=755867
Now with the testcase from bug #756552, I still get > GStreamer-CRITICAL **: gst_poll_get_read_gpollfd: assertion 'set != NULL' failed after a while. So we still leak *something* :)
After 508 iterations btw, and it's the bus that is failing with this error when creating it.
... because we're leaking all playbin instances for whatever reason.
Yeah.. this was not happening for me, because i changed to proper file path g_object_set (G_OBJECT (playbin), "uri", "file:///home/vineethtm/gst/master/media/11.ogg", NULL); With the same code it is still leaking because of the file not available.. Have to check more about this.
and just for reference, this leak is happening even without the commit mentioned above.. so there seems to be other leaks.
So in error cases we're leaking something somewhere. Start with checking where we first detect the error, and if we leak something in the error code path there. Or if we leak something when an error message arrives somewhere in a message handler inside uridecodebin or playbin.
Let's handle that in bug #756611 then :)
Thanks for the quick fix. Will this get pushed to 1.6 as well?
Yes, soon