[RFC,v1] libcamera: request: Add "queued" flag
diff mbox series

Message ID 20251202122730.1579065-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [RFC,v1] libcamera: request: Add "queued" flag
Related show

Commit Message

Barnabás Pőcze Dec. 2, 2025, 12:27 p.m. UTC
Add an atomic flag which is used to check if the request has been queued
using `Camera::queueRequest()`. This is used to diagnose misuses
of requests, especially double queueing.

Link: https://gitlab.freedesktop.org/camera/libcamera/-/issues/197
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
This is another approach to detect misuses of Request objects.

https://patchwork.libcamera.org/patch/25280/
---
 include/libcamera/internal/request.h | 11 +++++++++++
 src/libcamera/camera.cpp             |  8 +++++++-
 src/libcamera/pipeline_handler.cpp   |  3 +++
 src/libcamera/request.cpp            | 22 ++++++++++++++++++++++
 4 files changed, 43 insertions(+), 1 deletion(-)

--
2.52.0

Patch
diff mbox series

diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h
index 78cb99f360..0e6216d6b5 100644
--- a/include/libcamera/internal/request.h
+++ b/include/libcamera/internal/request.h
@@ -7,6 +7,7 @@ 

 #pragma once

+#include <atomic>
 #include <chrono>
 #include <map>
 #include <memory>
@@ -35,6 +36,15 @@  public:

 	Camera *camera() const { return camera_; }
 	bool hasPendingBuffers() const;
+	bool hasBeenQueued() const { return queued_.load(std::memory_order_relaxed); }
+
+	bool tryQueue()
+	{
+		bool expected = false;
+		return queued_.compare_exchange_strong(expected, true, std::memory_order_relaxed);
+	}
+
+	void unQueue() { queued_.store(false, std::memory_order_relaxed); }

 	bool completeBuffer(FrameBuffer *buffer);
 	void complete();
@@ -57,6 +67,7 @@  private:
 	bool cancelled_;
 	uint32_t sequence_ = 0;
 	bool prepared_ = false;
+	std::atomic_bool queued_ = false;

 	std::unordered_set<FrameBuffer *> pending_;
 	std::map<FrameBuffer *, EventNotifier> notifiers_;
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 2e1e146a25..8fa45376d3 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -1340,11 +1340,15 @@  int Camera::queueRequest(Request *request)
 		return -EXDEV;
 	}

-	if (request->status() != Request::RequestPending) {
+	if (request->status() != Request::RequestPending || !request->_d()->tryQueue()) {
 		LOG(Camera, Error) << request->toString() << " is not valid";
 		return -EINVAL;
 	}

+	utils::scope_exit queuedFlagGuard([&] {
+		request->_d()->unQueue();
+	});
+
 	/*
 	 * The camera state may change until the end of the function. No locking
 	 * is however needed as PipelineHandler::queueRequest() will handle
@@ -1371,6 +1375,8 @@  int Camera::queueRequest(Request *request)
 	d->pipe_->invokeMethod(&PipelineHandler::queueRequest,
 			       ConnectionTypeQueued, request);

+	queuedFlagGuard.release();
+
 	return 0;
 }

diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 5c469e5bad..fade0b040d 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -470,6 +470,8 @@  void PipelineHandler::queueRequest(Request *request)
 {
 	LIBCAMERA_TRACEPOINT(request_queue, request);

+	ASSERT(request->_d()->hasBeenQueued());
+
 	Camera *camera = request->_d()->camera();
 	Camera::Private *data = camera->_d();
 	data->waitingRequests_.push(request);
@@ -591,6 +593,7 @@  void PipelineHandler::completeRequest(Request *request)

 	while (!data->queuedRequests_.empty()) {
 		Request *req = data->queuedRequests_.front();
+		ASSERT(req->_d()->hasBeenQueued());
 		if (req->status() == Request::RequestPending)
 			break;

diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index 60565f5984..6df73ba13a 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -82,6 +82,23 @@  bool Request::Private::hasPendingBuffers() const
 	return !pending_.empty();
 }

+/**
+ * \fn Request::Private::hasBeenQueued()
+ * \brief Check if a the request has the "queued" flag set
+ * \return True if the request has been marked as queued
+ */
+
+/**
+ * \fn Request::Private::tryQueue()
+ * \brief Try to set the "queued" flag if not already set
+ * \return True if it has been set successfully
+ */
+
+/**
+ * \fn Request::Private::unQueue()
+ * \brief Clear the "queued" flag
+ */
+
 /**
  * \brief Complete a buffer for the request
  * \param[in] buffer The buffer that has completed
@@ -123,6 +140,7 @@  void Request::Private::complete()

 	ASSERT(request->status() == RequestPending);
 	ASSERT(!hasPendingBuffers());
+	ASSERT(hasBeenQueued());

 	request->status_ = cancelled_ ? RequestCancelled : RequestComplete;

@@ -160,6 +178,7 @@  void Request::Private::cancel()

 	Request *request = _o<Request>();
 	ASSERT(request->status() == RequestPending);
+	ASSERT(hasBeenQueued());

 	doCancelRequest();
 }
@@ -175,6 +194,7 @@  void Request::Private::reset()
 	sequence_ = 0;
 	cancelled_ = false;
 	prepared_ = false;
+	unQueue();
 	pending_.clear();
 	notifiers_.clear();
 	timer_.reset();
@@ -391,6 +411,8 @@  void Request::reuse(ReuseFlag flags)
 {
 	LIBCAMERA_TRACEPOINT(request_reuse, this);

+	ASSERT(!(status_ == RequestPending && _d()->hasBeenQueued()));
+
 	_d()->reset();

 	if (flags & ReuseBuffers) {