[{"id":11634,"web_url":"https://patchwork.libcamera.org/comment/11634/","msgid":"<20200727150533.GE20890@pendragon.ideasonboard.com>","date":"2020-07-27T15:05:33","subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Mon, Jul 27, 2020 at 12:32:59PM +0000, Umang Jain wrote:\n> If the currently streaming camera is hot-unplugged,\n> a camera reference was still held by MainWindow::camera_,\n> preventing it to be destructed, until qcam window is\n> closed. Plug the leak in the hot-unplug handler itself.\n> \n> Signed-off-by: Umang Jain \n\nReviewed-by: Laurent Pinchart \n\nDoes this fix all the shared pointer refcount issues you've been chasing\n?\n\n> ---\n> src/qcam/main_window.cpp | 2 ++\n> 1 file changed, 2 insertions(+)\n> \n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 7bc1360..c8298ec 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -587,6 +587,8 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> \t\t/* Check if the currently-streaming camera is removed. */\n> \t\tif (camera == camera_.get()) {\n> \t\t\ttoggleCapture(false);\n> +\t\t\tcamera_->release();\n> +\t\t\tcamera_.reset();\n> \t\t\tcameraCombo_->setCurrentIndex(0);\n> \t\t}\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 0D7CCBD86F\n\tfor ;\n\tMon, 27 Jul 2020 15:05:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F27661253;\n\tMon, 27 Jul 2020 17:05:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ACCCF605B2\n\tfor ;\n\tMon, 27 Jul 2020 17:05:41 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2BF6C556;\n\tMon, 27 Jul 2020 17:05:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Kz3ghoWL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595862341;\n\tbh=HTovEVP0Ldvw8VIO0AR5g5/KiNAMeza19OsjABcJTDY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Kz3ghoWLoipWvToNqil2JwPEzZCqpJ00ahpTIqQUbiGwhWReMiCA7V7JS+4ra+tRV\n\t5AWr8MNyI+qrPg8iUQhGk9YIKzzdCzQnS1DSbt8IDUDripBxItrPYeeFeMeHAaI/Nk\n\tt0NVUgpUGX+hQT/2iJK4zXo4pA1r6V4CIML8T5wA=","Date":"Mon, 27 Jul 2020 18:05:33 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"<20200727150533.GE20890@pendragon.ideasonboard.com>","References":"<20200727123247.20412-1-email@uajain.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200727123247.20412-1-email@uajain.com>","Subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","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@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11641,"web_url":"https://patchwork.libcamera.org/comment/11641/","msgid":"","date":"2020-07-27T15:38:00","subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Laurent,\n\nThanks for the review.\n\nOn 7/27/20 8:35 PM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> Thank you for the patch.\n>\n> On Mon, Jul 27, 2020 at 12:32:59PM +0000, Umang Jain wrote:\n>> If the currently streaming camera is hot-unplugged,\n>> a camera reference was still held by MainWindow::camera_,\n>> preventing it to be destructed, until qcam window is\n>> closed. Plug the leak in the hot-unplug handler itself.\n>>\n>> Signed-off-by: Umang Jain \n> Reviewed-by: Laurent Pinchart \n>\n> Does this fix all the shared pointer refcount issues you've been chasing\n> ?\nThe refcount issues are sorted here. I spent extensive time on it (both \nfor hotplug\nand un-plug refcount).\n\nHowever, the other issue I am chasing (earlier believed was caused by \nrefcount issue)\nis still present - which primarily deals with event loop and how \nevent-notifiers are processed.\n\nI can still trigger ASSERT(iter != notifiers_.end()); in \nEventDispatcherPoll::processNotifiers(),\non hot-unplugging a currently streaming camera. This is probably due to \nthe fact that, when\ndestroying a Camera object, it always destroys V4L2VideoDevice() - which \nin turn, \"delete\"s\ntwo EventNotifiers in it's destructor() and then processNotifiers() \nisn't aware about the deletion\nand still trying to find a EventNotifier with concerned pollfd and \nfails  :-(\n\n>\n>> ---\n>> src/qcam/main_window.cpp | 2 ++\n>> 1 file changed, 2 insertions(+)\n>>\n>> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n>> index 7bc1360..c8298ec 100644\n>> --- a/src/qcam/main_window.cpp\n>> +++ b/src/qcam/main_window.cpp\n>> @@ -587,6 +587,8 @@ void MainWindow::processHotplug(HotplugEvent *e)\n>> \t\t/* Check if the currently-streaming camera is removed. */\n>> \t\tif (camera == camera_.get()) {\n>> \t\t\ttoggleCapture(false);\n>> +\t\t\tcamera_->release();\n>> +\t\t\tcamera_.reset();\n>> \t\t\tcameraCombo_->setCurrentIndex(0);\n>> \t\t}\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 AC285BD86F\n\tfor ;\n\tMon, 27 Jul 2020 15:38:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 789E661253;\n\tMon, 27 Jul 2020 17:38:04 +0200 (CEST)","from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6F6FD605B2\n\tfor ;\n\tMon, 27 Jul 2020 17:38:01 +0200 (CEST)","by filterdrecv-p3mdw1-75c584b9c6-xjd6c with SMTP id\n\tfilterdrecv-p3mdw1-75c584b9c6-xjd6c-20-5F1EF4D8-11\n\t2020-07-27 15:38:00.35082422 +0000 UTC m=+2672902.323514980","from mail.uajain.com (unknown)\n\tby ismtpd0005p1maa1.sendgrid.net (SG) with ESMTP\n\tid Z7HdSN9PS5Wp62VfP_y5SQ Mon, 27 Jul 2020 15:37:59.730 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"merw/kHs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com;\n\th=subject:references:from:mime-version:in-reply-to:to:cc:content-type:\n\tcontent-transfer-encoding;\n\ts=s1; bh=yJuBIvB6/b5F/7/gaJf+XLN7kDBD4c6igmGvQ03ctBU=;\n\tb=merw/kHs1KQypAnAluIHEWGitiR/WbRYSMuFbpMiYpeHYFN6KVL35fNCSOK9lCJ2eSmE\n\tZsylr0/uJZa2MSPqpdAcCzkEUBa73bSqIGenJpw9nH3hlas1AqDmtbkxiUBvJjvALdBPFK\n\tAclwtzcX7BufoV9ljx9WED1hBPTNYNP8U=","References":"<20200727123247.20412-1-email@uajain.com>\n\t<20200727150533.GE20890@pendragon.ideasonboard.com>","From":"Umang Jain ","Message-ID":"","Date":"Mon, 27 Jul 2020 15:38:00 +0000 (UTC)","Mime-Version":"1.0","In-Reply-To":"<20200727150533.GE20890@pendragon.ideasonboard.com>","X-SG-EID":"1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPcf4tThB+biFNxNhp9C3ilfxokrA2RsT5XTZtt414vM9vxCioeAE7xNJya8NrCeJwyptA+EBG9zEI0B96GKObeXuQ4SqNJi4Kg1fInk9q9dYqyl12yEeFnov2yhdAZYIL9pCaJUx8NTFnftHRdepxHe1bMWo3zMknqSiRysjF89Zl0rKaDudguzhPcIoQtJ/5MFt8ZQXVY24gHE+4lq66j2w==","To":"Laurent Pinchart ","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","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@lists.libcamera.org","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=\"iso-8859-1\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11647,"web_url":"https://patchwork.libcamera.org/comment/11647/","msgid":"<20200727155034.GC17521@pendragon.ideasonboard.com>","date":"2020-07-27T15:50:34","subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Mon, Jul 27, 2020 at 03:38:00PM +0000, Umang Jain wrote:\n> On 7/27/20 8:35 PM, Laurent Pinchart wrote:\n> > On Mon, Jul 27, 2020 at 12:32:59PM +0000, Umang Jain wrote:\n> >> If the currently streaming camera is hot-unplugged,\n> >> a camera reference was still held by MainWindow::camera_,\n> >> preventing it to be destructed, until qcam window is\n> >> closed. Plug the leak in the hot-unplug handler itself.\n> >>\n> >> Signed-off-by: Umang Jain \n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > Does this fix all the shared pointer refcount issues you've been chasing\n> > ?\n>\n> The refcount issues are sorted here. I spent extensive time on it (both for hotplug\n> and un-plug refcount).\n> \n> However, the other issue I am chasing (earlier believed was caused by refcount issue)\n> is still present - which primarily deals with event loop and how event-notifiers are processed.\n> \n> I can still trigger ASSERT(iter != notifiers_.end()); in EventDispatcherPoll::processNotifiers(),\n> on hot-unplugging a currently streaming camera. This is probably due to the fact that, when\n> destroying a Camera object, it always destroys V4L2VideoDevice() - which in turn, \"delete\"s\n> two EventNotifiers in it's destructor() and then processNotifiers() isn't aware about the deletion\n> and still trying to find a EventNotifier with concerned pollfd and fails  :-(\n\nThe issue is that the camera object is meant to be destroyed in the\nthread it belongs to, as explained in a previous e-mail (see [1]). Is\nthat something you can try fixing, with the pointers in that e-mail, or\nwould you like to discuss it further first ?\n\n[1] https://lists.libcamera.org/pipermail/libcamera-devel/2020-July/010951.html\n\n> >> ---\n> >> src/qcam/main_window.cpp | 2 ++\n> >> 1 file changed, 2 insertions(+)\n> >>\n> >> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> >> index 7bc1360..c8298ec 100644\n> >> --- a/src/qcam/main_window.cpp\n> >> +++ b/src/qcam/main_window.cpp\n> >> @@ -587,6 +587,8 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> >> \t\t/* Check if the currently-streaming camera is removed. */\n> >> \t\tif (camera == camera_.get()) {\n> >> \t\t\ttoggleCapture(false);\n> >> +\t\t\tcamera_->release();\n> >> +\t\t\tcamera_.reset();\n> >> \t\t\tcameraCombo_->setCurrentIndex(0);\n> >> \t\t}\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 5A5A0BD86F\n\tfor ;\n\tMon, 27 Jul 2020 15:50:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D3FD86118A;\n\tMon, 27 Jul 2020 17:50:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E7B160536\n\tfor ;\n\tMon, 27 Jul 2020 17:50:43 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D29C7556;\n\tMon, 27 Jul 2020 17:50:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"nBQLEjhj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595865042;\n\tbh=ob5tT64qxizNNcuTnuHIpdnqHH8F+TQ4zChFMinVrOw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nBQLEjhjSfId6f/bdt817zTgxtusfumX5MIKw8GCA0OXx6Ho6bNsIpvpcxxel1kHl\n\t3QGx9Z8t0u2tivPGi+DSiOQxJXFCAU6WkYDacklQdfjjPQVj+knyTzINBh8SEM9m+G\n\tQdYp4Epfcpay3wLwlWo6yt8f8ZliHPo6RWPlxH4I=","Date":"Mon, 27 Jul 2020 18:50:34 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"<20200727155034.GC17521@pendragon.ideasonboard.com>","References":"<20200727123247.20412-1-email@uajain.com>\n\t<20200727150533.GE20890@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","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@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11652,"web_url":"https://patchwork.libcamera.org/comment/11652/","msgid":"<05cf7d9d-ae81-1a75-2db7-dd5a05aa00a1@uajain.com>","date":"2020-07-27T16:08:43","subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Laurent,\n\nOn 7/27/20 9:20 PM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> On Mon, Jul 27, 2020 at 03:38:00PM +0000, Umang Jain wrote:\n>> On 7/27/20 8:35 PM, Laurent Pinchart wrote:\n>>> On Mon, Jul 27, 2020 at 12:32:59PM +0000, Umang Jain wrote:\n>>>> If the currently streaming camera is hot-unplugged,\n>>>> a camera reference was still held by MainWindow::camera_,\n>>>> preventing it to be destructed, until qcam window is\n>>>> closed. Plug the leak in the hot-unplug handler itself.\n>>>>\n>>>> Signed-off-by: Umang Jain \n>>> Reviewed-by: Laurent Pinchart \n>>>\n>>> Does this fix all the shared pointer refcount issues you've been chasing\n>>> ?\n>> The refcount issues are sorted here. I spent extensive time on it (both for hotplug\n>> and un-plug refcount).\n>>\n>> However, the other issue I am chasing (earlier believed was caused by refcount issue)\n>> is still present - which primarily deals with event loop and how event-notifiers are processed.\n>>\n>> I can still trigger ASSERT(iter != notifiers_.end()); in EventDispatcherPoll::processNotifiers(),\n>> on hot-unplugging a currently streaming camera. This is probably due to the fact that, when\n>> destroying a Camera object, it always destroys V4L2VideoDevice() - which in turn, \"delete\"s\n>> two EventNotifiers in it's destructor() and then processNotifiers() isn't aware about the deletion\n>> and still trying to find a EventNotifier with concerned pollfd and fails  :-(\n> The issue is that the camera object is meant to be destroyed in the\n> thread it belongs to, as explained in a previous e-mail (see [1]). Is\n> that something you can try fixing, with the pointers in that e-mail, or\n> would you like to discuss it further first ?\n>\n> [1] https://lists.libcamera.org/pipermail/libcamera-devel/2020-July/010951.html\nYou were right all along.\nI will try to follow the pointers and come up with an implementation.\n>\n>>>> ---\n>>>> src/qcam/main_window.cpp | 2 ++\n>>>> 1 file changed, 2 insertions(+)\n>>>>\n>>>> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n>>>> index 7bc1360..c8298ec 100644\n>>>> --- a/src/qcam/main_window.cpp\n>>>> +++ b/src/qcam/main_window.cpp\n>>>> @@ -587,6 +587,8 @@ void MainWindow::processHotplug(HotplugEvent *e)\n>>>> \t\t/* Check if the currently-streaming camera is removed. */\n>>>> \t\tif (camera == camera_.get()) {\n>>>> \t\t\ttoggleCapture(false);\n>>>> +\t\t\tcamera_->release();\n>>>> +\t\t\tcamera_.reset();\n>>>> \t\t\tcameraCombo_->setCurrentIndex(0);\n>>>> \t\t}\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 111C8BD878\n\tfor ;\n\tMon, 27 Jul 2020 16:08:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8041F613B6;\n\tMon, 27 Jul 2020 18:08:46 +0200 (CEST)","from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E4005605B2\n\tfor ;\n\tMon, 27 Jul 2020 18:08:44 +0200 (CEST)","by filterdrecv-p3mdw1-75c584b9c6-xpzlh with SMTP id\n\tfilterdrecv-p3mdw1-75c584b9c6-xpzlh-18-5F1EFC0B-28\n\t2020-07-27 16:08:43.492916297 +0000 UTC m=+2674746.846109015","from mail.uajain.com (unknown)\n\tby ismtpd0006p1maa1.sendgrid.net (SG) with ESMTP\n\tid dvMlTZNkQPCnujmdy93V9g Mon, 27 Jul 2020 16:08:42.882 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"amGkMyld\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com;\n\th=subject:references:from:mime-version:in-reply-to:to:cc:content-type:\n\tcontent-transfer-encoding;\n\ts=s1; bh=6spptpIH4KvKE0+8OJU8iVtqRxj8BsvPbDCLWVxkU7g=;\n\tb=amGkMyldFMkKiyaYQWv9ZFm69ujDyTNYTIi+R3FNo84UHLt2KRBJIRFBmSrQtEXUH+4Q\n\tECA+t+81rzBbwnWtnib6/Q/fRBLbLtcMlfhxcL8gLOrS2i0hBgD/BKdtJ0M9tOkusp6jd+\n\tAvZMO60OD4Nvg6iXIXF+nHFN/YBo3Toxg=","References":"<20200727123247.20412-1-email@uajain.com>\n\t<20200727150533.GE20890@pendragon.ideasonboard.com>\n\t\n\t<20200727155034.GC17521@pendragon.ideasonboard.com>","From":"Umang Jain ","Message-ID":"<05cf7d9d-ae81-1a75-2db7-dd5a05aa00a1@uajain.com>","Date":"Mon, 27 Jul 2020 16:08:43 +0000 (UTC)","Mime-Version":"1.0","In-Reply-To":"<20200727155034.GC17521@pendragon.ideasonboard.com>","X-SG-EID":"1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPcO0wIVNwfic3Culi7Uik3l/acIM66+jgwJ/AW8Kx47574f5Uj1efDnytrs1Mfs3dTctpwf0ckxfGXF6ibS8EAdqo6DALuvB80bPLpT2ySL8fY/OGxucJcBVt4NTwIK4R4wgnmResBtemGOycpbrRpqYsSmdFg9H5JAQBKuEn0D9C/a4ScLBL5EemL3eUPdOMZXbZG3uVESR+oj91Y93/q4g==","To":"Laurent Pinchart ","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","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@lists.libcamera.org","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=\"iso-8859-1\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11655,"web_url":"https://patchwork.libcamera.org/comment/11655/","msgid":"","date":"2020-07-27T20:20:40","subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi again,\n\nOn 7/27/20 9:20 PM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> On Mon, Jul 27, 2020 at 03:38:00PM +0000, Umang Jain wrote:\n>> On 7/27/20 8:35 PM, Laurent Pinchart wrote:\n>>> On Mon, Jul 27, 2020 at 12:32:59PM +0000, Umang Jain wrote:\n>>>> If the currently streaming camera is hot-unplugged,\n>>>> a camera reference was still held by MainWindow::camera_,\n>>>> preventing it to be destructed, until qcam window is\n>>>> closed. Plug the leak in the hot-unplug handler itself.\n>>>>\n>>>> Signed-off-by: Umang Jain \n>>> Reviewed-by: Laurent Pinchart \n>>>\n>>> Does this fix all the shared pointer refcount issues you've been chasing\n>>> ?\n>> The refcount issues are sorted here. I spent extensive time on it (both for hotplug\n>> and un-plug refcount).\n>>\n>> However, the other issue I am chasing (earlier believed was caused by refcount issue)\n>> is still present - which primarily deals with event loop and how event-notifiers are processed.\n>>\n>> I can still trigger ASSERT(iter != notifiers_.end()); in EventDispatcherPoll::processNotifiers(),\n>> on hot-unplugging a currently streaming camera. This is probably due to the fact that, when\n>> destroying a Camera object, it always destroys V4L2VideoDevice() - which in turn, \"delete\"s\n>> two EventNotifiers in it's destructor() and then processNotifiers() isn't aware about the deletion\n>> and still trying to find a EventNotifier with concerned pollfd and fails  :-(\n> The issue is that the camera object is meant to be destroyed in the\n> thread it belongs to, as explained in a previous e-mail (see [1]). Is\n> that something you can try fixing, with the pointers in that e-mail, or\n> would you like to discuss it further first ?\n>\n> [1] https://lists.libcamera.org/pipermail/libcamera-devel/2020-July/010951.html\nhmm, I figured that I need to call the destructor via \nObject::invokeMethod() with\nconnection type ConnectionTypeQueued to ensure the destructor is called \nwithin\nit's own thread. Something on the lines:\n\nvoid Object::deleteLater()\n\n{\n\nthis->invokeMethod(&(typeid(this).name()::customDeletorFunc), \nConnectionTypeQueued);\n\n}\n\nand\n\nvoid className::customDeletorFunc()\n{\n\n~className();\n\n}\n\nwhich obviously does not compile for me.\n\n\nHow would one go about calling a destructor(in this case Camera's) from \nit's one\nof the parent class(i.e. Object)? I am very cloudy around the syntax \nformation for\nthis->invokeMethod() above. Any help is appreciated.\n\n>\n>>>> ---\n>>>> src/qcam/main_window.cpp | 2 ++\n>>>> 1 file changed, 2 insertions(+)\n>>>>\n>>>> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n>>>> index 7bc1360..c8298ec 100644\n>>>> --- a/src/qcam/main_window.cpp\n>>>> +++ b/src/qcam/main_window.cpp\n>>>> @@ -587,6 +587,8 @@ void MainWindow::processHotplug(HotplugEvent *e)\n>>>> \t\t/* Check if the currently-streaming camera is removed. */\n>>>> \t\tif (camera == camera_.get()) {\n>>>> \t\t\ttoggleCapture(false);\n>>>> +\t\t\tcamera_->release();\n>>>> +\t\t\tcamera_.reset();\n>>>> \t\t\tcameraCombo_->setCurrentIndex(0);\n>>>> \t\t}\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 2A2BBBD878\n\tfor ;\n\tMon, 27 Jul 2020 20:20:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9538261223;\n\tMon, 27 Jul 2020 22:20:43 +0200 (CEST)","from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1CB9B60536\n\tfor ;\n\tMon, 27 Jul 2020 22:20:41 +0200 (CEST)","by filterdrecv-p3iad2-5b55dcd864-t97fw with SMTP id\n\tfilterdrecv-p3iad2-5b55dcd864-t97fw-17-5F1F3718-3F\n\t2020-07-27 20:20:40.577718529 +0000 UTC m=+2689882.212012389","from mail.uajain.com (unknown) by geopod-ismtpd-2-1 (SG) with ESMTP\n\tid eUpTlkYYSVu2xqWVYv6z0Q Mon, 27 Jul 2020 20:20:40.315 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"acSAS5JX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com;\n\th=subject:references:from:mime-version:in-reply-to:to:cc:content-type:\n\tcontent-transfer-encoding;\n\ts=s1; bh=Wborenz/ppRqSQifToGrBJU934LvptDd8UNcWNEWs8c=;\n\tb=acSAS5JXMOxVsUzMntyQLFWBWpqzBMtyR1doqbJodSgF66hhktBILeDjBsHSOdQ0htNd\n\tvF+a1kMwGhBI9XeGhtFXsSJC3JyGrV8kMj7RzSwDINEH1WfDwkpk7IZf2FX7g1t3le6ZgR\n\tiC6aBg6xXCQrh5ax0BIGFuk68NQOS8+wk=","References":"<20200727123247.20412-1-email@uajain.com>\n\t<20200727150533.GE20890@pendragon.ideasonboard.com>\n\t\n\t<20200727155034.GC17521@pendragon.ideasonboard.com>","From":"Umang Jain ","Message-ID":"","Date":"Mon, 27 Jul 2020 20:20:40 +0000 (UTC)","Mime-Version":"1.0","In-Reply-To":"<20200727155034.GC17521@pendragon.ideasonboard.com>","X-SG-EID":"1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPcjyJADjWRSqlSUDT552L9UFA27naIhOhmMZ0AwzDCQ6BVxc5S+8bKiYrs6JZ8xC07y4OL776An+zLrRRnlOPpimtgK3BmrQ0QpICQhmTKJsWngKlrLHmqSZtmNHu4FNTsltBzD5vctIDjaTQK8i3OdsZlL4EyAdnRAK2NHQftC3U9GFpsGQ2iXvL2rQ4i3+mEgT0wMRpaPvw0t68tSVgl0g==","To":"Laurent Pinchart ","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","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@lists.libcamera.org","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=\"iso-8859-1\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11658,"web_url":"https://patchwork.libcamera.org/comment/11658/","msgid":"<20200727230948.GD15448@pendragon.ideasonboard.com>","date":"2020-07-27T23:09:48","subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Mon, Jul 27, 2020 at 08:20:40PM +0000, Umang Jain wrote:\n> On 7/27/20 9:20 PM, Laurent Pinchart wrote:\n> > On Mon, Jul 27, 2020 at 03:38:00PM +0000, Umang Jain wrote:\n> >> On 7/27/20 8:35 PM, Laurent Pinchart wrote:\n> >>> On Mon, Jul 27, 2020 at 12:32:59PM +0000, Umang Jain wrote:\n> >>>> If the currently streaming camera is hot-unplugged,\n> >>>> a camera reference was still held by MainWindow::camera_,\n> >>>> preventing it to be destructed, until qcam window is\n> >>>> closed. Plug the leak in the hot-unplug handler itself.\n> >>>>\n> >>>> Signed-off-by: Umang Jain \n> >>>\n> >>> Reviewed-by: Laurent Pinchart \n> >>>\n> >>> Does this fix all the shared pointer refcount issues you've been chasing\n> >>> ?\n> >>\n> >> The refcount issues are sorted here. I spent extensive time on it (both for hotplug\n> >> and un-plug refcount).\n> >>\n> >> However, the other issue I am chasing (earlier believed was caused by refcount issue)\n> >> is still present - which primarily deals with event loop and how event-notifiers are processed.\n> >>\n> >> I can still trigger ASSERT(iter != notifiers_.end()); in EventDispatcherPoll::processNotifiers(),\n> >> on hot-unplugging a currently streaming camera. This is probably due to the fact that, when\n> >> destroying a Camera object, it always destroys V4L2VideoDevice() - which in turn, \"delete\"s\n> >> two EventNotifiers in it's destructor() and then processNotifiers() isn't aware about the deletion\n> >> and still trying to find a EventNotifier with concerned pollfd and fails  :-(\n> >\n> > The issue is that the camera object is meant to be destroyed in the\n> > thread it belongs to, as explained in a previous e-mail (see [1]). Is\n> > that something you can try fixing, with the pointers in that e-mail, or\n> > would you like to discuss it further first ?\n> >\n> > [1] https://lists.libcamera.org/pipermail/libcamera-devel/2020-July/010951.html\n>\n> hmm, I figured that I need to call the destructor via \n> Object::invokeMethod() with\n> connection type ConnectionTypeQueued to ensure the destructor is called \n> within\n> it's own thread. Something on the lines:\n> \n> void Object::deleteLater()\n> {\n> \tthis->invokeMethod(&(typeid(this).name()::customDeletorFunc),\n> \t\t\t ConnectionTypeQueued);\n\nYou can drop the 'this->', calling a member function of an object from\nwithin the same object doesn't require explicitly accessing 'this'.\n\n> }\n> \n> and\n> \n> void className::customDeletorFunc()\n> {\n> \t~className();\n> }\n> \n> which obviously does not compile for me.\n>\n> How would one go about calling a destructor(in this case Camera's) from it's one\n> of the parent class(i.e. Object)? I am very cloudy around the syntax formation for\n> this->invokeMethod() above. Any help is appreciated.\n\nThis indeed won't compile. The good news is that it can be simplified,\nfixing compilation at the same time :-)\n\nYour code, calling ~className(), would indeed call the destructor, but\nwouldn't free memory. Destruction of an object involves two steps, first\nthe destructors are called (in order from the most derived class to the\nparent class) to perform cleanups specific to the object. Then, memory\nis freed. When you allocate an object on the stack with\n\n\t{\n\t\tObject o(...);\n\t\t...\n\t} <-- 1\n\nat point 1 the object becomes out of scope and is destroyed. The\ncompiler generates code to call the destructors, and then simply updates\nthe stack pointer to free the memory.\n\nWhen you allocate an object on the heap with\n\n\tObject *o = new Object(...);\n\t...\n\tdelete o; <-- 1\n\nat point 1 the object is explicitly destroyed. The destructors are\ncalled, as in the stack case, but to free the memory, the compiler calls\nthe delete operator (to be pedantic, a delete expression, formed by the\n'delete' keyword followed by an expression, is compiled as an invocation\nof the destructor, followed by a call to the delete operator to free the\nmemory - see [1] if you're interested in the details).\n\nFreeing memory is thus independent of the destructor, so calling the\ndestructor manually isn't enough. The good news is that we don't need to\ndo so manually. The Object class has a virtual destructor, so deleting\nan object using an Object pointer will call the destructor of the\nderived class. You don't need to get hold of a pointer to the derived\nclass. You can thus simply implement\n\nvoid Object::customDeletorFunc()\n{\n\tdelete this;\n}\n\nvoid Object::deleteLater()\n{\n\tinvokeMethod(&Object::customDeletorFunc, ConnectionTypeQueued);\n}\n\nHowever, I think we can optimize the implementation a little bit to\navoid going through the complex machinery of invokeMethod(). I would\nrecommend adding a new DeferredDelete entry to the Message::Type\nenumeration, and simply posting the message with\n\nvoid Object::deleteLater()\n{\n\tpostMessage(std::make_unique(Message::DeferredDelete));\n}\n\nand to handle that, in Object::message(), add\n\n\tcase Message::DeferredDelete:\n\t\tdelete this;\n\t\tbreak;\n\nFinally, you'll have to wire that with a custom deleter when creating\nthe camera shared pointer. Fortunately we have one already in\nCamera::create() :-) It should thus just be a matter of replacing\n\n\tstruct Deleter : std::default_delete {\n\t\tvoid operator()(Camera *camera)\n\t\t{\n\t\t\tdelete camera;\n\t\t}\n\t};\n\nwith\n\n\tstruct Deleter : std::default_delete {\n\t\tvoid operator()(Camera *camera)\n\t\t{\n\t\t\tcamera->deleteLater();\n\t\t}\n\t};\n\nand of course crossing fingers :-)\n\n[1] https://en.cppreference.com/w/cpp/language/delete\n\n> >>>> ---\n> >>>> src/qcam/main_window.cpp | 2 ++\n> >>>> 1 file changed, 2 insertions(+)\n> >>>>\n> >>>> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> >>>> index 7bc1360..c8298ec 100644\n> >>>> --- a/src/qcam/main_window.cpp\n> >>>> +++ b/src/qcam/main_window.cpp\n> >>>> @@ -587,6 +587,8 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> >>>> \t\t/* Check if the currently-streaming camera is removed. */\n> >>>> \t\tif (camera == camera_.get()) {\n> >>>> \t\t\ttoggleCapture(false);\n> >>>> +\t\t\tcamera_->release();\n> >>>> +\t\t\tcamera_.reset();\n> >>>> \t\t\tcameraCombo_->setCurrentIndex(0);\n> >>>> \t\t}\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 A01A2BD86F\n\tfor ;\n\tMon, 27 Jul 2020 23:09:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1F830613C6;\n\tTue, 28 Jul 2020 01:09:58 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5EF086053C\n\tfor ;\n\tTue, 28 Jul 2020 01:09:56 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C56F4556;\n\tTue, 28 Jul 2020 01:09:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sylh9wUB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595891396;\n\tbh=Z+wHnIiTihEVLNF5sNWYqwwzCx3Od5eIQ0oJVJq+MSw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sylh9wUB/95lH7E3IGRAp6IsqIHLjGdQ2sMXDdgxD7Jsb31itVcDqih9lN/ZRZ29R\n\tZNbp5pr+rzE54DvwW/f8FpTgvBEBkQ5OISs4QeGQJ02V0X1roULcXmwJzoUO3aNzwV\n\tcS8LyAeK4TNwB90qg/B/8eXqYqvkK7yXByLRcArs=","Date":"Tue, 28 Jul 2020 02:09:48 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"<20200727230948.GD15448@pendragon.ideasonboard.com>","References":"<20200727123247.20412-1-email@uajain.com>\n\t<20200727150533.GE20890@pendragon.ideasonboard.com>\n\t\n\t<20200727155034.GC17521@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","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@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11666,"web_url":"https://patchwork.libcamera.org/comment/11666/","msgid":"","date":"2020-07-28T11:01:44","subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Laurent,\n\nOn 7/28/20 4:39 AM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> On Mon, Jul 27, 2020 at 08:20:40PM +0000, Umang Jain wrote:\n>> On 7/27/20 9:20 PM, Laurent Pinchart wrote:\n>>> On Mon, Jul 27, 2020 at 03:38:00PM +0000, Umang Jain wrote:\n>>>> On 7/27/20 8:35 PM, Laurent Pinchart wrote:\n>>>>> On Mon, Jul 27, 2020 at 12:32:59PM +0000, Umang Jain wrote:\n>>>>>> If the currently streaming camera is hot-unplugged,\n>>>>>> a camera reference was still held by MainWindow::camera_,\n>>>>>> preventing it to be destructed, until qcam window is\n>>>>>> closed. Plug the leak in the hot-unplug handler itself.\n>>>>>>\n>>>>>> Signed-off-by: Umang Jain \n>>>>> Reviewed-by: Laurent Pinchart \n>>>>>\n>>>>> Does this fix all the shared pointer refcount issues you've been chasing\n>>>>> ?\n>>>> The refcount issues are sorted here. I spent extensive time on it (both for hotplug\n>>>> and un-plug refcount).\n>>>>\n>>>> However, the other issue I am chasing (earlier believed was caused by refcount issue)\n>>>> is still present - which primarily deals with event loop and how event-notifiers are processed.\n>>>>\n>>>> I can still trigger ASSERT(iter != notifiers_.end()); in EventDispatcherPoll::processNotifiers(),\n>>>> on hot-unplugging a currently streaming camera. This is probably due to the fact that, when\n>>>> destroying a Camera object, it always destroys V4L2VideoDevice() - which in turn, \"delete\"s\n>>>> two EventNotifiers in it's destructor() and then processNotifiers() isn't aware about the deletion\n>>>> and still trying to find a EventNotifier with concerned pollfd and fails  :-(\n>>> The issue is that the camera object is meant to be destroyed in the\n>>> thread it belongs to, as explained in a previous e-mail (see [1]). Is\n>>> that something you can try fixing, with the pointers in that e-mail, or\n>>> would you like to discuss it further first ?\n>>>\n>>> [1] https://lists.libcamera.org/pipermail/libcamera-devel/2020-July/010951.html\n>> hmm, I figured that I need to call the destructor via\n>> Object::invokeMethod() with\n>> connection type ConnectionTypeQueued to ensure the destructor is called\n>> within\n>> it's own thread. Something on the lines:\n>>\n>> void Object::deleteLater()\n>> {\n>> \tthis->invokeMethod(&(typeid(this).name()::customDeletorFunc),\n>> \t\t\t ConnectionTypeQueued);\n> You can drop the 'this->', calling a member function of an object from\n> within the same object doesn't require explicitly accessing 'this'.\n>\n>> }\n>>\n>> and\n>>\n>> void className::customDeletorFunc()\n>> {\n>> \t~className();\n>> }\n>>\n>> which obviously does not compile for me.\n>>\n>> How would one go about calling a destructor(in this case Camera's) from it's one\n>> of the parent class(i.e. Object)? I am very cloudy around the syntax formation for\n>> this->invokeMethod() above. Any help is appreciated.\n> This indeed won't compile. The good news is that it can be simplified,\n> fixing compilation at the same time :-)\n>\n> Your code, calling ~className(), would indeed call the destructor, but\n> wouldn't free memory. Destruction of an object involves two steps, first\n> the destructors are called (in order from the most derived class to the\n> parent class) to perform cleanups specific to the object. Then, memory\n> is freed. When you allocate an object on the stack with\n>\n> \t{\n> \t\tObject o(...);\n> \t\t...\n> \t} <-- 1\n>\n> at point 1 the object becomes out of scope and is destroyed. The\n> compiler generates code to call the destructors, and then simply updates\n> the stack pointer to free the memory.\n>\n> When you allocate an object on the heap with\n>\n> \tObject *o = new Object(...);\n> \t...\n> \tdelete o; <-- 1\n>\n> at point 1 the object is explicitly destroyed. The destructors are\n> called, as in the stack case, but to free the memory, the compiler calls\n> the delete operator (to be pedantic, a delete expression, formed by the\n> 'delete' keyword followed by an expression, is compiled as an invocation\n> of the destructor, followed by a call to the delete operator to free the\n> memory - see [1] if you're interested in the details).\nThanks for the explaination. I indeed was missing these basics concepts :(\n>\n> Freeing memory is thus independent of the destructor, so calling the\n> destructor manually isn't enough. The good news is that we don't need to\n> do so manually. The Object class has a virtual destructor, so deleting\n> an object using an Object pointer will call the destructor of the\n> derived class. You don't need to get hold of a pointer to the derived\n> class. You can thus simply implement\n>\n> void Object::customDeletorFunc()\n> {\n> \tdelete this;\n> }\n>\n> void Object::deleteLater()\n> {\n> \tinvokeMethod(&Object::customDeletorFunc, ConnectionTypeQueued);\n> }\n>\n> However, I think we can optimize the implementation a little bit to\n> avoid going through the complex machinery of invokeMethod(). I would\n> recommend adding a new DeferredDelete entry to the Message::Type\n> enumeration, and simply posting the message with\n>\n> void Object::deleteLater()\n> {\n> \tpostMessage(std::make_unique(Message::DeferredDelete));\n> }\n>\n> and to handle that, in Object::message(), add\n>\n> \tcase Message::DeferredDelete:\n> \t\tdelete this;\n> \t\tbreak;\nYeah, this quite comes close to what Qt does for it's \nQObject::deleteLater().\nIt posts an event to defer delete.\n>\n> Finally, you'll have to wire that with a custom deleter when creating\n> the camera shared pointer. Fortunately we have one already in\n> Camera::create() :-) It should thus just be a matter of replacing\n>\n> \tstruct Deleter : std::default_delete {\n> \t\tvoid operator()(Camera *camera)\n> \t\t{\n> \t\t\tdelete camera;\n> \t\t}\n> \t};\n>\n> with\n>\n> \tstruct Deleter : std::default_delete {\n> \t\tvoid operator()(Camera *camera)\n> \t\t{\n> \t\t\tcamera->deleteLater();\n> \t\t}\n> \t};\n>\n> and of course crossing fingers :-)\n\nThanks for the input.\nThis is really simplified now and does not look as scary as I thought \ninitially :)\nI have pushed patches for review right now on the list.\n\n>\n> [1] https://en.cppreference.com/w/cpp/language/delete\n>\n>>>>>> ---\n>>>>>> src/qcam/main_window.cpp | 2 ++\n>>>>>> 1 file changed, 2 insertions(+)\n>>>>>>\n>>>>>> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n>>>>>> index 7bc1360..c8298ec 100644\n>>>>>> --- a/src/qcam/main_window.cpp\n>>>>>> +++ b/src/qcam/main_window.cpp\n>>>>>> @@ -587,6 +587,8 @@ void MainWindow::processHotplug(HotplugEvent *e)\n>>>>>> \t\t/* Check if the currently-streaming camera is removed. */\n>>>>>> \t\tif (camera == camera_.get()) {\n>>>>>> \t\t\ttoggleCapture(false);\n>>>>>> +\t\t\tcamera_->release();\n>>>>>> +\t\t\tcamera_.reset();\n>>>>>> \t\t\tcameraCombo_->setCurrentIndex(0);\n>>>>>> \t\t}\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 180A4BD878\n\tfor ;\n\tTue, 28 Jul 2020 11:01:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 72B426118A;\n\tTue, 28 Jul 2020 13:01:47 +0200 (CEST)","from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E3446039B\n\tfor ;\n\tTue, 28 Jul 2020 13:01:45 +0200 (CEST)","by filterdrecv-p3iad2-5b55dcd864-hp5f5 with SMTP id\n\tfilterdrecv-p3iad2-5b55dcd864-hp5f5-19-5F200597-1A8\n\t2020-07-28 11:01:44.05637956 +0000 UTC m=+2742742.099807544","from mail.uajain.com (unknown)\n\tby ismtpd0002p1hnd1.sendgrid.net (SG) with ESMTP\n\tid qtbSihxSTRS0bzE86dDO7g Tue, 28 Jul 2020 11:01:43.564 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"pcFvoEY3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com;\n\th=subject:references:from:mime-version:in-reply-to:to:cc:content-type:\n\tcontent-transfer-encoding;\n\ts=s1; bh=FA7mUVMrnDyU+DCSitI4a+b137sVAr+F0RHfeEcz/NI=;\n\tb=pcFvoEY36EgqzqkiS9sOOelGf22M1gGYRBAoI8buKpSe2LtszHs97zAXhlqwiAOGojBb\n\tLgJclwVW5etKEmEEE1mV4rzCLaMfSQ6JDMBcbxeLbQs1xVPMM9IfeUTbTlWUCuBLtV4TSY\n\tG11AA0F85tSO1uRxFVdthfYLaIf9w6H0U=","References":"<20200727123247.20412-1-email@uajain.com>\n\t<20200727150533.GE20890@pendragon.ideasonboard.com>\n\t\n\t<20200727155034.GC17521@pendragon.ideasonboard.com>\n\t\n\t<20200727230948.GD15448@pendragon.ideasonboard.com>","From":"Umang Jain ","Message-ID":"","Date":"Tue, 28 Jul 2020 11:01:44 +0000 (UTC)","Mime-Version":"1.0","In-Reply-To":"<20200727230948.GD15448@pendragon.ideasonboard.com>","X-SG-EID":"1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPc4om/Oij3lMEQBxKPi/30O17Ecjv97P2mf0EP94kse14N6tceEIZK8fRZUsoRxDcKZCdv9ptFjx4sX5QqepwQIlXtI1A9bCB5Z/gDY25hDZs2Kop/TJo66zva1XwJgix1QCCZwLPkEIPk308EXJAJP03uel8aKq9n4RRzGk5exFdjxX6Ot+taeuXX7e6E8RaUcKV2/lNIc/ozB9p6ZxswLg==","To":"Laurent Pinchart ","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] qcam: Fix camera reference leak on\n\thot-unplug","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@lists.libcamera.org","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=\"iso-8859-1\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]