[{"id":2479,"web_url":"https://patchwork.libcamera.org/comment/2479/","msgid":"<20190819092357.4sf3lxo3e57467hs@uno.localdomain>","date":"2019-08-19T09:23:57","subject":"Re: [libcamera-devel] [PATCH 12/14] android: camera_hal_manager:\n\tClean up resources when terminating","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Sun, Aug 18, 2019 at 04:13:27AM +0300, Laurent Pinchart wrote:\n> The CameraHalManager starts the libcamera CameraManager and creates\n> CameraProxy instances for each camera in the system. Clean up those\n> resources when the CameraHalManager terminates.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/android/camera_hal_manager.cpp | 4 ++++\n> 1 file changed, 4 insertions(+)\n>\n> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\n> index a1ffb3713d7e..cf981720bca4 100644\n> --- a/src/android/camera_hal_manager.cpp\n> +++ b/src/android/camera_hal_manager.cpp\n> @@ -90,6 +90,10 @@ void CameraHalManager::run()\n>\n> \t/* Now start processing events and messages. */\n> \texec();\n> +\n> +\t/* Clean up the resources we have allocated. */\n> +\tproxies_.clear();\n\nProxies are unique_ptr\n\tstd::vector> proxies_;\n\nand they are unique_ptr so that they should be deleted at\nCameraHalManager deletion time. However, the CameraHalManager instance\nis defined as a static one in camera3_hal.cpp and the destructor of\nthe static instance should be called at library teardown time. However\nI noticed with the help of valgrind the proxies memory did not get\nreleased properly (according to valgrind, which might fail to detect\ndeletion after the library as been unloaded). So if you recall, I\ntried to address this with __attribute((destructor)) withtout seeing\nany change.\n\nWith your change, which I think is good, I guess proxies_ can now be\nmade a vector of plain pointers, doesn't it?\n\nThanks\n j\n\n\n\n> +\tcameraManager_->stop();\n> }\n>\n> CameraProxy *CameraHalManager::open(unsigned int id,\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8D73860E38\n\tfor ;\n\tMon, 19 Aug 2019 11:22:29 +0200 (CEST)","from uno.localdomain (unknown [87.18.63.98])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id CFF6FFF806;\n\tMon, 19 Aug 2019 09:22:28 +0000 (UTC)"],"X-Originating-IP":"87.18.63.98","Date":"Mon, 19 Aug 2019 11:23:57 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190819092357.4sf3lxo3e57467hs@uno.localdomain>","References":"<20190818011329.14499-1-laurent.pinchart@ideasonboard.com>\n\t<20190818011329.14499-13-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"tpwavijfkmmxzzqc\"","Content-Disposition":"inline","In-Reply-To":"<20190818011329.14499-13-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 12/14] android: camera_hal_manager:\n\tClean up resources when terminating","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 19 Aug 2019 09:22:29 -0000"}},{"id":2491,"web_url":"https://patchwork.libcamera.org/comment/2491/","msgid":"<20190819151855.GL5011@pendragon.ideasonboard.com>","date":"2019-08-19T15:18:55","subject":"Re: [libcamera-devel] [PATCH 12/14] android: camera_hal_manager:\n\tClean up resources when terminating","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Aug 19, 2019 at 11:23:57AM +0200, Jacopo Mondi wrote:\n> On Sun, Aug 18, 2019 at 04:13:27AM +0300, Laurent Pinchart wrote:\n> > The CameraHalManager starts the libcamera CameraManager and creates\n> > CameraProxy instances for each camera in the system. Clean up those\n> > resources when the CameraHalManager terminates.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/android/camera_hal_manager.cpp | 4 ++++\n> > 1 file changed, 4 insertions(+)\n> >\n> > diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\n> > index a1ffb3713d7e..cf981720bca4 100644\n> > --- a/src/android/camera_hal_manager.cpp\n> > +++ b/src/android/camera_hal_manager.cpp\n> > @@ -90,6 +90,10 @@ void CameraHalManager::run()\n> >\n> > \t/* Now start processing events and messages. */\n> > \texec();\n> > +\n> > +\t/* Clean up the resources we have allocated. */\n> > +\tproxies_.clear();\n> \n> Proxies are unique_ptr\n> \tstd::vector> proxies_;\n> \n> and they are unique_ptr so that they should be deleted at\n> CameraHalManager deletion time. However, the CameraHalManager instance\n> is defined as a static one in camera3_hal.cpp and the destructor of\n> the static instance should be called at library teardown time. However\n> I noticed with the help of valgrind the proxies memory did not get\n> released properly (according to valgrind, which might fail to detect\n> deletion after the library as been unloaded). So if you recall, I\n> tried to address this with __attribute((destructor)) withtout seeing\n> any change.\n> \n> With your change, which I think is good, I guess proxies_ can now be\n> made a vector of plain pointers, doesn't it?\n\nclear() will delete all items in the vector. Deleting the\nstd::unique_ptr will thus delete the CameraProxy instances.\nIf we made them plain pointers, proxies_.clear() would clear the vector,\nbut not delete the objects pointed by the vector members. We thus need\neo keep unique_ptr.\n\n> > +\tcameraManager_->stop();\n> > }\n> >\n> > CameraProxy *CameraHalManager::open(unsigned int id,","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C479C60C1E\n\tfor ;\n\tMon, 19 Aug 2019 17:19:00 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 307BE510;\n\tMon, 19 Aug 2019 17:19:00 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1566227940;\n\tbh=cBwljQVrHDGekkSxvCZ/7YLXMRT3mUWe23TCq8vqAog=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZhaHVBaoGmP2Of5tjON9nbmJWYj+2eC9cOvtpLEEFGubOxiAPdKRvcd+ONMpI/e0n\n\tHfxrzBbgC8q6xVSvIlB/9A/dBn355KBdGNBZa9PGOFvTY02Kk/bShAig30CdbWtWTF\n\tP4aTXnEVYVEgHlmEZ3mdHedqwz1u5MC7MukZJ1uw=","Date":"Mon, 19 Aug 2019 18:18:55 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190819151855.GL5011@pendragon.ideasonboard.com>","References":"<20190818011329.14499-1-laurent.pinchart@ideasonboard.com>\n\t<20190818011329.14499-13-laurent.pinchart@ideasonboard.com>\n\t<20190819092357.4sf3lxo3e57467hs@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190819092357.4sf3lxo3e57467hs@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 12/14] android: camera_hal_manager:\n\tClean up resources when terminating","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 19 Aug 2019 15:19:00 -0000"}},{"id":2494,"web_url":"https://patchwork.libcamera.org/comment/2494/","msgid":"<20190819154041.5ej22pxwpiagisd7@uno.localdomain>","date":"2019-08-19T15:40:41","subject":"Re: [libcamera-devel] [PATCH 12/14] android: camera_hal_manager:\n\tClean up resources when terminating","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"HI Laurent,\n\nOn Mon, Aug 19, 2019 at 06:18:55PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Mon, Aug 19, 2019 at 11:23:57AM +0200, Jacopo Mondi wrote:\n> > On Sun, Aug 18, 2019 at 04:13:27AM +0300, Laurent Pinchart wrote:\n> > > The CameraHalManager starts the libcamera CameraManager and creates\n> > > CameraProxy instances for each camera in the system. Clean up those\n> > > resources when the CameraHalManager terminates.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/android/camera_hal_manager.cpp | 4 ++++\n> > > 1 file changed, 4 insertions(+)\n> > >\n> > > diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\n> > > index a1ffb3713d7e..cf981720bca4 100644\n> > > --- a/src/android/camera_hal_manager.cpp\n> > > +++ b/src/android/camera_hal_manager.cpp\n> > > @@ -90,6 +90,10 @@ void CameraHalManager::run()\n> > >\n> > > \t/* Now start processing events and messages. */\n> > > \texec();\n> > > +\n> > > +\t/* Clean up the resources we have allocated. */\n> > > +\tproxies_.clear();\n> >\n> > Proxies are unique_ptr\n> > \tstd::vector> proxies_;\n> >\n> > and they are unique_ptr so that they should be deleted at\n> > CameraHalManager deletion time. However, the CameraHalManager instance\n> > is defined as a static one in camera3_hal.cpp and the destructor of\n> > the static instance should be called at library teardown time. However\n> > I noticed with the help of valgrind the proxies memory did not get\n> > released properly (according to valgrind, which might fail to detect\n> > deletion after the library as been unloaded). So if you recall, I\n> > tried to address this with __attribute((destructor)) withtout seeing\n> > any change.\n> >\n> > With your change, which I think is good, I guess proxies_ can now be\n> > made a vector of plain pointers, doesn't it?\n>\n> clear() will delete all items in the vector. Deleting the\n> std::unique_ptr will thus delete the CameraProxy instances.\n> If we made them plain pointers, proxies_.clear() would clear the vector,\n> but not delete the objects pointed by the vector members. We thus need\n> eo keep unique_ptr.\n\nYeah, of course we would need to delete each single proxy if we make\nthem plain pointers. So is it worth keeping them as unique_ptr just to\nbe able to delete all of them with a single .clear ?\n\nTrue that the overhead is minimal, but I don't see a reason apart for\nthe above mentioned one to have them as unique_ptr\n\nThanks\n j\n\n>\n> > > +\tcameraManager_->stop();\n> > > }\n> > >\n> > > CameraProxy *CameraHalManager::open(unsigned int id,\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 31F5160C1E\n\tfor ;\n\tMon, 19 Aug 2019 17:39:14 +0200 (CEST)","from uno.localdomain (unknown [87.18.63.98])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id 7775424000C;\n\tMon, 19 Aug 2019 15:39:13 +0000 (UTC)"],"Date":"Mon, 19 Aug 2019 17:40:41 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190819154041.5ej22pxwpiagisd7@uno.localdomain>","References":"<20190818011329.14499-1-laurent.pinchart@ideasonboard.com>\n\t<20190818011329.14499-13-laurent.pinchart@ideasonboard.com>\n\t<20190819092357.4sf3lxo3e57467hs@uno.localdomain>\n\t<20190819151855.GL5011@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"4z3uybjyhb3px6yr\"","Content-Disposition":"inline","In-Reply-To":"<20190819151855.GL5011@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 12/14] android: camera_hal_manager:\n\tClean up resources when terminating","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 19 Aug 2019 15:39:14 -0000"}},{"id":2495,"web_url":"https://patchwork.libcamera.org/comment/2495/","msgid":"<20190819154904.GO5011@pendragon.ideasonboard.com>","date":"2019-08-19T15:49:04","subject":"Re: [libcamera-devel] [PATCH 12/14] android: camera_hal_manager:\n\tClean up resources when terminating","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Aug 19, 2019 at 05:40:41PM +0200, Jacopo Mondi wrote:\n> On Mon, Aug 19, 2019 at 06:18:55PM +0300, Laurent Pinchart wrote:\n> > On Mon, Aug 19, 2019 at 11:23:57AM +0200, Jacopo Mondi wrote:\n> >> On Sun, Aug 18, 2019 at 04:13:27AM +0300, Laurent Pinchart wrote:\n> >>> The CameraHalManager starts the libcamera CameraManager and creates\n> >>> CameraProxy instances for each camera in the system. Clean up those\n> >>> resources when the CameraHalManager terminates.\n> >>>\n> >>> Signed-off-by: Laurent Pinchart \n> >>> ---\n> >>> src/android/camera_hal_manager.cpp | 4 ++++\n> >>> 1 file changed, 4 insertions(+)\n> >>>\n> >>> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\n> >>> index a1ffb3713d7e..cf981720bca4 100644\n> >>> --- a/src/android/camera_hal_manager.cpp\n> >>> +++ b/src/android/camera_hal_manager.cpp\n> >>> @@ -90,6 +90,10 @@ void CameraHalManager::run()\n> >>>\n> >>> \t/* Now start processing events and messages. */\n> >>> \texec();\n> >>> +\n> >>> +\t/* Clean up the resources we have allocated. */\n> >>> +\tproxies_.clear();\n> >>\n> >> Proxies are unique_ptr\n> >> \tstd::vector> proxies_;\n> >>\n> >> and they are unique_ptr so that they should be deleted at\n> >> CameraHalManager deletion time. However, the CameraHalManager instance\n> >> is defined as a static one in camera3_hal.cpp and the destructor of\n> >> the static instance should be called at library teardown time. However\n> >> I noticed with the help of valgrind the proxies memory did not get\n> >> released properly (according to valgrind, which might fail to detect\n> >> deletion after the library as been unloaded). So if you recall, I\n> >> tried to address this with __attribute((destructor)) withtout seeing\n> >> any change.\n> >>\n> >> With your change, which I think is good, I guess proxies_ can now be\n> >> made a vector of plain pointers, doesn't it?\n> >\n> > clear() will delete all items in the vector. Deleting the\n> > std::unique_ptr will thus delete the CameraProxy instances.\n> > If we made them plain pointers, proxies_.clear() would clear the vector,\n> > but not delete the objects pointed by the vector members. We thus need\n> > eo keep unique_ptr.\n> \n> Yeah, of course we would need to delete each single proxy if we make\n> them plain pointers. So is it worth keeping them as unique_ptr just to\n> be able to delete all of them with a single .clear ?\n> \n> True that the overhead is minimal, but I don't see a reason apart for\n> the above mentioned one to have them as unique_ptr\n\nYes, that's the whole point, std::vector> is a\nself-deleting vector of T* :-) With std::vector I would\nhave to write\n\n\tfor (CameraProxy *proxy : proxies_)\n\t\tdelete proxy;\n\tproxies_.clear();\n\nand make sure not to forget any path where the vector is cleared (either\nexplicitly like here, or implicitly in the destructor).\n\nDo you think that would be better ?\n\n> >>> +\tcameraManager_->stop();\n> >>> }\n> >>>\n> >>> CameraProxy *CameraHalManager::open(unsigned int id,","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8338A60C1E\n\tfor ;\n\tMon, 19 Aug 2019 17:49:10 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 06F19510;\n\tMon, 19 Aug 2019 17:49:09 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1566229750;\n\tbh=ulcdnSNsZI7hZxOZmsOHXdXuJZfy4fgAQR6dketOq4U=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CWtbwbpxZPzKiDEKtXi9WCoTYlp88gypB2ngDms9Tlma2snE+ATK1zYFGjOIylkn+\n\tIHRXU4pr4VGWIb6Nyh5PQYeh0VqD8z9XA6q48/jrvt0ZZ3Set09n7EWlwzC0jtMIGC\n\tUN6w3VKJ8SKpy+Ql1ekDcrtUsOr1ex5hyzld8d3A=","Date":"Mon, 19 Aug 2019 18:49:04 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190819154904.GO5011@pendragon.ideasonboard.com>","References":"<20190818011329.14499-1-laurent.pinchart@ideasonboard.com>\n\t<20190818011329.14499-13-laurent.pinchart@ideasonboard.com>\n\t<20190819092357.4sf3lxo3e57467hs@uno.localdomain>\n\t<20190819151855.GL5011@pendragon.ideasonboard.com>\n\t<20190819154041.5ej22pxwpiagisd7@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190819154041.5ej22pxwpiagisd7@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 12/14] android: camera_hal_manager:\n\tClean up resources when terminating","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 19 Aug 2019 15:49:10 -0000"}},{"id":2499,"web_url":"https://patchwork.libcamera.org/comment/2499/","msgid":"<20190819160605.bk3xgbjrsfpvuwbn@uno.localdomain>","date":"2019-08-19T16:06:05","subject":"Re: [libcamera-devel] [PATCH 12/14] android: camera_hal_manager:\n\tClean up resources when terminating","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Aug 19, 2019 at 06:49:04PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Mon, Aug 19, 2019 at 05:40:41PM +0200, Jacopo Mondi wrote:\n> > On Mon, Aug 19, 2019 at 06:18:55PM +0300, Laurent Pinchart wrote:\n> > > On Mon, Aug 19, 2019 at 11:23:57AM +0200, Jacopo Mondi wrote:\n> > >> On Sun, Aug 18, 2019 at 04:13:27AM +0300, Laurent Pinchart wrote:\n> > >>> The CameraHalManager starts the libcamera CameraManager and creates\n> > >>> CameraProxy instances for each camera in the system. Clean up those\n> > >>> resources when the CameraHalManager terminates.\n> > >>>\n> > >>> Signed-off-by: Laurent Pinchart \n> > >>> ---\n> > >>> src/android/camera_hal_manager.cpp | 4 ++++\n> > >>> 1 file changed, 4 insertions(+)\n> > >>>\n> > >>> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\n> > >>> index a1ffb3713d7e..cf981720bca4 100644\n> > >>> --- a/src/android/camera_hal_manager.cpp\n> > >>> +++ b/src/android/camera_hal_manager.cpp\n> > >>> @@ -90,6 +90,10 @@ void CameraHalManager::run()\n> > >>>\n> > >>> \t/* Now start processing events and messages. */\n> > >>> \texec();\n> > >>> +\n> > >>> +\t/* Clean up the resources we have allocated. */\n> > >>> +\tproxies_.clear();\n> > >>\n> > >> Proxies are unique_ptr\n> > >> \tstd::vector> proxies_;\n> > >>\n> > >> and they are unique_ptr so that they should be deleted at\n> > >> CameraHalManager deletion time. However, the CameraHalManager instance\n> > >> is defined as a static one in camera3_hal.cpp and the destructor of\n> > >> the static instance should be called at library teardown time. However\n> > >> I noticed with the help of valgrind the proxies memory did not get\n> > >> released properly (according to valgrind, which might fail to detect\n> > >> deletion after the library as been unloaded). So if you recall, I\n> > >> tried to address this with __attribute((destructor)) withtout seeing\n> > >> any change.\n> > >>\n> > >> With your change, which I think is good, I guess proxies_ can now be\n> > >> made a vector of plain pointers, doesn't it?\n> > >\n> > > clear() will delete all items in the vector. Deleting the\n> > > std::unique_ptr will thus delete the CameraProxy instances.\n> > > If we made them plain pointers, proxies_.clear() would clear the vector,\n> > > but not delete the objects pointed by the vector members. We thus need\n> > > eo keep unique_ptr.\n> >\n> > Yeah, of course we would need to delete each single proxy if we make\n> > them plain pointers. So is it worth keeping them as unique_ptr just to\n> > be able to delete all of them with a single .clear ?\n> >\n> > True that the overhead is minimal, but I don't see a reason apart for\n> > the above mentioned one to have them as unique_ptr\n>\n> Yes, that's the whole point, std::vector> is a\n> self-deleting vector of T* :-) With std::vector I would\n> have to write\n>\n> \tfor (CameraProxy *proxy : proxies_)\n> \t\tdelete proxy;\n> \tproxies_.clear();\n>\n> and make sure not to forget any path where the vector is cleared (either\n> explicitly like here, or implicitly in the destructor).\n>\n> Do you think that would be better ?\n>\n\nLet's say that if I had to write it from scratch I would not use\nunique_ptr, but they do not harm if you want to keep them there, no\nissues...\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> > >>> +\tcameraManager_->stop();\n> > >>> }\n> > >>>\n> > >>> CameraProxy *CameraHalManager::open(unsigned int id,\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E72AB60C1E\n\tfor ;\n\tMon, 19 Aug 2019 18:04:37 +0200 (CEST)","from uno.localdomain (unknown [87.18.63.98])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 4FAD6C000F;\n\tMon, 19 Aug 2019 16:04:37 +0000 (UTC)"],"X-Originating-IP":"87.18.63.98","Date":"Mon, 19 Aug 2019 18:06:05 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190819160605.bk3xgbjrsfpvuwbn@uno.localdomain>","References":"<20190818011329.14499-1-laurent.pinchart@ideasonboard.com>\n\t<20190818011329.14499-13-laurent.pinchart@ideasonboard.com>\n\t<20190819092357.4sf3lxo3e57467hs@uno.localdomain>\n\t<20190819151855.GL5011@pendragon.ideasonboard.com>\n\t<20190819154041.5ej22pxwpiagisd7@uno.localdomain>\n\t<20190819154904.GO5011@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"nkqyg3i4zb2heup7\"","Content-Disposition":"inline","In-Reply-To":"<20190819154904.GO5011@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 12/14] android: camera_hal_manager:\n\tClean up resources when terminating","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 19 Aug 2019 16:04:38 -0000"}}]