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

[Umang Jain]

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_;
>