GNOME Bugzilla – Bug 755247
{track,videotrack,smart-video-mixer}: leaks their internal objects
Last modified: 2015-09-30 10:02:16 UTC
Here's one of report from valgrind which my proposed patch solved. (When I run valgrind, too many points reported so I add this partially) ==29654== 8,932 (464 direct, 8,468 indirect) bytes in 1 blocks are definitely lost in loss record 3,164 of 3,172 ==29654== at 0x6B6FD8A: g_type_create_instance (gtype.c:1849) ==29654== by 0x6B53246: g_object_new_internal (gobject.c:1774) ==29654== by 0x6B54B9C: g_object_newv (gobject.c:1922) ==29654== by 0x6B552EB: g_object_new (gobject.c:1614) ==29654== by 0x4E89F91: ges_smart_mixer_new (ges-smart-video-mixer.c:273) ==29654== by 0x4E681BF: ges_video_track_get_mixing_element (ges-video-track.c:134) ==29654== by 0x4E66430: ges_track_constructed (ges-track.c:514) ==29654== by 0x6B533E3: g_object_new_internal (gobject.c:1814) ==29654== by 0x6B54FC4: g_object_new_valist (gobject.c:2034) ==29654== by 0x6B552D3: g_object_new (gobject.c:1617) ==29654== by 0x4E682CC: ges_video_track_new (ges-video-track.c:172) ==29654== by 0x401463: test_ges_scenario (basic.c:69) ==29654== by 0x6315018: tcase_run_tfun_fork (check_run.c:450) ==29654== by 0x6315018: srunner_iterate_tcase_tfuns (check_run.c:222) ==29654== by 0x6315018: srunner_run_tcase (check_run.c:362) ==29654== by 0x6315018: srunner_iterate_suites (check_run.c:195) ==29654== by 0x6315018: srunner_run (check_run.c:706) ==29654== by 0x630A3CD: gst_check_run_suite (gstcheck.c:824) ==29654== by 0x4011BC: main (basic.c:798)
Created attachment 311663 [details] [review] {track,videotrack,smart-video-mixer}: don't leak internal object
Review of attachment 311663 [details] [review]: ::: ges/ges-smart-video-mixer.c @@ -205,2 +205,3 @@ } + if (!gst_element_remove_pad (GST_ELEMENT (self), self->srcpad)) { That should not happen, and we should anyway have the infor from gst_element_remove_pad anyway. ::: ges/ges-track.c @@ +465,3 @@ + if (priv->nleobject) { + if (priv->mixer) { + gst_bin_remove (GST_BIN (priv->nleobject), priv->mixer); That should be done disposing ourself, then recursing down to all the object contained in it I think, why do you think it is needed here? @@ -520,2 +534,3 @@ } + self->priv->mixer = gst_object_ref (mixer); We should not need that, adding the mixer to the nleobject guarantees us we can use the mixer until we destroy the nleobject.
Comment on attachment 311663 [details] [review] {track,videotrack,smart-video-mixer}: don't leak internal object Yes, you are right. My proposed patch is totally wrong I just thought about how to decrease ref count. GstBin already does nested unref for its children. I missed it. However, it's also true that nleobject and mixer are not disposed even after calling gst_object_unref (track). I'll try to find the root cause of this leakage again. Thank you for review.
Created attachment 311775 [details] [review] timeline-element,track,framepositionner: don't leaks internal object
Review of attachment 311775 [details] [review]: Looks right :)
ping? please merge this if there's wrong. :)
(In reply to Justin J. Kim from comment #6) > ping? please merge this if there's wrong. :) sorry .. wrong ==> right.
commit 731bda15926418bedd9a7e1343efdd49ae3dc108 Author: Justin Kim <justin.kim@collabora.com> Date: Tue Sep 22 01:06:00 2015 +0900 timeline-element,track,framepositionner: don't leak internal object https://bugzilla.gnome.org/show_bug.cgi?id=755247