[libcamera-devel] [PATCH] lc-compliance: Fix segfault on request destruction
Kieran Bingham
kieran.bingham at ideasonboard.com
Wed Nov 30 11:55:50 CET 2022
Hi Nícolas,
Thanks for looking at this issue.
Quoting Nícolas F. R. A. Prado via libcamera-devel (2022-11-30 00:10:44)
> The Request destructor cancels all associated framebuffers, and
> therefore assumes they are all still valid. The SimpleCaptureBalanced
> and SimpleCaptureUnbalanced classes that run the capture sessions on
> lc-compliance however were freeing the framebuffers before the requests,
> stored in an automatic variable, were destroyed. This resulted in a
> segfault when running lc-compliance.
>
> Solve the issue by moving the requests vector to the SimpleCapture class
> and making sure we clear it on stop() before freeing the framebuffers.
I'm afraid Paul has 'just' beaten you to it with a patch posted
yesterday:
- https://patchwork.libcamera.org/patch/17906/
Could you review and test his patch please?
--
Kieran
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
>
> ---
>
> src/apps/lc-compliance/simple_capture.cpp | 8 ++++----
> src/apps/lc-compliance/simple_capture.h | 1 +
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/apps/lc-compliance/simple_capture.cpp b/src/apps/lc-compliance/simple_capture.cpp
> index ab5cb35c11f2..d02c7b45f180 100644
> --- a/src/apps/lc-compliance/simple_capture.cpp
> +++ b/src/apps/lc-compliance/simple_capture.cpp
> @@ -64,6 +64,8 @@ void SimpleCapture::stop()
>
> camera_->requestCompleted.disconnect(this);
>
> + requests_.clear();
> +
> Stream *stream = config_->at(0).stream();
> allocator_->free(stream);
> }
> @@ -95,7 +97,6 @@ void SimpleCaptureBalanced::capture(unsigned int numRequests)
> captureLimit_ = numRequests;
>
> /* Queue the recommended number of reqeuests. */
> - std::vector<std::unique_ptr<libcamera::Request>> requests;
> for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> std::unique_ptr<Request> request = camera_->createRequest();
> ASSERT_TRUE(request) << "Can't create request";
> @@ -104,7 +105,7 @@ void SimpleCaptureBalanced::capture(unsigned int numRequests)
>
> ASSERT_EQ(queueRequest(request.get()), 0) << "Failed to queue request";
>
> - requests.push_back(std::move(request));
> + requests_.push_back(std::move(request));
> }
>
> /* Run capture session. */
> @@ -156,7 +157,6 @@ void SimpleCaptureUnbalanced::capture(unsigned int numRequests)
> captureLimit_ = numRequests;
>
> /* Queue the recommended number of reqeuests. */
> - std::vector<std::unique_ptr<libcamera::Request>> requests;
> for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> std::unique_ptr<Request> request = camera_->createRequest();
> ASSERT_TRUE(request) << "Can't create request";
> @@ -165,7 +165,7 @@ void SimpleCaptureUnbalanced::capture(unsigned int numRequests)
>
> ASSERT_EQ(camera_->queueRequest(request.get()), 0) << "Failed to queue request";
>
> - requests.push_back(std::move(request));
> + requests_.push_back(std::move(request));
> }
>
> /* Run capture session. */
> diff --git a/src/apps/lc-compliance/simple_capture.h b/src/apps/lc-compliance/simple_capture.h
> index fd9d2a97fd8d..2911d6019923 100644
> --- a/src/apps/lc-compliance/simple_capture.h
> +++ b/src/apps/lc-compliance/simple_capture.h
> @@ -32,6 +32,7 @@ protected:
> std::shared_ptr<libcamera::Camera> camera_;
> std::unique_ptr<libcamera::FrameBufferAllocator> allocator_;
> std::unique_ptr<libcamera::CameraConfiguration> config_;
> + std::vector<std::unique_ptr<libcamera::Request>> requests_;
> };
>
> class SimpleCaptureBalanced : public SimpleCapture
> --
> 2.38.1
>
More information about the libcamera-devel
mailing list