[libcamera-devel] [PATCH 2/2] test: gstreamer_single_stream_test: Fix memory leak

[Vedant Paranjape]

Hi Umang, I'll take ownership of these patches.
Thanks for the fix. I was missing a key info about floating refs, so I
assumed that pipeline will take care of lifetime of other elements.

Regards,
Vedant Paranjape

On Tue, 14 Sep, 2021, 23:24 Umang Jain, <umang.jain at ideasonboard.com> wrote:

> Hi Nicolas,
>
> On 9/14/21 11:15 PM, Nicolas Dufresne wrote:
> > Le mardi 14 septembre 2021 à 17:47 +0530, Umang Jain a écrit :
> >> The test hold a valid reference to convert0_ and sink0_ but
> >> not released. This results in a memory leak and can be checked
> >> via valgrind. Drop the references with test cleanup() virtual
> >> function.
> >>
> >> Valgrind log:
> >> ==95352==    definitely lost: 1,688 bytes in 2 blocks
> >> ==95352==    indirectly lost: 11,901 bytes in 37 blocks
> >>
> >> The patch fixes the leaks reported by valgrind above to:
> >> ==120397==    definitely lost: 0 bytes in 0 blocks
> >> ==120397==    indirectly lost: 0 bytes in 0 blocks
> >>
> >> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> >> ---
> >>   test/gstreamer/gstreamer_single_stream_test.cpp | 6 ++++++
> >>   1 file changed, 6 insertions(+)
> >>
> >> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp
> b/test/gstreamer/gstreamer_single_stream_test.cpp
> >> index d992455c..763a77b1 100644
> >> --- a/test/gstreamer/gstreamer_single_stream_test.cpp
> >> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp
> >> @@ -70,6 +70,12 @@ protected:
> >>              return TestPass;
> >>      }
> >>
> >> +    void cleanup() override
> >> +    {
> >> +            gst_object_unref(convert0_);
> >> +            gst_object_unref(sink0_);
> >> +    }
> > This is correct of course, but I think we could cleanup more the test.
> With a
> > run/cleanup() set of function, I would expect to be able to loop over
> and run
> > the test multiple times, but it does not seem to be the case due to the
> fact the
>
>
> Indeed, we can and should. I went in a rush to fix the leak, to see if
> that's all we leak or more!
>
> +cc Vedant,
>
> Vedant, could you assume ownership of these patches/series and work upon
> it? I won't have more time to allocate to this and it will  generally
> help you as well to fix the multi stream test on top of this as well and
> make sure we don't have leaks anywhere.
>
> > pipeline is not fully constructed in run().
> >
> > For the leak being fixed:
> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
>
>
> Thanks for taking a look Nicolas.
>
> >
> >> +
> >>   private:
> >>      GstElement *convert0_;
> >>      GstElement *sink0_;
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210914/793ae62b/attachment.htm>