[libcamera-devel,v3,08/17] py: Use libcamera's Mutex classes
diff mbox series

Message ID 20220701084521.31831-9-tomi.valkeinen@ideasonboard.com
State Accepted
Headers show
Series
  • Python bindings event handling
Related show

Commit Message

Tomi Valkeinen July 1, 2022, 8:45 a.m. UTC
Use libcamera's Mutex and MutexLocker instead of the std versions to get
thread safety annotations.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 src/py/libcamera/py_camera_manager.cpp | 4 ++--
 src/py/libcamera/py_camera_manager.h   | 7 ++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

Comments

Jacopo Mondi Aug. 18, 2022, 2:26 p.m. UTC | #1
Hi Tomi,

On Fri, Jul 01, 2022 at 11:45:12AM +0300, Tomi Valkeinen wrote:
> Use libcamera's Mutex and MutexLocker instead of the std versions to get
> thread safety annotations.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

I think this should be squashed as well!

> ---
>  src/py/libcamera/py_camera_manager.cpp | 4 ++--
>  src/py/libcamera/py_camera_manager.h   | 7 ++++---
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp
> index 3d422c9e..5600f661 100644
> --- a/src/py/libcamera/py_camera_manager.cpp
> +++ b/src/py/libcamera/py_camera_manager.cpp
> @@ -104,14 +104,14 @@ void PyCameraManager::readFd()
>
>  void PyCameraManager::pushRequest(Request *req)
>  {
> -	std::lock_guard guard(completedRequestsMutex_);
> +	MutexLocker guard(completedRequestsMutex_);
>  	completedRequests_.push_back(req);
>  }
>
>  std::vector<Request *> PyCameraManager::getCompletedRequests()
>  {
>  	std::vector<Request *> v;
> -	std::lock_guard guard(completedRequestsMutex_);
> +	MutexLocker guard(completedRequestsMutex_);
>  	swap(v, completedRequests_);
>  	return v;
>  }
> diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h
> index 710163e8..56bea13d 100644
> --- a/src/py/libcamera/py_camera_manager.h
> +++ b/src/py/libcamera/py_camera_manager.h
> @@ -5,7 +5,7 @@
>
>  #pragma once
>
> -#include <mutex>
> +#include <libcamera/base/mutex.h>
>
>  #include <libcamera/libcamera.h>
>
> @@ -34,8 +34,9 @@ private:
>  	std::unique_ptr<CameraManager> cameraManager_;
>
>  	UniqueFD eventFd_;
> -	std::mutex completedRequestsMutex_;
> -	std::vector<Request *> completedRequests_;
> +	libcamera::Mutex completedRequestsMutex_;
> +	std::vector<Request *> completedRequests_
> +		LIBCAMERA_TSA_GUARDED_BY(completedRequestsMutex_);
>
>  	void writeFd();
>  	void readFd();
> --
> 2.34.1
>
Laurent Pinchart Aug. 18, 2022, 7:21 p.m. UTC | #2
Hi Tomi,

Thank you for the patch.

