[libcamera-devel] [PATCH] libcamera: base: Make MutexLocker(Mutex &mutex) non-blocking

[Eric Curtin]

On Thu, 28 Apr 2022 at 10:19, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Quoting Eric Curtin via libcamera-devel (2022-04-27 16:26:40)
> > Camera::acquire claims to be non-blocking, in order to achieve this,
> > std::unique_lock<std::mutex> constructor must be called with
> > try_to_lock so it doesn't block if a lock cannot be obtained.
> >
> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=127
> > Signed-off-by: Eric Curtin <ecurtin at redhat.com>
> > ---
> >  include/libcamera/base/mutex.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/libcamera/base/mutex.h b/include/libcamera/base/mutex.h
> > index 2d23e49e..c4a5f791 100644
> > --- a/include/libcamera/base/mutex.h
> > +++ b/include/libcamera/base/mutex.h
> > @@ -45,7 +45,7 @@ class LIBCAMERA_TSA_SCOPED_CAPABILITY MutexLocker final
> >  {
> >  public:
> >         explicit MutexLocker(Mutex &mutex) LIBCAMERA_TSA_ACQUIRE(mutex)
> > -               : lock_(mutex.mutex_)
> > +               : lock_(mutex.mutex_, std::try_to_lock)
>
> Does the code in question already call MutexLocker::try_lock(), but it
> still blocks?

By default, the std::unique_lock<Mutex>::unique_lock constructor tries
to obtain a blocking lock.

>
> I.e. - does this just enable / allow calling lock_.try_lock() where
> before callers would block regardless?

Yeah it just makes it non-blocking, so if it can't obtain a lock, it just fails.

>
> I can't quite grasp what the implications are here on other locations
> that would like to block ... does it make all calls to lock()
> non-blocking now? (even if they need it to be?)

Yes, this change probably is too broad, we could alternatively just
call the other constructor for this case alone, that's probably more
correct. Will resubmit this way.

>
> https://en.cppreference.com/w/cpp/thread/lock_tag_t doesn't seem to make

I found this link better personally:

https://en.cppreference.com/w/cpp/thread/unique_lock/unique_lock

> it clear what the full effects are :-(
>
> --
> Kieran
>
>
>
> >         {
> >         }
> >
> > --
> > 2.35.1
> >
>