[libcamera-devel] [PATCH] libcamera: base: thread: Protect exitCode by mutex in exit()

[Umang Jain]

Hi,

On 6/20/22 13:57, Laurent Pinchart wrote:
> Hi Umang,
>
> Thank you for the patch.
>
> On Mon, Jun 20, 2022 at 10:41:50AM +0530, Umang Jain via libcamera-devel wrote:
>> From: Hirokazu Honda <hiroh at chromium.org>
>>
>> Thread::exit() accesses data_->exitCode without acquiring a lock.
>> Fix it.
> Could you explain here why the lock is needed ?

This is originally from "Apply clang thread safety annotation 
libcamera-core and v4l2" series

Isn't exitCode_ should always be accessed with the lock? It seems so, in 
the Thread class.

I get it it's exit() / cleanup part so we might not care of the locking,
but when we introduce thread annotation, it becomes a source of 
complaining, hence the patch.

>
>> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> ---
>>   src/libcamera/base/thread.cpp | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp
>> index 6bda9d14..ff65641e 100644
>> --- a/src/libcamera/base/thread.cpp
>> +++ b/src/libcamera/base/thread.cpp
>> @@ -392,7 +392,10 @@ void Thread::finishThread()
>>    */
>>   void Thread::exit(int code)
>>   {
>> +	data_->mutex_.lock();
>>   	data_->exitCode_ = code;
>> +	data_->mutex_.unlock();
>> +
>>   	data_->exit_.store(true, std::memory_order_release);
>>   
>>   	EventDispatcher *dispatcher = data_->dispatcher_.load(std::memory_order_relaxed);