Camera::acquire() is not thread-safe

Hans de Goede hdegoede at redhat.com
Thu Aug 15 17:33:54 CEST 2024


Hi,

On 8/15/24 5:09 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Thu, Aug 15, 2024 at 04:50:50PM +0200, Hans de Goede wrote:
>> On 8/15/24 4:37 PM, Laurent Pinchart wrote:
>>> On Thu, Aug 15, 2024 at 04:03:19PM +0200, Hans de Goede wrote:
>>>> Hi All,
>>>>
>>>> Camera::acquire() claims to be thread-safe, but it relies on
>>>> PipelineHandler::acquire() to provide that thread-safety and
>>>> PipelineHandler::acquire() allows multiple acquires of the same
>>>> pipeline-handler since a single pipeline-handler may register multiple
>>>> cameras.
>>>>
>>>> So that leaves the following possible race:
>>>>
>>>> 1. Thread A calls Camera::acquire()
>>>>    and passed the isAccessAllowed() check
>>>>
>>>> 2. Thread B calls Camera::acquire()
>>>>    and passed the isAccessAllowed() check
>>>>
>>>> 3. Thread A successfully calls pipe_->acquire()
>>>>
>>>> 4. Thread B successfully calls pipe_->acquire()
>>>
>>> In this example, are threads A and B running in the same process ?
>>
>> Yes I'm talking about 2 threads of the same process
>> racing here.
> 
> You are right, two threads in the same application could successfully
> acquire the same camera.
> 
> This shouldn't be hard to fix, but I'm wondering if libcamera should
> handle that, or if we should remove the threadsafe annotation for the
> function. I expect the vast majority of applications to not acquire the
> same camera from multiple threads, so adding a lock would be an extra
> cost for little gain. Any opinion ?

I agree that just removing the thread-safe annotation might be
the best fix. release() is already marked as not being thread-safe.

Regards,

Hans





>>>> 5. Thread A calls setState(Private::CameraAcquired)
>>>>
>>>> 6. Thread B calls setState(Private::CameraAcquired)
>>>>
>>>> And now we have 2 threads succesfully having acquired the same camera
>>>> which I believe is not supposed to be possible.
> 



More information about the libcamera-devel mailing list