[{"id":19683,"web_url":"https://patchwork.libcamera.org/comment/19683/","msgid":"<29bb4d09d1f96b3587912d41889739938fe8d332.camel@ndufresne.ca>","date":"2021-09-14T17:45:49","subject":"Re: [libcamera-devel] [PATCH 2/2] test:\n\tgstreamer_single_stream_test: Fix memory leak","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le mardi 14 septembre 2021 à 17:47 +0530, Umang Jain a écrit :\n> The test hold a valid reference to convert0_ and sink0_ but\n> not released. This results in a memory leak and can be checked\n> via valgrind. Drop the references with test cleanup() virtual\n> function.\n> \n> Valgrind log:\n> ==95352== definitely lost: 1,688 bytes in 2 blocks\n> ==95352== indirectly lost: 11,901 bytes in 37 blocks\n> \n> The patch fixes the leaks reported by valgrind above to:\n> ==120397== definitely lost: 0 bytes in 0 blocks\n> ==120397== indirectly lost: 0 bytes in 0 blocks\n> \n> Signed-off-by: Umang Jain \n> ---\n> test/gstreamer/gstreamer_single_stream_test.cpp | 6 ++++++\n> 1 file changed, 6 insertions(+)\n> \n> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp\n> index d992455c..763a77b1 100644\n> --- a/test/gstreamer/gstreamer_single_stream_test.cpp\n> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp\n> @@ -70,6 +70,12 @@ protected:\n> \t\treturn TestPass;\n> \t}\n> \n> +\tvoid cleanup() override\n> +\t{\n> +\t\tgst_object_unref(convert0_);\n> +\t\tgst_object_unref(sink0_);\n> +\t}\n\nThis is correct of course, but I think we could cleanup more the test. With a\nrun/cleanup() set of function, I would expect to be able to loop over and run\nthe test multiple times, but it does not seem to be the case due to the fact the\npipeline is not fully constructed in run().\n\nFor the leak being fixed:\nReviewed-by: Nicolas Dufresne \n\n> +\n> private:\n> \tGstElement *convert0_;\n> \tGstElement *sink0_;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 90329BDC71\n\tfor ;\n\tTue, 14 Sep 2021 17:45:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0A7DC69183;\n\tTue, 14 Sep 2021 19:45:53 +0200 (CEST)","from mail-qt1-x82e.google.com (mail-qt1-x82e.google.com\n\t[IPv6:2607:f8b0:4864:20::82e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8AB8360132\n\tfor ;\n\tTue, 14 Sep 2021 19:45:51 +0200 (CEST)","by mail-qt1-x82e.google.com with SMTP id x5so12219247qtq.13\n\tfor ;\n\tTue, 14 Sep 2021 10:45:51 -0700 (PDT)","from nicolas-tpx395.localdomain (173-246-12-168.qc.cable.ebox.net.\n\t[173.246.12.168]) by smtp.gmail.com with ESMTPSA id\n\tr140sm8068729qke.15.2021.09.14.10.45.49\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 14 Sep 2021 10:45:49 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20150623.gappssmtp.com\n\theader.i=@ndufresne-ca.20150623.gappssmtp.com\n\theader.b=\"GOChSzaJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20150623.gappssmtp.com; s=20150623;\n\th=message-id:subject:from:to:date:in-reply-to:references:user-agent\n\t:mime-version:content-transfer-encoding;\n\tbh=xVP7hEQacRj5v3fU2DFPu2OREGreOB0nppi4EQP93Xo=;\n\tb=GOChSzaJSZTWl0ZsRp2zaFgTG/Fl3kuLSsr23+BTWC1QB8zlJuNT/QXEXddscaD4//\n\taiHM51UMexvD3M9Ll7GN+40PBstRx3pbosyeAngxhRlOi+x9BhlfJddQNmG7GmroHDZ/\n\tXxq+LBxZokg46F9gfMrF5U0o3tJzIlTcpdrpmzw9XYe5dunhjESWtMtplikvLhZDsgER\n\tlQl30iO13VpZywW4mYgicatwgngY/TfcjTsa9UaVcEVcOPqPkRyvaeycemgTxGWI+5wT\n\tOz8vLDIea2nQNwUhxVdKpCkWhIG/oI2krsH9/gvInFW0J8Nntd6u4SZHyrQVx2APP+mH\n\tI6sQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:message-id:subject:from:to:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=xVP7hEQacRj5v3fU2DFPu2OREGreOB0nppi4EQP93Xo=;\n\tb=OQOu6D0+gmdOOWcrNFtjLeUmcny3e4w8T9n6HbFwXqXG5YryyWVYy90kubp6HtDQB6\n\tIgIu2U7QcSWVF4OQanbgQZlsMq/fNQBedlxPcHH+M3Drs1DNRroo2v9QcJYbAvG7DerB\n\tmrzeGfHLC0nb9dTO2KVJoccotHGP151lVUkxy5nSjqiWmjtT67EH8q4W1sIYpoIKajSg\n\tg8N8WJ2hBl/j9a/tNLP+4wX3PN2DspWAPMEC+KaE3zhVsOCmgt5lLzKYhmQUYYTprSPX\n\tz3I39YR/AhfghtP3aIXO/Hp2oc3GizsnmCQb1xZINDOQcJLvxtghjzWkW8jdNRNvMVLe\n\tifIg==","X-Gm-Message-State":"AOAM533Gt8cE+8w90GLq5YNjX9rlWj2O5oBnDtJdwFNdYMlcvY7ufhQP\n\tOmSObPcQzFdc9K2N8p3M6GxE4b+evKR/vg==","X-Google-Smtp-Source":"ABdhPJwlC00yXR7d0Ut+REnK0e3OyUo6C8/hgyZY9HLwWQ5uLKFiY/Zx6C/mmhMg4Xj6fgA1AVcioQ==","X-Received":"by 2002:ac8:7c45:: with SMTP id o5mr5953223qtv.54.1631641550286; \n\tTue, 14 Sep 2021 10:45:50 -0700 (PDT)","Message-ID":"<29bb4d09d1f96b3587912d41889739938fe8d332.camel@ndufresne.ca>","From":"Nicolas Dufresne ","To":"Umang Jain , \n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 14 Sep 2021 13:45:49 -0400","In-Reply-To":"<20210914121700.122591-3-umang.jain@ideasonboard.com>","References":"<20210914121700.122591-1-umang.jain@ideasonboard.com>\n\t<20210914121700.122591-3-umang.jain@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.40.4 (3.40.4-1.fc34) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 2/2] test:\n\tgstreamer_single_stream_test: Fix memory leak","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19686,"web_url":"https://patchwork.libcamera.org/comment/19686/","msgid":"","date":"2021-09-14T17:53:54","subject":"Re: [libcamera-devel] [PATCH 2/2] test:\n\tgstreamer_single_stream_test: Fix memory leak","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Nicolas,\n\nOn 9/14/21 11:15 PM, Nicolas Dufresne wrote:\n> Le mardi 14 septembre 2021 à 17:47 +0530, Umang Jain a écrit :\n>> The test hold a valid reference to convert0_ and sink0_ but\n>> not released. This results in a memory leak and can be checked\n>> via valgrind. Drop the references with test cleanup() virtual\n>> function.\n>>\n>> Valgrind log:\n>> ==95352== definitely lost: 1,688 bytes in 2 blocks\n>> ==95352== indirectly lost: 11,901 bytes in 37 blocks\n>>\n>> The patch fixes the leaks reported by valgrind above to:\n>> ==120397== definitely lost: 0 bytes in 0 blocks\n>> ==120397== indirectly lost: 0 bytes in 0 blocks\n>>\n>> Signed-off-by: Umang Jain \n>> ---\n>> test/gstreamer/gstreamer_single_stream_test.cpp | 6 ++++++\n>> 1 file changed, 6 insertions(+)\n>>\n>> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp\n>> index d992455c..763a77b1 100644\n>> --- a/test/gstreamer/gstreamer_single_stream_test.cpp\n>> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp\n>> @@ -70,6 +70,12 @@ protected:\n>> \t\treturn TestPass;\n>> \t}\n>> \n>> +\tvoid cleanup() override\n>> +\t{\n>> +\t\tgst_object_unref(convert0_);\n>> +\t\tgst_object_unref(sink0_);\n>> +\t}\n> This is correct of course, but I think we could cleanup more the test. With a\n> run/cleanup() set of function, I would expect to be able to loop over and run\n> the test multiple times, but it does not seem to be the case due to the fact the\n\n\nIndeed, we can and should. I went in a rush to fix the leak, to see if \nthat's all we leak or more!\n\n+cc Vedant,\n\nVedant, could you assume ownership of these patches/series and work upon \nit? I won't have more time to allocate to this and it will  generally \nhelp you as well to fix the multi stream test on top of this as well and \nmake sure we don't have leaks anywhere.\n\n> pipeline is not fully constructed in run().\n>\n> For the leak being fixed:\n> Reviewed-by: Nicolas Dufresne \n\n\nThanks for taking a look Nicolas.\n\n>\n>> +\n>> private:\n>> \tGstElement *convert0_;\n>> \tGstElement *sink0_;\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5A241BDB1D\n\tfor ;\n\tTue, 14 Sep 2021 17:54:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B27FD60249;\n\tTue, 14 Sep 2021 19:54:01 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F115D60248\n\tfor ;\n\tTue, 14 Sep 2021 19:53:59 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.94])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D105F2A5;\n\tTue, 14 Sep 2021 19:53:58 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"IfzrSg4m\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631642039;\n\tbh=ppt8wOBbFkZeo6cXvTsrz7kux9+BkcqZeWlwip5Kd4g=;\n\th=Subject:To:References:Cc:From:Date:In-Reply-To:From;\n\tb=IfzrSg4mbESn25mM7wXwc1P5Oj64azmxMazBpSom4WcDKihlGi6u/Qr/GlM4FiU/m\n\ta/jXkgsAQa8Y4bTz9LelG6+wdkylzYfiT5HiZxTNEkcEsMigkGWEL3XgUtjDaNuoD9\n\tbt2MQ6eh7mhfJbh36j/NTBCkt9GdgVMTuOqDUUeo=","To":"Nicolas Dufresne ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210914121700.122591-1-umang.jain@ideasonboard.com>\n\t<20210914121700.122591-3-umang.jain@ideasonboard.com>\n\t<29bb4d09d1f96b3587912d41889739938fe8d332.camel@ndufresne.ca>","From":"Umang Jain ","Message-ID":"","Date":"Tue, 14 Sep 2021 23:23:54 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<29bb4d09d1f96b3587912d41889739938fe8d332.camel@ndufresne.ca>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 2/2] test:\n\tgstreamer_single_stream_test: Fix memory leak","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Vedant Paranjape ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19687,"web_url":"https://patchwork.libcamera.org/comment/19687/","msgid":"","date":"2021-09-14T17:57:00","subject":"Re: [libcamera-devel] [PATCH 2/2] test:\n\tgstreamer_single_stream_test: Fix memory leak","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Hi Umang, I'll take ownership of these patches.\nThanks for the fix. I was missing a key info about floating refs, so I\nassumed that pipeline will take care of lifetime of other elements.\n\nRegards,\nVedant Paranjape\n\nOn Tue, 14 Sep, 2021, 23:24 Umang Jain, wrote:\n\n> Hi Nicolas,\n>\n> On 9/14/21 11:15 PM, Nicolas Dufresne wrote:\n> > Le mardi 14 septembre 2021 à 17:47 +0530, Umang Jain a écrit :\n> >> The test hold a valid reference to convert0_ and sink0_ but\n> >> not released. This results in a memory leak and can be checked\n> >> via valgrind. Drop the references with test cleanup() virtual\n> >> function.\n> >>\n> >> Valgrind log:\n> >> ==95352== definitely lost: 1,688 bytes in 2 blocks\n> >> ==95352== indirectly lost: 11,901 bytes in 37 blocks\n> >>\n> >> The patch fixes the leaks reported by valgrind above to:\n> >> ==120397== definitely lost: 0 bytes in 0 blocks\n> >> ==120397== indirectly lost: 0 bytes in 0 blocks\n> >>\n> >> Signed-off-by: Umang Jain \n> >> ---\n> >> test/gstreamer/gstreamer_single_stream_test.cpp | 6 ++++++\n> >> 1 file changed, 6 insertions(+)\n> >>\n> >> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp\n> b/test/gstreamer/gstreamer_single_stream_test.cpp\n> >> index d992455c..763a77b1 100644\n> >> --- a/test/gstreamer/gstreamer_single_stream_test.cpp\n> >> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp\n> >> @@ -70,6 +70,12 @@ protected:\n> >> return TestPass;\n> >> }\n> >>\n> >> + void cleanup() override\n> >> + {\n> >> + gst_object_unref(convert0_);\n> >> + gst_object_unref(sink0_);\n> >> + }\n> > This is correct of course, but I think we could cleanup more the test.\n> With a\n> > run/cleanup() set of function, I would expect to be able to loop over\n> and run\n> > the test multiple times, but it does not seem to be the case due to the\n> fact the\n>\n>\n> Indeed, we can and should. I went in a rush to fix the leak, to see if\n> that's all we leak or more!\n>\n> +cc Vedant,\n>\n> Vedant, could you assume ownership of these patches/series and work upon\n> it? I won't have more time to allocate to this and it will generally\n> help you as well to fix the multi stream test on top of this as well and\n> make sure we don't have leaks anywhere.\n>\n> > pipeline is not fully constructed in run().\n> >\n> > For the leak being fixed:\n> > Reviewed-by: Nicolas Dufresne \n>\n>\n> Thanks for taking a look Nicolas.\n>\n> >\n> >> +\n> >> private:\n> >> GstElement *convert0_;\n> >> GstElement *sink0_;\n> >\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6BD83BDC71\n\tfor ;\n\tTue, 14 Sep 2021 17:57:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A379D60249;\n\tTue, 14 Sep 2021 19:57:14 +0200 (CEST)","from mail-yb1-xb2a.google.com (mail-yb1-xb2a.google.com\n\t[IPv6:2607:f8b0:4864:20::b2a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3C6AF60248\n\tfor ;\n\tTue, 14 Sep 2021 19:57:13 +0200 (CEST)","by mail-yb1-xb2a.google.com with SMTP id y13so30194028ybi.6\n\tfor ;\n\tTue, 14 Sep 2021 10:57:13 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"NXBruIP4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=l80+iQcUW6ZXynT9Fntbm1C73he7rzlQ9fc2eyjZfAU=;\n\tb=NXBruIP4YQOmGWnrzPM4PUqI6zC0vLhjG27DtawRqdK8/jssgC78s4PINVaVcWigJY\n\tXtMOqhthIK2yApQw9+Rksyasl7HqIV6NSccqY404RdrDaLWxk0IX8EQRA+95DYuDgR5m\n\tW7bXc+o2JEozv9E3iZJREhVAm96TzQ4aKGq/2KV03HMS9WiBfxRaL58i4pK3pqmG0tWW\n\tWHF9WTU/KGaqxdY+pQNvtL0LYGX0awAswhYEeH9nmwVWuwXoUIvZSKPfwcAz7IYsKvZP\n\t+4uWpRQ0zYhZ9o0vfFkl/DtPuMDq0VGpBGdnS9kqEpBoiF/PZeI3wFWFpZef/rHmNjA0\n\tOLJA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=l80+iQcUW6ZXynT9Fntbm1C73he7rzlQ9fc2eyjZfAU=;\n\tb=BxdRDb1R0V4ldtUteshMlvkJnueAW2g9mUnTsnZg/jQmQO6Q9R5OjZBXFnrpEhqEFQ\n\tqb0nRWkenuoSHCJukrvY0XugU/Q7mVR1Y5W0+RCmq2MByyjGpPYG8HU+6MOlwC7n1IFq\n\tcVjkaWCxv73GP1IjW1zzVBFnRQVVqaGO8oodQPZ5LpjnTIb71FmxDyNZOsdSZrJTWBOc\n\tyBshnd7T+Rhws/xsJZDmWPtsdyAMR3uNvrl1/60hqn8xMJq4tVjklKUOZMexOIQ6xVPl\n\tkpJuvfL+OPZL0IfF+yw768vhjIequGsWgdBHP8cVhrc4Y0qUEfobaMMxQWypB8uJM1eb\n\tgw4Q==","X-Gm-Message-State":"AOAM5300jn86DFLZkxAdUxOx6FAXHpJhJGtLsSgpzKZmL0jJey8dk7mt\n\tC/xG/FJUMB7g71dJRko4AmN+NLmlew+1oWDWpao=","X-Google-Smtp-Source":"ABdhPJy1dfST8tqRxRqxs5anENHjKFGvnOMLIMxrqz9hQUP1nBWdl9ivWLuuFT3mt7BKVJlCyHp4oPkOzxGw8uGvzx0=","X-Received":"by 2002:a25:b5ce:: with SMTP id d14mr477473ybg.415.1631642232091;\n\tTue, 14 Sep 2021 10:57:12 -0700 (PDT)","MIME-Version":"1.0","References":"<20210914121700.122591-1-umang.jain@ideasonboard.com>\n\t<20210914121700.122591-3-umang.jain@ideasonboard.com>\n\t<29bb4d09d1f96b3587912d41889739938fe8d332.camel@ndufresne.ca>\n\t","In-Reply-To":"","From":"Vedant Paranjape ","Date":"Tue, 14 Sep 2021 23:27:00 +0530","Message-ID":"","To":"Umang Jain ","Content-Type":"multipart/alternative; boundary=\"000000000000a4f70005cbf84fc2\"","Subject":"Re: [libcamera-devel] [PATCH 2/2] test:\n\tgstreamer_single_stream_test: Fix memory leak","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"LibCamera Devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]