[{"id":25356,"web_url":"https://patchwork.libcamera.org/comment/25356/","msgid":"","date":"2022-10-08T17:34:43","subject":"Re: [libcamera-devel] [PATCH] libcamera: Release all devices on\n\tshutdown","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Christian\n\nOn 10/8/22 4:45 PM, Christian Rauch via libcamera-devel wrote:\n> Some devices may not be released via their 'PipelineHandler'. This results\n\nCan you explain better which are those devices / platform for which \nPipelineHandler does not release the devices? Do you have any particular \ncase in mind?\n\nIt's the job of the pipeline handler to correctly release the device, \nsince it's the one acquired it in the first place.\n> in these devices still being acquired or busy on shutdown. Make sure that\n> all devices are released when DeviceEnumerator is deconstructed.\n>\n> Signed-off-by: Christian Rauch \n> ---\n> src/libcamera/device_enumerator.cpp | 8 ++------\n> 1 file changed, 2 insertions(+), 6 deletions(-)\n>\n> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp\n> index f2e055de..d0968a0a 100644\n> --- a/src/libcamera/device_enumerator.cpp\n> +++ b/src/libcamera/device_enumerator.cpp\n> @@ -161,12 +161,8 @@ std::unique_ptr DeviceEnumerator::create()\n>\n> DeviceEnumerator::~DeviceEnumerator()\n> {\n> -\tfor (const std::shared_ptr &media : devices_) {\n> -\t\tif (media->busy())\n> -\t\t\tLOG(DeviceEnumerator, Error)\n> -\t\t\t\t<< \"Removing media device \" << media->deviceNode()\n> -\t\t\t\t<< \" while still in use\";\n> -\t}\n> +\tfor (const std::shared_ptr &media : devices_)\n> +\t\tmedia->release();\n\nI believe this shadows  a bug and forcibly releases a device here, which \nshould ideally be released by the pipeline handler in the first place. \nThe Error log is present there to catch such instances I think!\n> }\n>\n> /**\n> --\n> 2.34.1\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 30729C0DA4\n\tfor ;\n\tSat, 8 Oct 2022 12:06:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A378C62D48;\n\tSat, 8 Oct 2022 14:06:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5F7BE60A83\n\tfor ;\n\tSat, 8 Oct 2022 14:06:38 +0200 (CEST)","from [192.168.1.103] (unknown [103.251.226.3])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 08E854E6;\n\tSat, 8 Oct 2022 14:06:36 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1665230799;\n\tbh=WRmPtyUCuoyC8LSSGHkGHln39vx7Noy39aEYf8ljeR8=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=gS9u4PvN0NlIR9Ib55bk3XDOjMzga/kKD5lTUjTpMjsACy8xcZIjOa3BmayjVeidX\n\tp2o0YiyGPgIOvjz0AqymCtqZx5dDKwH0NJXKIKwRiFUKFsSZaZfK56rIOdroX2kCU8\n\tBkhgmDSFgHYr8YAFm151duUzJ5a1W+bqwX+Bv9iBtf9ma42X7USxILd3dpa4zU1KSi\n\t/kNdoWbTk75XpLkTX/QS5aSFLUpvK3qO4JLA9d76dYtCLL7b1k79HuQyK6qt8Qa8AJ\n\t1qOFFI/Nupe3RMdDCR9jxFCPmIrwGRwcxgVNZHPGrwQIYHiMb4A1rMMI/vFXtnsspU\n\tDfTDN4bdpYzcg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1665230797;\n\tbh=WRmPtyUCuoyC8LSSGHkGHln39vx7Noy39aEYf8ljeR8=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=rtKrYnnxRR2MIl95GtBqmyWh5wtuBiPw/ackxClKfwhaCw8bb09Ez9VsJGJiNikDU\n\twmdpUHLREuSRp7zi0jqM6B/avtK9TlIg7ebWx0objZnt14FJAbimMzZmoFIX59gv0e\n\tgwFV7iTvbbNDVUzbIXF8e1u65jFbr69xLYVkWTP4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rtKrYnnx\"; dkim-atps=neutral","Message-ID":"","Date":"Sat, 8 Oct 2022 23:04:43 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.2.1","To":"Christian Rauch ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20221008111500.40661-1-Rauch.Christian@gmx.de>","Content-Language":"en-US","In-Reply-To":"<20221008111500.40661-1-Rauch.Christian@gmx.de>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: Release all devices on\n\tshutdown","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25358,"web_url":"https://patchwork.libcamera.org/comment/25358/","msgid":"<166523473715.3760285.9598912606299529172@Monstersaurus>","date":"2022-10-08T13:12:17","subject":"Re: [libcamera-devel] [PATCH] libcamera: Release all devices on\n\tshutdown","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain via libcamera-devel (2022-10-08 18:34:43)\n> Hi Christian\n> \n> On 10/8/22 4:45 PM, Christian Rauch via libcamera-devel wrote:\n> > Some devices may not be released via their 'PipelineHandler'. This results\n> \n> Can you explain better which are those devices / platform for which \n> PipelineHandler does not release the devices? Do you have any particular \n> case in mind?\n> \n> It's the job of the pipeline handler to correctly release the device, \n> since it's the one acquired it in the first place.\n> > in these devices still being acquired or busy on shutdown. Make sure that\n> > all devices are released when DeviceEnumerator is deconstructed.\n> >\n> > Signed-off-by: Christian Rauch \n> > ---\n> > src/libcamera/device_enumerator.cpp | 8 ++------\n> > 1 file changed, 2 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp\n> > index f2e055de..d0968a0a 100644\n> > --- a/src/libcamera/device_enumerator.cpp\n> > +++ b/src/libcamera/device_enumerator.cpp\n> > @@ -161,12 +161,8 @@ std::unique_ptr DeviceEnumerator::create()\n> >\n> > DeviceEnumerator::~DeviceEnumerator()\n> > {\n> > - for (const std::shared_ptr &media : devices_) {\n> > - if (media->busy())\n> > - LOG(DeviceEnumerator, Error)\n> > - << \"Removing media device \" << media->deviceNode()\n> > - << \" while still in use\";\n> > - }\n> > + for (const std::shared_ptr &media : devices_)\n> > + media->release();\n> \n> I believe this shadows  a bug and forcibly releases a device here, which \n> should ideally be released by the pipeline handler in the first place. \n> The Error log is present there to catch such instances I think!\n\nThis is my worry too. I wonder if we can record who/where the media\ndevice is opened, and report that here?\n\nIf this releases the media device, does that leave the pipeline handler\nstill accessing it? (thus leading on to a new bug / use after free)\n\n--\nKieran\n\n\n> > }\n> >\n> > /**\n> > --\n> > 2.34.1\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 18DFDBD16B\n\tfor ;\n\tSat, 8 Oct 2022 13:12:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6077C62D4A;\n\tSat, 8 Oct 2022 15:12:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E50EA60A83\n\tfor ;\n\tSat, 8 Oct 2022 15:12:19 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3EF714E6;\n\tSat, 8 Oct 2022 15:12:19 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1665234741;\n\tbh=7hsZGoQrqkwThwVRglzRPK6dWk8Czsd+FsecgwSKui8=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=niq7oSfURCndQJWj+/IhlIGSXfLwAH6HzQz/dDh+hWWN6WGrAMT5gHHdUr/z+//aR\n\tC5hh8SQ90BdIyqpvLc47vdUqWHI+mR8oL021syYSBnInD7COQlwP8UZ0xLlGAcT3C1\n\t9KYPt0E1Se7nrAl+x4BF9f5SpmULvnseMAFyVJG1CRomCdX7880rw9BnEnffEWrEO4\n\tSuD/Mqn7l9jSru7cqDFeo2IN+63I+/9uQnghidBtUki+7PmO7FmTA8LxoDohKR7FGB\n\t600I8daEPUJd+xEARkn1SmhAnEmd+86mcXl42sEqppyFArxn8roQt2se52s7DWH5aM\n\t3xsCWGnNpx1Lw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1665234739;\n\tbh=7hsZGoQrqkwThwVRglzRPK6dWk8Czsd+FsecgwSKui8=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=OQC22Cx21CWpsIArmKkqdulTq+G8BQVL+kkUJQTaGZwm0TUYxBj5SaXMaG+ptUvIv\n\tUvB/7vAZ/EMDNa4D2ZPXxoku7HyGOw6c5WVHIAKtASJ6r1Svv8yE0qr6/Ydkhl0M2g\n\td1h+d67NgySPK6LdvrW3KrYFlmWsl8HT1za36qLo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"OQC22Cx2\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20221008111500.40661-1-Rauch.Christian@gmx.de>\n\t","To":"Christian Rauch ,\n\tUmang Jain ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Sat, 08 Oct 2022 14:12:17 +0100","Message-ID":"<166523473715.3760285.9598912606299529172@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] libcamera: Release all devices on\n\tshutdown","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25360,"web_url":"https://patchwork.libcamera.org/comment/25360/","msgid":"","date":"2022-10-09T10:14:14","subject":"Re: [libcamera-devel] [PATCH] libcamera: Release all devices on\n\tshutdown","submitter":{"id":111,"url":"https://patchwork.libcamera.org/api/people/111/","name":"Christian Rauch","email":"Rauch.Christian@gmx.de"},"content":"Hi,\n\nFor some reason, I did not receive the original response.\n\nAm 08.10.22 um 15:12 schrieb Kieran Bingham:\n> Quoting Umang Jain via libcamera-devel (2022-10-08 18:34:43)\n>> Hi Christian\n>>\n>> On 10/8/22 4:45 PM, Christian Rauch via libcamera-devel wrote:\n>>> Some devices may not be released via their 'PipelineHandler'. This results\n>>\n>> Can you explain better which are those devices / platform for which\n>> PipelineHandler does not release the devices? Do you have any particular\n>> case in mind?\n\nSpecifically, it's https://bugs.libcamera.org/show_bug.cgi?id=155.\n\n>>\n>> It's the job of the pipeline handler to correctly release the device,\n>> since it's the one acquired it in the first place.\n\nThe PipelineHandler does release the device when it is deconstructed,\nbut the issue is here that not all PipelineHandler are deconstructed.\nHence, devices are left acquired.\n\n>>> in these devices still being acquired or busy on shutdown. Make sure that\n>>> all devices are released when DeviceEnumerator is deconstructed.\n>>>\n>>> Signed-off-by: Christian Rauch \n>>> ---\n>>> src/libcamera/device_enumerator.cpp | 8 ++------\n>>> 1 file changed, 2 insertions(+), 6 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp\n>>> index f2e055de..d0968a0a 100644\n>>> --- a/src/libcamera/device_enumerator.cpp\n>>> +++ b/src/libcamera/device_enumerator.cpp\n>>> @@ -161,12 +161,8 @@ std::unique_ptr DeviceEnumerator::create()\n>>>\n>>> DeviceEnumerator::~DeviceEnumerator()\n>>> {\n>>> - for (const std::shared_ptr &media : devices_) {\n>>> - if (media->busy())\n>>> - LOG(DeviceEnumerator, Error)\n>>> - << \"Removing media device \" << media->deviceNode()\n>>> - << \" while still in use\";\n>>> - }\n>>> + for (const std::shared_ptr &media : devices_)\n>>> + media->release();\n>>\n>> I believe this shadows  a bug and forcibly releases a device here, which\n>> should ideally be released by the pipeline handler in the first place.\n>> The Error log is present there to catch such instances I think!\n>\n> This is my worry too. I wonder if we can record who/where the media\n> device is opened, and report that here?\n>\n> If this releases the media device, does that leave the pipeline handler\n> still accessing it? (thus leading on to a new bug / use after free)\n\nAre PipelineHandler expected to still access devices after the\nDeviceEnumerator has been deconstructed?\n\n>\n> --\n> Kieran\n>\n>\n>>> }\n>>>\n>>> /**\n>>> --\n>>> 2.34.1\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 65EC5C0DA4\n\tfor ;\n\tSun, 9 Oct 2022 10:14:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD99662D4F;\n\tSun, 9 Oct 2022 12:14:17 +0200 (CEST)","from mout.gmx.net (mout.gmx.net [212.227.17.20])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D302960AC2\n\tfor ;\n\tSun, 9 Oct 2022 12:14:15 +0200 (CEST)","from [192.168.0.158] ([88.152.184.103]) by mail.gmx.net (mrgmx105\n\t[212.227.17.168]) with ESMTPSA (Nemesis) id\n\t1MHXBp-1ouxq214PR-00DTqT for\n\t; Sun, 09 Oct 2022 12:14:15 +0200"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1665310457;\n\tbh=qXBJzYmPyNLbmFNoW4yJR7wq/W9oFQ/FEJTkYP5wyxc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=d1XL2gwMxuOL3EXNPoZ89JMJ/gbfImxDDelpVXSo0RumeGj2f4O+jN0dwD9L8ig/A\n\tvuyw6leYc5Xsz9S7taolDPlCWJC3JPuttp0MWPjS9WsiZUVQZC0iIgIGFc9nRvi5Wd\n\tQN4r0/1qglvpwS/yEMi5064Sd1zK0HVQi5bqurqNCc+K3+tWIgyU1inetTdKVV3KXj\n\tHVOSRi114gbMN39hGVar/RybA40jxMTNls5CSxiycd9oJYRbR4OV40KznlFnfyDXze\n\tjNN4i6DuYcg3kFVZm/qo948C1ybdcacTIz2rNMu+B8zoXCkNWhDMkyTJG1cUL3kdvv\n\tJ2NVfidajSw6Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net;\n\ts=badeba3b8450; t=1665310455;\n\tbh=qXBJzYmPyNLbmFNoW4yJR7wq/W9oFQ/FEJTkYP5wyxc=;\n\th=X-UI-Sender-Class:Date:Subject:To:References:From:In-Reply-To;\n\tb=VFR+mBgnAwEjQv5SXLM/BbnZtAYBtLMLro+IRML/TIpvDI1DREQj40tyhkWeieA+c\n\tyfoNJ5SvQZP/TUQqGYjznbcNHssGEzZfmaL02+9UlLg+wbfXSa3p309cJjuh2XKcQG\n\tfymjV9SRNHoYyGHuNG1ZQ54raSyny+IG+pHhs5Z8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=gmx.net header.i=@gmx.net\n\theader.b=\"VFR+mBgn\"; dkim-atps=neutral","X-UI-Sender-Class":"01bb95c1-4bf8-414a-932a-4f6e2808ef9c","Message-ID":"","Date":"Sun, 9 Oct 2022 12:14:14 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.2.2","Content-Language":"en-GB","To":"libcamera-devel@lists.libcamera.org","References":"<20221008111500.40661-1-Rauch.Christian@gmx.de>\n\t\n\t<166523473715.3760285.9598912606299529172@Monstersaurus>","In-Reply-To":"<166523473715.3760285.9598912606299529172@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"quoted-printable","X-Provags-ID":"V03:K1:7XLZB32kk9aYpEjdNUYTaSM1LaBw/cIOOA3tWSOAZiG2t9JGutV\n\taFjQDtoQ8KWs7HsRxM0KQjzG12QzNaDBZhvGNWOIurk6rlBGUrQc7CBCii76G4hwd8obbDK\n\tVJoxSot8vscS7p0MRR5mO2KejZHiYQr+F4Gum6vgQ/J5Gpj8hSTzYE8wsi3bcUYWoeJ8Or7\n\tZFWj7O6B5GPrV/8y07/Cw==","X-Spam-Flag":"NO","X-UI-Out-Filterresults":"notjunk:1; V03:K0:C8btxnie1J4=:go2qqoxIFAISCLtuQb+FvZ\n\tBGeeCsebyVG+01Vh59vToKD62diCribvLtqR65zsgDTJMdZh2cxcHmQJDgOnyplFYcMAy6H+L\n\tjcnD2vtLDc/gJnvNi/akA6yCCQTP7MM0aK8lV5SF0GPwr17PMRPYC4tGRf5TVd66333pVENR9\n\t0MZ2OYbvA8lFOJTNcUTnn0oKYWig3RJfEvnQu4I/aJ9QDP7Nc4ujUhRo84Gy/LoHni8qziXs1\n\tl3UNrYDGmtRnkWMpaE7XoMufrePrvteSyEWal6BvWcxUt4WIsu9fr3n1VNAkdE6PJqdQjTi6F\n\t4AndEgRqKKMaRS3Xss9Fn4PjJVLcDkjgn6vuk+j4NupIQrTfZxdXPqxm1MDDozuqki8yU1c14\n\tJHKbiJrTHGyu14/wvNG1gBQ3LzG83V2fYuDX7b3Xq9lglfBAKd6TxW3drMGamXeBx8BtPd0u9\n\tMk8oBdiZ8pzZd3X90cVp/LkXFiqUe21xmjUN+oCy28XYwNWGdVgbUI8tbio5foHf73DpzRe5q\n\tXLvfC0sghBQY2zuNiZmFs3UnjwB/WkHbNSXgFAPi22pAq0V/lVc8hQ7MCqLmEu1NKB2OvWgbg\n\t2izKfZZAXotxfPDYEWksdISECEjUHehl1Tg52xaUhYxXPptMNrzZnsc7Xkm3IYUycZzyiEhhe\n\tt5UW1QGl/0QIbTF8oIaH1ZmwkvTJ60BRd2RhCZKRSIbrFfw7BfyB/sL/zZOodmOTeZisVr7IH\n\tbaoCfLLt9kHxEFogcXcd0N4AN7Qj36oCcp1+KSst1CujI9rO6vbgenQM48uLeBqR1SgeugRdu\n\tV6rR/SsjzBW2jGJSjsQ/iWWALua/X2vmaSy6vfZbhaMnPnCCazUGwju3ceygSezhx5ywWHgbx\n\t5U04UCwoFe7AeiW8hCToPWMBFHYf1e227DKVmIZ4cRhe0sBDITPrbEDgIodaBHDo/KN8aGhUL\n\tVumaKPVnvTqdfhxssqQfIaGkh96ha0lDIgwDL7b1EYC6MkCzCA2vYZXKxhheXs6TBuw/Rhw0r\n\tLtEZ6Yzsd+/ldXaDpS8z5YH2CgmonPK+6lIdr8lmr3D+2Ap0NAhHxWMUQg6pKzwUqwzOI2umk\n\tvUl8b2ZMUGeZfqF/6no/VI/guxClRp1ocNpmm2OIJsvSQMKMFGxGC49PsL5eMBE0TfM+EgWn1\n\tmZA4kUkXeSj51M0h8U8aKtnNvvfvfigErkrhyEIAJbJ9SPRsckps553xfsTe6YfI+CfE5q/8X\n\td0D2F8kNu36rhWXncqPXdHoNEjDxiU2vMHyHDYk81T2QNnqEZwZeJYPymauT1Atrdid8zbpw3\n\tRygkgQgM93uZBFUh9nnEojnrIxCHot4+rjpDTCtjqAifc4X2D0BU5nAg7qYQVXCRcJstIxc8e\n\tf8642N2YOYCTlQdVkPF5sDZKeA+eMJGQAxcKBIWpkZM5wd8ghgPiB5flV0iFTgKn1HiFuLoxu\n\tHJB7tycRiNhnM7DXq1ERUPPkdhElCw0UkyLCPpQaNAtd+y9/bcUJisBDddRKBra6Psf/9/HY9\n\ti/oiNmL7zHeEIq1d+KhyjMzxYctg/dQS/oaxMG1pFeF8U","Subject":"Re: [libcamera-devel] [PATCH] libcamera: Release all devices on\n\tshutdown","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","From":"Christian Rauch via libcamera-devel\n\t","Reply-To":"Christian Rauch ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25362,"web_url":"https://patchwork.libcamera.org/comment/25362/","msgid":"<67f28222-e4a1-657a-141a-977bcbebd506@ideasonboard.com>","date":"2022-10-10T11:33:46","subject":"Re: [libcamera-devel] [PATCH] libcamera: Release all devices on\n\tshutdown","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Christian,\n\nOn 10/9/22 3:44 PM, Christian Rauch via libcamera-devel wrote:\n> Hi,\n>\n> For some reason, I did not receive the original response.\n>\n> Am 08.10.22 um 15:12 schrieb Kieran Bingham:\n>> Quoting Umang Jain via libcamera-devel (2022-10-08 18:34:43)\n>>> Hi Christian\n>>>\n>>> On 10/8/22 4:45 PM, Christian Rauch via libcamera-devel wrote:\n>>>> Some devices may not be released via their 'PipelineHandler'. This \n>>>> results\n>>>\n>>> Can you explain better which are those devices / platform for which\n>>> PipelineHandler does not release the devices? Do you have any \n>>> particular\n>>> case in mind?\n>\n> Specifically, it's https://bugs.libcamera.org/show_bug.cgi?id=155.\n\nThe bug doesn't tell which platform is used (sensor + pipeline-handler). \nCan you provide addiitional information on the bug itself\n\nIt would be good to inspect further using the debug logs.\n\n       ($) LIBCAMERA_LOG_LEVELS=0  cam -c1 -C\n\nwould be first step to know what's going on...\n>\n>>>\n>>> It's the job of the pipeline handler to correctly release the device,\n>>> since it's the one acquired it in the first place.\n>\n> The PipelineHandler does release the device when it is deconstructed,\n> but the issue is here that not all PipelineHandler are deconstructed.\n\nWe need instances of such use-cases (if they are happening in the wild)\n\nI think DeviceEnumerator getting  destroyed without getting \nPipelineHandler destroy - is quite ab-normal to me.\n\n\n> Hence, devices are left acquired.\n\nMaybe what your are experiencing is the crash before cleanup / destroy \nfunctions are invoked. Hence you see media devices are released etc stuff.\n\nBut again, only can be ensured once we have additional information\n>\n>>>> in these devices still being acquired or busy on shutdown. Make \n>>>> sure that\n>>>> all devices are released when DeviceEnumerator is deconstructed.\n>>>>\n>>>> Signed-off-by: Christian Rauch \n>>>> ---\n>>>>    src/libcamera/device_enumerator.cpp | 8 ++------\n>>>>    1 file changed, 2 insertions(+), 6 deletions(-)\n>>>>\n>>>> diff --git a/src/libcamera/device_enumerator.cpp \n>>>> b/src/libcamera/device_enumerator.cpp\n>>>> index f2e055de..d0968a0a 100644\n>>>> --- a/src/libcamera/device_enumerator.cpp\n>>>> +++ b/src/libcamera/device_enumerator.cpp\n>>>> @@ -161,12 +161,8 @@ std::unique_ptr \n>>>> DeviceEnumerator::create()\n>>>>\n>>>>    DeviceEnumerator::~DeviceEnumerator()\n>>>>    {\n>>>> -     for (const std::shared_ptr &media : devices_) {\n>>>> -             if (media->busy())\n>>>> -                     LOG(DeviceEnumerator, Error)\n>>>> -                             << \"Removing media device \" << \n>>>> media->deviceNode()\n>>>> -                             << \" while still in use\";\n>>>> -     }\n>>>> +     for (const std::shared_ptr &media : devices_)\n>>>> +             media->release();\n>>>\n>>> I believe this shadows  a bug and forcibly releases a device here, \n>>> which\n>>> should ideally be released by the pipeline handler in the first place.\n>>> The Error log is present there to catch such instances I think!\n>>\n>> This is my worry too. I wonder if we can record who/where the media\n>> device is opened, and report that here?\n>>\n>> If this releases the media device, does that leave the pipeline handler\n>> still accessing it? (thus leading on to a new bug / use after free)\n>\n> Are PipelineHandler expected to still access devices after the\n> DeviceEnumerator has been deconstructed?\n>\n>>\n>> -- \n>> Kieran\n>>\n>>\n>>>>    }\n>>>>\n>>>>    /**\n>>>> -- \n>>>> 2.34.1\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 10A54BD16B\n\tfor ;\n\tMon, 10 Oct 2022 06:04:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F2AB62D51;\n\tMon, 10 Oct 2022 08:04:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 013B161F67\n\tfor ;\n\tMon, 10 Oct 2022 08:04:37 +0200 (CEST)","from [192.168.1.103] (unknown [103.86.18.148])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2CC034DB;\n\tMon, 10 Oct 2022 08:04:35 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1665381880;\n\tbh=8tyIX0qwXQknHxrXNHjS9C9tYN81Gi5zhr37v7WdSqI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=2ZConZIicYaz6ZSyFcnij5XcCEsOfBU+wEmrMSfM6bprNyHAP2VSiqRdB1VwNltFn\n\tLKKsPyCiWDuq0/QYAwLBWz+PvP4PLAu/X6P11HDTOvo5v3EsSCJ3tbKf7phNoL62XP\n\tutm8FllgSVyXwHOUiduu927kdIOjNBdkh0ESJDhwp4b0FWP1VKjwFLo0Wxe0PB+ges\n\tF79zgVvwhGlA40yKAou2R19nNg/K9VC+rREXE/KHlb9MX15FNjy09u/HIqlYc8u04r\n\ttd6FDmH7l+8anyfSl0wdAczirN6IE43xfs1aNx+NT6ysNiHWrBzXCFbWJGWqk+VEl9\n\tfPiHI0Y69PaCw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1665381877;\n\tbh=8tyIX0qwXQknHxrXNHjS9C9tYN81Gi5zhr37v7WdSqI=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=k67cC1RjYGlVh2Z359ok0jS8mLsbcF+R6UujkABwBJGD+sD6AfqKwOnOjXIrwpD8K\n\tSLQUcXlhznQWdGiDSq9RnZylikxnPwTnLsHQxzG9WXu4KNpwC2+XHfXs2dCzvxbJXY\n\tAq+1i+rIfSf2AUMIhJYkdexgydNJpfvq8i/7cZao="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"k67cC1Rj\"; dkim-atps=neutral","Message-ID":"<67f28222-e4a1-657a-141a-977bcbebd506@ideasonboard.com>","Date":"Mon, 10 Oct 2022 17:03:46 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.2.1","To":"Christian Rauch ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20221008111500.40661-1-Rauch.Christian@gmx.de>\n\t\n\t<166523473715.3760285.9598912606299529172@Monstersaurus>\n\t","Content-Language":"en-US","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: Release all devices on\n\tshutdown","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]