On Fri, Jul 01, 2022 at 11:45:12AM +0300, Tomi Valkeinen wrote:
> Use libcamera's Mutex and MutexLocker instead of the std versions to get
> thread safety annotations.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Note that due to a recent change that made the mutex.h header private,
you will need to use libcamera_private instead of libcamera_public in
pycamera_deps in meson.build.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/py/libcamera/py_camera_manager.cpp | 4 ++--
>  src/py/libcamera/py_camera_manager.h   | 7 ++++---
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp
> index 3d422c9e..5600f661 100644
> --- a/src/py/libcamera/py_camera_manager.cpp
> +++ b/src/py/libcamera/py_camera_manager.cpp
> @@ -104,14 +104,14 @@ void PyCameraManager::readFd()
>  
>  void PyCameraManager::pushRequest(Request *req)
>  {
> -	std::lock_guard guard(completedRequestsMutex_);
> +	MutexLocker guard(completedRequestsMutex_);
>  	completedRequests_.push_back(req);
>  }
>  
>  std::vector<Request *> PyCameraManager::getCompletedRequests()
>  {
>  	std::vector<Request *> v;
> -	std::lock_guard guard(completedRequestsMutex_);
> +	MutexLocker guard(completedRequestsMutex_);
>  	swap(v, completedRequests_);
>  	return v;
>  }
> diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h
> index 710163e8..56bea13d 100644
> --- a/src/py/libcamera/py_camera_manager.h
> +++ b/src/py/libcamera/py_camera_manager.h
> @@ -5,7 +5,7 @@
>  
>  #pragma once
>  
> -#include <mutex>
> +#include <libcamera/base/mutex.h>
>  
>  #include <libcamera/libcamera.h>
>  
> @@ -34,8 +34,9 @@ private:
>  	std::unique_ptr<CameraManager> cameraManager_;
>  
>  	UniqueFD eventFd_;
> -	std::mutex completedRequestsMutex_;
> -	std::vector<Request *> completedRequests_;
> +	libcamera::Mutex completedRequestsMutex_;
> +	std::vector<Request *> completedRequests_
> +		LIBCAMERA_TSA_GUARDED_BY(completedRequestsMutex_);
>  
>  	void writeFd();
>  	void readFd();
Tomi Valkeinen Aug. 19, 2022, 5:52 a.m. UTC | #3
On 18/08/2022 22:21, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Fri, Jul 01, 2022 at 11:45:12AM +0300, Tomi Valkeinen wrote:
>> Use libcamera's Mutex and MutexLocker instead of the std versions to get
>> thread safety annotations.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> 
> Note that due to a recent change that made the mutex.h header private,
> you will need to use libcamera_private instead of libcamera_public in
> pycamera_deps in meson.build.

Hmm, compiles fine with today's upstream master without changing to 
libcamera_private.

  Tomi
Tomi Valkeinen Aug. 19, 2022, 6:20 a.m. UTC | #4
On 19/08/2022 08:52, Tomi Valkeinen wrote:
> On 18/08/2022 22:21, Laurent Pinchart wrote:
>> Hi Tomi,
>>
>> Thank you for the patch.
>>
>> On Fri, Jul 01, 2022 at 11:45:12AM +0300, Tomi Valkeinen wrote:
>>> Use libcamera's Mutex and MutexLocker instead of the std versions to get
>>> thread safety annotations.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>
>> Note that due to a recent change that made the mutex.h header private,
>> you will need to use libcamera_private instead of libcamera_public in
>> pycamera_deps in meson.build.
> 
> Hmm, compiles fine with today's upstream master without changing to 
> libcamera_private.

Ah, we had "'-DLIBCAMERA_BASE_PRIVATE'" there. I'll drop that and switch 
to libcamera_private.

  Tomi

Patch
diff mbox series

diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp
index 3d422c9e..5600f661 100644
--- a/src/py/libcamera/py_camera_manager.cpp
+++ b/src/py/libcamera/py_camera_manager.cpp
@@ -104,14 +104,14 @@  void PyCameraManager::readFd()
 
 void PyCameraManager::pushRequest(Request *req)
 {
-	std::lock_guard guard(completedRequestsMutex_);
+	MutexLocker guard(completedRequestsMutex_);
 	completedRequests_.push_back(req);
 }
 
 std::vector<Request *> PyCameraManager::getCompletedRequests()
 {
 	std::vector<Request *> v;
-	std::lock_guard guard(completedRequestsMutex_);
+	MutexLocker guard(completedRequestsMutex_);
 	swap(v, completedRequests_);
 	return v;
 }
diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h
index 710163e8..56bea13d 100644
--- a/src/py/libcamera/py_camera_manager.h
+++ b/src/py/libcamera/py_camera_manager.h
@@ -5,7 +5,7 @@ 
 
 #pragma once
 
-#include <mutex>
+#include <libcamera/base/mutex.h>
 
 #include <libcamera/libcamera.h>
 
@@ -34,8 +34,9 @@  private:
 	std::unique_ptr<CameraManager> cameraManager_;
 
 	UniqueFD eventFd_;
-	std::mutex completedRequestsMutex_;
-	std::vector<Request *> completedRequests_;
+	libcamera::Mutex completedRequestsMutex_;
+	std::vector<Request *> completedRequests_
+		LIBCAMERA_TSA_GUARDED_BY(completedRequestsMutex_);
 
 	void writeFd();
 	void readFd();