GNOME Bugzilla – Bug 672006
[playsink] leaks every instance
Last modified: 2012-07-25 09:01:57 UTC
Both playsink and the helper internal convert bin use following fragments frequently: gst_pad_set_blocked_async_full (opad, FALSE, sinkpad_blocked_cb, gst_object_ref (playsink), (GDestroyNotify) gst_object_unref); The result is a (indirect) circular ref between opad (a pad somewhere internal in the bin) and the bin (e.g. playsink), so none of this gets cleaned up at overall pipeline _unref time. It is also not reported by valgrind, unless one digs into the "still reachable". Not entirely sure why additional refs were taken here in the first place in such persistent fashion to begin with ...
Created attachment 209626 [details] [review] playsink: avoid dangling circular refs Patch breaks circular ref at shutdown (state change) time. Only works though if core is also patched slightly (patch to follow).
Created attachment 209627 [details] [review] pad: also update callback info when pad in desired blocked state To allow previous patch to really get rid of extra ref, core needs patching slightly to update callback info in more cases. Since it seems that the fields in question appear to be public ones in the pad instance struct, the previous patch could also have manipulated those directly (or done something else altogether), but this feels like a cleaner approach.
I think the additional refs/unrefs for the pad blocks are not needed at all here.
Created attachment 209686 [details] [review] playsink: avoid circular ref between bin and internal pad In that case, alternatively, present patch removes the additional _ref/_unref altogether.
Could you check in the other elements in the playback directory too? IIRC the same pattern is used in subtitleoverlay too, and decodebin2 too maybe. Taking an additional ref should always be unneeded if the pad in question belongs to a child element of our bin
Looks like it was already fixed in subtitleoverlay a while ago, and the extra ref in decodebin2 is taken on some internal pad, so it does not create a circular ref and will eventually go away.
So, latest patch should do it then: commit 9fc640b9e0ba807b7f8ce6bdbd7c754bdbfdb761 Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Wed Mar 14 11:04:25 2012 +0100 playsink: remove circular ref between bin and internal pad ... by not assigning an additional ref to an async blocked callback, which should not be called anyway by the time the object is gone. Fixes #672006.
(In reply to comment #7) > So, latest patch should do it then: > > commit 9fc640b9e0ba807b7f8ce6bdbd7c754bdbfdb761 > Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> > Date: Wed Mar 14 11:04:25 2012 +0100 > > playsink: remove circular ref between bin and internal pad > > ... by not assigning an additional ref to an async blocked callback, > which should not be called anyway by the time the object is gone. > > Fixes #672006. hi, Is there any patch necessary for gstreamer-0.10.36 tar file for this issue?
No, the patch is in gst-plugins-base, and I gave you the link in the other bug report.
(In reply to comment #9) > No, the patch is in gst-plugins-base, and I gave you the link in the other bug > report. hi Sir, this may the cause for memory leak issue in base 0.10.36. the verification testing is running. Because the previous comments mentioned the patch for core (gstpad.c), I just like to confirm whether necessary patch core 0.10.36 or not. And, Could you explain more detail about : playsink: remove circular ref between bin and internal pad thanks!