[{"id":25928,"web_url":"https://patchwork.libcamera.org/comment/25928/","msgid":"<166980575093.1079859.5458882092490560008@Monstersaurus>","date":"2022-11-30T10:55:50","subject":"Re: [libcamera-devel] [PATCH] lc-compliance: Fix segfault on\n\trequest destruction","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Nícolas,\n\nThanks for looking at this issue.\n\nQuoting Nícolas F. R. A. Prado via libcamera-devel (2022-11-30 00:10:44)\n> The Request destructor cancels all associated framebuffers, and\n> therefore assumes they are all still valid. The SimpleCaptureBalanced\n> and SimpleCaptureUnbalanced classes that run the capture sessions on\n> lc-compliance however were freeing the framebuffers before the requests,\n> stored in an automatic variable, were destroyed. This resulted in a\n> segfault when running lc-compliance.\n> \n> Solve the issue by moving the requests vector to the SimpleCapture class\n> and making sure we clear it on stop() before freeing the framebuffers.\n\nI'm afraid Paul has 'just' beaten you to it with a patch posted\nyesterday:\n\n - https://patchwork.libcamera.org/patch/17906/\n\n\nCould you review and test his patch please?\n\n--\nKieran\n\n\n> Signed-off-by: Nícolas F. R. A. Prado \n> \n> ---\n> \n> src/apps/lc-compliance/simple_capture.cpp | 8 ++++----\n> src/apps/lc-compliance/simple_capture.h | 1 +\n> 2 files changed, 5 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/apps/lc-compliance/simple_capture.cpp b/src/apps/lc-compliance/simple_capture.cpp\n> index ab5cb35c11f2..d02c7b45f180 100644\n> --- a/src/apps/lc-compliance/simple_capture.cpp\n> +++ b/src/apps/lc-compliance/simple_capture.cpp\n> @@ -64,6 +64,8 @@ void SimpleCapture::stop()\n> \n> camera_->requestCompleted.disconnect(this);\n> \n> + requests_.clear();\n> +\n> Stream *stream = config_->at(0).stream();\n> allocator_->free(stream);\n> }\n> @@ -95,7 +97,6 @@ void SimpleCaptureBalanced::capture(unsigned int numRequests)\n> captureLimit_ = numRequests;\n> \n> /* Queue the recommended number of reqeuests. */\n> - std::vector> requests;\n> for (const std::unique_ptr &buffer : buffers) {\n> std::unique_ptr request = camera_->createRequest();\n> ASSERT_TRUE(request) << \"Can't create request\";\n> @@ -104,7 +105,7 @@ void SimpleCaptureBalanced::capture(unsigned int numRequests)\n> \n> ASSERT_EQ(queueRequest(request.get()), 0) << \"Failed to queue request\";\n> \n> - requests.push_back(std::move(request));\n> + requests_.push_back(std::move(request));\n> }\n> \n> /* Run capture session. */\n> @@ -156,7 +157,6 @@ void SimpleCaptureUnbalanced::capture(unsigned int numRequests)\n> captureLimit_ = numRequests;\n> \n> /* Queue the recommended number of reqeuests. */\n> - std::vector> requests;\n> for (const std::unique_ptr &buffer : buffers) {\n> std::unique_ptr request = camera_->createRequest();\n> ASSERT_TRUE(request) << \"Can't create request\";\n> @@ -165,7 +165,7 @@ void SimpleCaptureUnbalanced::capture(unsigned int numRequests)\n> \n> ASSERT_EQ(camera_->queueRequest(request.get()), 0) << \"Failed to queue request\";\n> \n> - requests.push_back(std::move(request));\n> + requests_.push_back(std::move(request));\n> }\n> \n> /* Run capture session. */\n> diff --git a/src/apps/lc-compliance/simple_capture.h b/src/apps/lc-compliance/simple_capture.h\n> index fd9d2a97fd8d..2911d6019923 100644\n> --- a/src/apps/lc-compliance/simple_capture.h\n> +++ b/src/apps/lc-compliance/simple_capture.h\n> @@ -32,6 +32,7 @@ protected:\n> std::shared_ptr camera_;\n> std::unique_ptr allocator_;\n> std::unique_ptr config_;\n> + std::vector> requests_;\n> };\n> \n> class SimpleCaptureBalanced : public SimpleCapture\n> -- \n> 2.38.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 99BE9BE08B\n\tfor ;\n\tWed, 30 Nov 2022 10:55:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 26BC76333F;\n\tWed, 30 Nov 2022 11:55:54 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3C44060483\n\tfor ;\n\tWed, 30 Nov 2022 11:55:53 +0100 (CET)","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 DB5C555A;\n\tWed, 30 Nov 2022 11:55:52 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669805754;\n\tbh=dq2CbTS+/y+N5wGRuKBOlNZ08FWXs8i2nW09d4OcwJQ=;\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=jVvbp6qO9xmBFtu2GA4e1fLugOzE8R7U0dyQ5LZFu9bB1NlsmZcOyWzDaL7l0wBhV\n\tQV35crb/w59HhdevRxb3dvOaOERnuVzRT0VeRisgQfvXVM9o5W1PlG83Dotv/IMMcY\n\tPFh5nAb3QQgMtzkbuR1xmFGk67rcpO3j9WMQHMPEvVkqAFqD8gpeRAWu2BBAOYrik0\n\t3Biv46vC7WZDnTOfVe4lPz+w2DnTHuhwsbGdTB28UOcpq5IMReZCAV+nybKTOxxOCX\n\tsQQ+IYfJp0gIhHi0rOws7jd6x+obZ4m21bBlfoEQlmGJdzsGM1Sy2Ns1QMIwm0dj67\n\tiYOUtmxpmzI7g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669805753;\n\tbh=dq2CbTS+/y+N5wGRuKBOlNZ08FWXs8i2nW09d4OcwJQ=;\n\th=In-Reply-To:References:Subject:From:To:Cc:Date:From;\n\tb=dP7eknhdrkDAr+C28qHXPUxxOXFCZMAiEf0ZIDQ/A50Xby7RI+H3fFQTFspSRRt8w\n\t7g9ty1B0RdKz2ZExrDY9xH5h5c2bTohQMVE8fg7eoSrNyuxc/yB2xD3qTZMbLc//sR\n\t6YivpqaheO299XIuavyjnathRoEDoNEZoj6Y51Mc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"dP7eknhd\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20221130001044.651663-1-nfraprado@collabora.com>","References":"<20221130001044.651663-1-nfraprado@collabora.com>","To":"=?utf-8?q?N=C3=ADcolas?= F. R. A. Prado ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 30 Nov 2022 10:55:50 +0000","Message-ID":"<166980575093.1079859.5458882092490560008@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] lc-compliance: Fix segfault on\n\trequest destruction","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":25944,"web_url":"https://patchwork.libcamera.org/comment/25944/","msgid":"<20221130171840.vkb7r3ptsxmz3dgw@notapiano>","date":"2022-11-30T17:18:40","subject":"Re: [libcamera-devel] [PATCH] lc-compliance: Fix segfault on\n\trequest destruction","submitter":{"id":84,"url":"https://patchwork.libcamera.org/api/people/84/","name":"Nícolas F. R. A. Prado","email":"nfraprado@collabora.com"},"content":"On Wed, Nov 30, 2022 at 10:55:50AM +0000, Kieran Bingham wrote:\n> Hi Nícolas,\n> \n> Thanks for looking at this issue.\n> \n> Quoting Nícolas F. R. A. Prado via libcamera-devel (2022-11-30 00:10:44)\n> > The Request destructor cancels all associated framebuffers, and\n> > therefore assumes they are all still valid. The SimpleCaptureBalanced\n> > and SimpleCaptureUnbalanced classes that run the capture sessions on\n> > lc-compliance however were freeing the framebuffers before the requests,\n> > stored in an automatic variable, were destroyed. This resulted in a\n> > segfault when running lc-compliance.\n> > \n> > Solve the issue by moving the requests vector to the SimpleCapture class\n> > and making sure we clear it on stop() before freeing the framebuffers.\n> \n> I'm afraid Paul has 'just' beaten you to it with a patch posted\n> yesterday:\n> \n> - https://patchwork.libcamera.org/patch/17906/\n> \n> \n> Could you review and test his patch please?\n\nAh indeed, I missed that. Well, it's almost exactly the same patch, so that's\nreassuring :). I'll reply there, thanks.\n\nNícolas","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 DE79CBDE6B\n\tfor ;\n\tWed, 30 Nov 2022 17:18:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2BDF761F24;\n\tWed, 30 Nov 2022 18:18:48 +0100 (CET)","from madras.collabora.co.uk (madras.collabora.co.uk\n\t[46.235.227.172])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 98C4761F23\n\tfor ;\n\tWed, 30 Nov 2022 18:18:45 +0100 (CET)","from notapiano (unknown\n\t[IPv6:2600:4041:5b1a:cd00:524d:e95d:1a9c:492a])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\t(Authenticated sender: nfraprado)\n\tby madras.collabora.co.uk (Postfix) with ESMTPSA id B47516600363;\n\tWed, 30 Nov 2022 17:18:44 +0000 (GMT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669828728;\n\tbh=nBG+A9WMaR5Il2P6SxzK+7hu848WH3lAZsaBIRWqk64=;\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:Cc:\n\tFrom;\n\tb=ra0y8ZZINPEYTMvYLYYyBoD/OfV6oE12t2DglyCBQLxR8wNw4y4jYQkH0+IFrtF6M\n\tO8IeaiMKBX8/FQFWQamrhS5ta1Wsr9gVBmyw0ju7lxQttPtR3LiBkP0kVST1QZ5ZFi\n\tNt1tAPR+iodGjYKZxIMgSPlqgnVuF4KlXtFOouOT7o9mPWHsm8v8ZH4bjuICWP2xtK\n\tRcZFGhXZfnI6ddttFXDy8nF7UgQW4UNT+3HOx+Ez3U680vSIy9M5uBXmmyDDdnbq91\n\ti+iTZQE0i3gFI6HOextSeK8yKpUtYQ+7QQii5ow8PYU/qZnYnfeVQugwLlTlrFc6O2\n\t608SaWZp5P73w==","v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1669828725;\n\tbh=nBG+A9WMaR5Il2P6SxzK+7hu848WH3lAZsaBIRWqk64=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OAPDHaAfptbZF2HMxJMJJGhV7Cx2Ak/TTY/uTTx50/QXaQNpsSrM3+dUjx9RBBm04\n\tu9+FUfQwhK9mNp7VjV7HyZ4Fmt41rKjn0SLhxGoHDKeneMXqzwZNnDTsosPUe8bGjk\n\tbm/9KpxR0qDYTJWBhAYVIOLMfzDUS6W16aG9XlFX2A/r4RReziwDIGB7FV0lTrY4Jf\n\tR6nuziwZyiyAvwZ9KD4/Sblbmjg9P+nds2l4mgGbtrw1iUDKKyfIqnH+Ixrj6luBPz\n\tkHRhTpgBrHOz4HhVl+wHB2/Z8RDFEq69rYbVtu047wqPydxZtnna73Q8qII1bdXRT7\n\thdG0D1ulaid2w=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=collabora.com\n\theader.i=@collabora.com\n\theader.b=\"OAPDHaAf\"; dkim-atps=neutral","Date":"Wed, 30 Nov 2022 12:18:40 -0500","To":"Kieran Bingham ","Message-ID":"<20221130171840.vkb7r3ptsxmz3dgw@notapiano>","References":"<20221130001044.651663-1-nfraprado@collabora.com>\n\t<166980575093.1079859.5458882092490560008@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<166980575093.1079859.5458882092490560008@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH] lc-compliance: Fix segfault on\n\trequest destruction","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":"=?utf-8?q?N=C3=ADcolas_F=2E_R=2E_A=2E_Prado_via_libcamera-devel?=\n\t","Reply-To":"=?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4=?= Prado\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]